[Delphi7] Threading and synchronizing with Critical Section

[Atak_Snajpera]

Worker Thread 1

EnterCriticalSection(CriticalSection1);
Server1Progress:=Received_Data; // Server1Progress is a global variable, Received_Data is declared in thread
LeaveCriticalSection(CriticalSection1);

Supervisor Thread
procedure TForm1.JvThreadSupervisorExecute(Sender: TObject; Params: Pointer);
begin

while form1.JvThreadSupervisor.Terminated=false do
begin

form1.JvThreadSupervisor.Synchronize(UpdateTotalStatusinGUI);
sleep(250);

end;

end;


procedure TForm1.UpdateTotalStatusinGUI;
begin

form1.JvListView1.Items.Item[ StrToInt(Server1AssociatedChunk)-1 ].SubItems[1] := Server1Progress;
if (Pos('encoded',Server1Progress)<>0) or (Pos('Queue',Server1Progress)<>0) then Server1Progress:='';
application.ProcessMessages;

...


And here is my question.
Do I also have to add EnterCriticalSection() and LeaveCriticalSection() in .UpdateTotalStatusinGUI ?

procedure TForm1.UpdateTotalStatusinGUI;
begin

EnterCriticalSection();

form1.JvListView1.Items.Item[ StrToInt(Server1AssociatedChunk)-1 ].SubItems[1] := Server1Progress;
if (Pos('encoded',Server1Progress)<>0) or (Pos('Queue',Server1Progress)<>0) then Server1Progress:='';
application.ProcessMessages;

LeaveCriticalSection();

...

[LoRd_MuldeR]

In Delphi as well as many other GUI frame-works the GUI widgets are not thread-safe. That's why you are not allowed to access any GUI widgets directly from a thread, except for the applications "main" thread. With TThread.Synchronize() an arbitrary thread can execute a method in the context of the "main" thread, pretty similar (and equally ugly!) to BeginInvoke() in C#. That's why you have to use Synchronize() whenever you want to access a GUI control from a thread other than the "main" thread. Now to your question: You may use this mechanism to serialize the access to a global variable. As long as all threads that access the global variable for reading or writing always do this via Synchronize(), all access to that variable will effectively be done from the "main" thread and will therefore be serialized. You don't need a Mutex (CriticalSection) in that case. If, however, some thread somewhere accesses the variable outside of a Synchronize(), everything is lost! Furthermore: Instead of making all threads that need to access the global variable use Synchronize() for this purpose, you may use a Mutex (CriticalSection) to "protect" the variable. This will probably cause less overhead, because it doesn't involve the "main" thread and still guarantees that only one thread can access the variable at a time - as long as all access to that variable is always protected by the Mutex (CriticalSection). Finally: Note that for accessing a GUI widget, using a Mutex (CriticalSection) is not sufficient as a replacement for Synchronize(). That's because the Mutex would only ensure that your threads don't try to access the GUI widget concurrently. Still, while one of your threads is accessing the widget, some other thread (e.g. the "main" thread that is processing some Message in its Message Loop!) may access the control at the same time and cause big trouble! The "main" Message Loop, implemented deep inside the VCL, doesn't know anything about your Mutex...

[Atak_Snajpera]

My goal is to protect only global variables which are read/written in few places (threads). I know that Synchronize() must always be used when thread wants to update something in GUI. (you taught me that :)

[LoRd_MuldeR]

Well, if you really only want to "protect" your own global variable, i.e. serialize all access to that variable from multiple threads, then I think using Synchronize() would be overkill here. Guarding the access to the variable with a Mutex (Critical Section) should be perfectly sufficient, as long as you do it in all places where the variable is accessed. Also be sure that on each code path the Mutex is released after acquisition, otherwise you can easily get a deadlock.

On the other hand, methods that are called via Synchronize() already are serialized, so using EnterCriticalSection() inside a Synchronize()'d method is pointless and an unnecessary overhead - unless, of course, there is another non-Synchronize()'d method that may access the shared resource too...

[Atak_Snajpera]

As always thanks for your help