Page 1 of 2

I still ask about thread synchronization

PostPosted: Tue Jun 11, 2019 4:42 am
by 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?

Re: I still ask about thread synchronization

PostPosted: Tue Jun 11, 2019 12:48 pm
by rlebeau
mark_c wrote: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.

mark_c wrote: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.

Re: I still ask about thread synchronization

PostPosted: Tue Jun 11, 2019 1:06 pm
by 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

Re: I still ask about thread synchronization

PostPosted: Tue Jun 11, 2019 2:57 pm
by rlebeau
mark_c wrote: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;
}

Re: I still ask about thread synchronization

PostPosted: Wed Jun 12, 2019 7:04 am
by mark_c
thanks as always Remy,
I believe the time has come to retire the good BCB6, companion of many adventures.

Re: I still ask about thread synchronization

PostPosted: Wed Jun 12, 2019 1:10 pm
by 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.

Re: I still ask about thread synchronization

PostPosted: Wed Jun 12, 2019 1:30 pm
by 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.

Re: I still ask about thread synchronization

PostPosted: Wed Jun 12, 2019 7:11 pm
by 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.

Re: I still ask about thread synchronization

PostPosted: Thu Jun 13, 2019 12:55 am
by mark_c
thank you for the tip; but must the UI be redesigned or is there some trick?

Re: I still ask about thread synchronization

PostPosted: Thu Jun 13, 2019 12:58 am
by 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;
}
//---------------------------------------------------------------------------

Re: I still ask about thread synchronization

PostPosted: Thu Jun 13, 2019 10:10 am
by HsiaLin
mark_c wrote: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.

Re: I still ask about thread synchronization

PostPosted: Sun Jun 16, 2019 8:54 pm
by samuelv
mark_c wrote: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.

Re: I still ask about thread synchronization

PostPosted: Mon Jun 17, 2019 10:28 am
by 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

Re: I still ask about thread synchronization

PostPosted: Tue Jun 18, 2019 2:05 pm
by rlebeau
mark_c wrote: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;
}
//---------------------------------------------------------------------------

Re: I still ask about thread synchronization

PostPosted: Wed Jun 19, 2019 12:37 am
by 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?