I still ask about thread synchronization

[mark_c]

Hello,
an old problem has already been discussed here, but I was observing that there is no mechanism to synchronize the VCL and CreateThread () methods. If from CreateThread I try to write in a label I almost certainly have a collision.
Is there a way?

[rlebeau]

I was observing that there is no mechanism to synchronize the VCL and CreateThread () methods.

Sure, there is - the TThread::Synchronize() and TThread::Queue() methods, which have static versions available that can be called outside of TThread-oriented threads.

Or, you can use the Win32 SendMessage() and PostMessage() functions to send messages to UI windows, and then have the message handlers do the UI work as needed.

If from CreateThread I try to write in a label I almost certainly have a collision.
Is there a way?

You must sync with the main UI thread in order to access VCL controls correctly and safely.

[mark_c]

thanks Remy.
Do you have any examples of use of CreateThread () that writes in a Label (UI) managed by the VCL?

Thank you

[rlebeau]

Do you have any examples of use of CreateThread () that writes in a Label (UI) managed by the VCL?

There are many different ways to handle this:

1. using the static TThread methods:

Code: Select all

void __fastcall TForm1::DoUpdateLabel1()
{
    String S = String().sprintf(_D("Hello @ %u"), GetTickCount());
    Label1->Caption = S;
}

// Thread procedure used with CreateThread()...
DWORD WINAPI MyThreadProc(LPVOID lpParameter)
{
    TThread::Synchronize(NULL, Form1->DoUpdateLabel1);
    or:
    TThread::Queue(NULL, Form1->DoUpdateLabel1);

    return 0;
}


Alternatively:

Code: Select all

struct Label1Updater
{
    String NewCaption;

    void __fastcall DoUpdateSync()
    {
        Form1->Label1->Caption = NewCaption;
    }

    void __fastcall DoUpdateAsync()
    {
        DoUpdateSync();
        delete this;
    }
};

void __fastcall DoUpdateLabel1(String NewCaption, bool Synchronous)
{
    if (Synchronous)
    {
        Label1Updater updater;
        updater.NewCaption = NewCaption;
        TThread::Synchronize(NULL, updater.DoUpdateSync);
    }
    else
    {
        Label1Updater *updater = new Label1Updater;
        updater->NewCaption = NewCaption;
        TThread::Queue(NULL, updater->DoUpdateAsync);
    }
}

DWORD WINAPI MyThreadProc(LPVOID lpParameter)
{
    String S = String().sprintf(_D("Hello @ %u"), GetTickCount());

    DoUpdateLabel1(S, true);
    or:
    DoUpdateLabel1(S, false);

    return 0;
}


Alternatively (Clang-based compilers only, see How to Handle Delphi Anonymous Methods in C++: Using a Lambda Expression):

Code: Select all

DWORD WINAPI MyThreadProc(LPVOID lpParameter)
{
    String S = String().sprintf(_D("Hello @ %u"), GetTickCount());

    TThread::Synchronize(NULL,
        [=](){ Form1->Label1->Caption = S; }
    );

    or:

    TThread::Queue(NULL,
        [=](){ Form1->Label1->Caption = S; }
    );

    return 0;
}


Alternatively (non CLang-based compilers only, see How to Handle Delphi Anonymous Methods in C++: Using a Functor (Function Object)):

Code: Select all

struct Label1Updater
{
    String NewCaption;

    Label1Updater(String s) : NewCaption(s) {}
    bool operator()() { Form1->Label1->Caption = NewCaption; }
};
 
DWORD WINAPI MyThreadProc(LPVOID lpParameter)
{
    String S = String().sprintf(_D("Hello @ %u"), GetTickCount());

    typedef TMethodRef<TThreadProcedure, Label1Updater, void> MyMethRef;
    _di_TThreadProcedure proc(new MyMethRef(Label1Updater(S)));

    TThread::Synchronize(NULL,  proc);
    or:
    TThread::Queue(NULL, proc);

    return 0;
}


2. using the Win32 API:

Code: Select all

const UINT APPWM_UPDATELABEL1 = WM_APP + 1;

__fastcall TForm1::TForm1(TComponent *Owner)
    : TForm(Owner)
{
    FMyThreadWnd = AllocateHWnd(MyThreadWndProc);
}

__fastcall TForm1::~TForm1()
{
    DeallocateHWnd(FMyThreadWnd);
};

void __fastcall TForm1::MyThreadWndProc(TMessage &Message)
{
    if (Message.Msg == APPWM_UPDATELABEL1)
    {
        String *S = reinterpret_cast<String*>(Message.LParam);
        Label1->Caption = *S;
        if (Message.WParam)
            delete S;
    }
    else
        Message.Result = DefWindowProc(FMyThreadWnd, Message.Msg, Message.WParam, Message.LParam);
}

void __fastcall TForm1::UpdateLabel1(String NewCaption, bool Synchronous)
{
    if (Synchronous)
    {
        SendMessage(FMyThreadWnd, APPWM_UPDATELABEL1, 0, reinterpret_cast<LPARAM>(&S));
    }
    else
    {
        String *S = new String(NewCaption);
        if (!PostMessage(FMyThreadWnd, APPWM_UPDATELABEL1, 1, reinterpret_cast<LPARAM>(S)))
            delete S;
    }
}

DWORD WINAPI MyThreadProc(LPVOID lpParameter)
{
    String S = String().sprintf(_D("Hello @ %u"), GetTickCount());

    Form1->UpdateLabel1(S, true);
    or:
    Form1->UpdateLabel1(S, false);

    return 0;
}


[mark_c]

thanks as always Remy,
I believe the time has come to retire the good BCB6, companion of many adventures.

[HsiaLin]

I had many doubts about leaving bcb6 too but once you get used to the new IDE its much better than the old ways, in my opinion.

[mark_c]

I didn't want to change the glorious BCB6 because the BCB6 projects are not directly compatible with Embarcadero but I have to copy and paste an infinite number of times.
Anyway thanks for your testimony.

[HsiaLin]

You dont have to copy and paste, just make a new empty project and add your old cpp and h files to it. Dont forget to add the .dfm files to the folder too.

You`ll probably run into a couple things that need to change, like any Strings you have declared will be unicode strings in XE.

[mark_c]

thank you for the tip; but must the UI be redesigned or is there some trick?

[mark_c]

I ask you if you think there may be collisions in this version, I don't see any

thank you

Code: Select all

//---------------------------------------------------------------------------

#include <vcl.h>
#pragma hdrstop

#include "Unit1.h"

#include <windows.h>
#include <stdlib.h>
#include <stdio.h>

bool bAbort;

void SendMyMessage(char *msg);

DWORD WINAPI Thread1( LPVOID lpParameter );
DWORD WINAPI Thread2( LPVOID lpParameter );
DWORD ID1, ID2;
HANDLE Trd1, Trd2;




int a, b;
AnsiString msg;

//---------------------------------------------------------------------------
#pragma package(smart_init)
#pragma resource "*.dfm"
TForm1 *Form1;
//---------------------------------------------------------------------------
__fastcall TForm1::TForm1(TComponent* Owner)
   : TForm(Owner)
{
   ServerSocket1->Port = 5000;
   ServerSocket1->Active = True;
}
//---------------------------------------------------------------------------

void __fastcall TForm1::Button1Click(TObject *Sender)
{
   bAbort = false;

   Trd1 = CreateThread(NULL,0,Thread1,NULL,0,&ID1);
   Trd2 = CreateThread(NULL,0,Thread2,NULL,0,&ID2);

   Button1->Enabled=false;
   Button2->Enabled=true;

    Form1->Caption = "Started.....";
}
//---------------------------------------------------------------------------

void __fastcall TForm1::Button2Click(TObject *Sender)
{
   bAbort = true;

   Button1->Enabled=true;
   Button2->Enabled=false;

   Form1->Caption = "Stopped.....";
}
//---------------------------------------------------------------------------
void __fastcall TForm1::DoUpdateLabel()
{
   Label1->Caption = a;
   Label2->Caption = b;
}
//---------------------------------------------------------------------------
DWORD WINAPI Thread1( LPVOID lpParameter )
{
   while(!bAbort)
   {
      msg=" msg1 ";
      TThread::Synchronize(NULL, Form1->SendMyMessage);
      a++;
      TThread::Synchronize(NULL, Form1->DoUpdateLabel);
      Sleep(1);
   }
   return 0;
}
//---------------------------------------------------------------------------

DWORD WINAPI Thread2( LPVOID lpParameter )
{
   while(!bAbort)
   {
      msg=" msg2 ";
      TThread::Synchronize(NULL, Form1->SendMyMessage);
      b++;
      TThread::Synchronize(NULL, Form1->DoUpdateLabel);
      Sleep(1);
   }
   return 0;
}

//---------------------------------------------------------------------------

//void SendMyMessage(char *msg)
void __fastcall TForm1::SendMyMessage()
{
   try{
      for(int actconn = 0; actconn < Form1->ServerSocket1->Socket->ActiveConnections; actconn++)
      Form1->ServerSocket1->Socket->Connections[actconn]->SendText(msg);

   } catch(...) { }

}
//---------------------------------------------------------------------------

void __fastcall TForm1::FormDestroy(TObject *Sender)
{
   TerminateThread(Trd1,0);
   TerminateThread(Trd2,0);

   ServerSocket1->Active = False;
}
//---------------------------------------------------------------------------

void __fastcall TForm1::ServerSocket1ClientError(TObject *Sender, TCustomWinSocket *Socket,
          TErrorEvent ErrorEvent, int &ErrorCode)
{
   if (ErrorCode == 10053) {
      Socket->Close();
   }

   ErrorCode = 0;
}
//---------------------------------------------------------------------------


[HsiaLin]

thank you for the tip; but must the UI be redesigned or is there some trick?

Your programs UI is stored in the .dfm files. As long as you have cpp, h and dfm files together you good to go.

[samuelv]

I didn't want to change the glorious BCB6 because the BCB6 projects are not directly compatible with geometry dash Embarcadero but I have to copy and paste an infinite number of times.
Anyway thanks for your testimony.

You just need to create a new empty project then add old cpp and h files to it.

[mark_c]

I tried to create a new project and then overwriting some embarcadero files with those of bcb6 and it seems to work correctly

[rlebeau]

I ask you if you think there may be collisions in this version, I don't see any

There are some issues with your code.

You have a race condition on your global 'msg' variable. Both threads are modifying 'msg' without synchronizing with each other, and without adequately synchronizing with the main UI thread. For instance, after thread 1 modifies 'msg' and then blocks waiting for the main UI thread to use 'msg', thread 2 is still free to modify 'msg' at will, even while the loop inside of SendMyMessage() is actively using it. So you need to put a full synchronization lock around 'msg', such as a critical section or mutex.

You also have a race condition on your global 'a' and 'b' variables, too. For those, you can simply use the Interlocked API instead of using a full locking mechanism.

Also, in your OnDestroy event (and FYI, DO NOT use the OnDestroy event in C++! Use the Form's destructor instead), DO NOT use TerminateThread() at all! Signal your threads to abort, and then WAIT for them to actually finish their work.

With all of that said, try something more like the following instead (which BTW, really isn't the best way to handle what you are doing, but this is just to show how to fix the code you provided):

Code: Select all

//---------------------------------------------------------------------------

#include <vcl.h>
#pragma hdrstop

#include "Unit1.h"

#include <stdlib.h>
#include <stdio.h>

bool bAbort = false;

void SendMyMessage(char *msg);

DWORD WINAPI Thread1( LPVOID lpParameter );
DWORD WINAPI Thread2( LPVOID lpParameter );
HANDLE Trd1 = NULL, Trd2 = NULL;

LONG a, b;

System::String msg;
CRITICAL_SECTION msglock;

//---------------------------------------------------------------------------
#pragma package(smart_init)
#pragma resource "*.dfm"
TForm1 *Form1;
//---------------------------------------------------------------------------
__fastcall TForm1::TForm1(TComponent* Owner)
   : TForm(Owner)
{
   InitializeCriticalSection(&msglock);
   ServerSocket1->Port = 5000;
   ServerSocket1->Active = True;
}
//---------------------------------------------------------------------------
__fastcall TForm1::~TForm1()
{
   StopThreads();
   ServerSocket1->Active = False;
   DeleteCriticalSection(&msglock);
}
//---------------------------------------------------------------------------
void __fastcall TForm1::Button1Click(TObject *Sender)
{
   StopThreads();

   bAbort = false;
   a = b = 0;

   Trd1 = CreateThread(NULL, 0, Thread1, NULL, 0, NULL);
   Trd2 = CreateThread(NULL, 0, Thread2, NULL, 0, NULL);

   Button1->Enabled = false;
   Button2->Enabled = true;

   Caption = "Started.....";
}
//---------------------------------------------------------------------------
void __fastcall TForm1::Button2Click(TObject *Sender)
{
   StopThreads();

   Button1->Enabled = true;
   Button2->Enabled = false;

   Caption = "Stopped.....";
}
//---------------------------------------------------------------------------
void __fastcall TForm1::StopThreads()
{
   if ((!Trd1) && (!Trd2))
      return;

   bAbort = true;

   HANDLE arr[3];
   DWORD num = 0;

   arr[num++] = reinterpret_cast<HANDLE>(System::Classes::SyncEvent);
   if (Trd1) arr[num++] = Trd1;
   if (Trd2) arr[num++] = Trd2;

   do
   {
      DWORD WaitResult = MsgWaitForMultipleObjects(num, arr, FALSE, INFINITE, QS_SENDMESSAGE);
      if (WaitResult == WAIT_FAILED)
      {
         RaiseLastOSError();
         break;
      }

      if ((WaitResult >= WAIT_OBJECT_0) && (WaitResult < (WAIT_OBJECT_0 + num)))
      {
         DWORD idx = WaitResult - WAIT_OBJECT_0;
         if (idx == 0)
            System::Classes::CheckSynchronize();
         else
         {
            HANDLE h = arr[idx];
            CloseHandle(h);
            if (h == Trd1) Trd1 = NULL;
            else if (h == Trd2) Trd2 = NULL;

            num = 1;
            if (Trd1) arr[num++] = Trd1;
            if (Trd2) arr[num++] = Trd2;
         }
      }
      else if (WaitResult == (WAIT_OBJECT_0 + num))
      {
         MSG msg;
         PeekMessage(&Msg, 0, 0, 0, PM_NOREMOVE);
      }
   }
   while (num > 1);
}
//---------------------------------------------------------------------------
void __fastcall TForm1::DoUpdateLabel()
{
   Label1->Caption = InterlockedExchangeAdd(&a, 0);
   Label2->Caption = InterlockedExchangeAdd(&b, 0);
}
//---------------------------------------------------------------------------
DWORD WINAPI Thread1( LPVOID lpParameter )
{
   while (!bAbort)
   {
      EnterCriticalSection(&msglock);
      msg = " msg1 ";
      LeaveCriticalSection(&msglock);
      TThread::Synchronize(NULL, Form1->SendMyMessage);
      InterlockedIncrement(&a);
      TThread::Synchronize(NULL, Form1->DoUpdateLabel);
      Sleep(1);
   }
   return 0;
}
//---------------------------------------------------------------------------
DWORD WINAPI Thread2( LPVOID lpParameter )
{
   while (!bAbort)
   {
      EnterCriticalSection(&msglock);
      msg=" msg2 ";
      LeaveCriticalSection(&msglock);
      TThread::Synchronize(NULL, Form1->SendMyMessage);
      InterlockedIncrement(&b);
      TThread::Synchronize(NULL, Form1->DoUpdateLabel);
      Sleep(1);
   }
   return 0;
}
//---------------------------------------------------------------------------
//void SendMyMessage(char *msg)
void __fastcall TForm1::SendMyMessage()
{
   System::String local_msg;
   try
   {
      EnterCriticalSection(&msglock);
      local_msg = msg;
      LeaveCriticalSection(&msglock);

      local_msg.Unique();

      for(int actconn = 0; actconn < ServerSocket1->Socket->ActiveConnections; ++actconn)
         ServerSocket1->Socket->Connections[actconn]->SendText(local_msg);
   }
   catch(...)
   {
   }
}
//---------------------------------------------------------------------------
void __fastcall TForm1::ServerSocket1ClientError(TObject *Sender, TCustomWinSocket *Socket,
   TErrorEvent ErrorEvent, int &ErrorCode)
{
   Socket->Close();
   ErrorCode = 0;
}
//---------------------------------------------------------------------------


[mark_c]

thanks Remy as always.
I ask you: if instead of using a single global variable msg I would use two global variables, and instead of using a single socket I would use two with two distinct ports:

Code: Select all

Ansistring msg1, msg2;

ServerSocket1-> Port = 5000;
ServerSocket2-> Port = 5001;


even if not very elegant, in this way I would solve all the problems of race condition, right?