Sign in Join

Live chat for Free Pascal, Lazarus, Delphi and every Pascal programmer.

Mailing lists / fpc-pascal / 2008-10

PostMessage() from a thread (in a thread save manner)

5 messages · 4 people ·

Reply by email Subscribe to this list Your subscription

Read-only archive. Use j and k to move between messages.

Graeme Geldenhuys

Some developers prefer to pass in a GUI component to a thread instance, so the thread knows what widget to update. Other users prefer to post a "gui update" message to the application event queue.

I'm very new to threads and might know just enough to make in dangerous. :-) In fpGUI the application's "custom" event queue is a linked-list structure. If threads post messages to that queue, I obviously need to make the queue manipulation code thread safe.

fpgApplication is a global singleton of the TfpgApplication class. fpgPostMessage() is a stand-alone function (not a member of TfpgApplication class). To make fpgPostMessage() thread save, I instantiated a TCriticalSection instance in the TfpgApplication class
- in the constructor. I added two member functions .Lock and .Unlock which calls FCritSect.Enter and FCritSect.Leave respectively. In fpgPostMessage, I call fpgApplication.Lock, then manipulate the message queue and then call fpgApplication.Unlock.

Is this the correct way to make my event queue thread save?

eg:

procedure fpgPostMessage(Sender, Dest: TObject; MsgCode: integer;
    var aparams: TfpgMessageParams); overload;
var
  p: PfpgMessageRec;
begin
  if Dest = nil then
    Exit; //==>
  fpgApplication.Lock;   // Enter the critical section
  try
    p := fpgAllocateMessage;
    if p  nil then
    begin
      p^.MsgCode := MsgCode;
      p^.Sender  := Sender;
      p^.Dest    := Dest;
      p^.Params  := aparams;
    end
    else
      DebugLn('THE MESSAGE QUEUE IS FULL.');
  finally
    fpgApplication.Unlock;  // leave the critical section
  end;
end;

Regards,
- Graeme -

_______________________________________________
fpGUI - a cross-platform Free Pascal GUI toolkit
http://opensoft.homeip.net/fpgui/

dmitry boyarintsev
to Graeme Geldenhuys

Your code is safe for multiple threads to post to the queue.

But you also must make sure, that Processing a messsage from queue, would not break Posting safetiness. The easiest way, is to wrap message extraction into the same fpgApplication.Lock

You must also make sure, that no thread can try to post message, after fpgApplicaiton is released. And it's quite possible situation, that causes "strange errors" on application finalization.

Vinzent Höfler
to Graeme Geldenhuys: Some developers prefer to pass in a GUI component to a thread instance, so the thread…
Graeme Geldenhuys wrote 19 lines
Some developers prefer to pass in a GUI component to a thread instance, so the thread knows what widget to update. Other users prefer to post a "gui update" message to the application event queue. I'm very new to threads and might know just enough to make in dangerous. :-) In fpGUI the application's "custom" event queue is a linked-list structure. If threads post messages to that queue, I obviously need to make the queue manipulation code thread safe. fpgApplication is a global singleton of the TfpgApplication class. fpgPostMessage() is a stand-alone function (not a member of TfpgApplication class). To make fpgPostMessage() thread save, I instantiated a TCriticalSection instance in the TfpgApplication class - in the constructor. I added two member functions .Lock and .Unlock which calls FCritSect.Enter and FCritSect.Leave respectively. In fpgPostMessage, I call fpgApplication.Lock, then manipulate the message queue and then call fpgApplication.Unlock. Is this the correct way to make my event queue thread save?

Yes and no. Allocating the message from the queue might be thread safe now, but I strongly suspect that other threads are trying to do something with the queue (like evaluating or removing messages).

Show quoted text 15 lines
fpgApplication.Lock; // Enter the critical section try p := fpgAllocateMessage; if p nil then begin p^.MsgCode := MsgCode; p^.Sender := Sender; p^.Dest := Dest; p^.Params := aparams; end else DebugLn('THE MESSAGE QUEUE IS FULL.'); finally fpgApplication.Unlock; // leave the critical section end;

So this piece of code is probably not enough to make the queue thread safe. IMO it would be best to encapsulate any thread-safety inside the queue class itself, instead of sprinkling "fpgApplication.{Un}Lock" all over the source. (I know, this can be inefficient, but when having the choice between "efficient" or "correct", I'd rather go for "correct".)

Vinzent.

Lukas Gradl
to Graeme Geldenhuys: Some developers prefer to pass in a GUI component to a thread instance, so the thread…
Show quoted text 19 lines
Some developers prefer to pass in a GUI component to a thread instance, so the thread knows what widget to update. Other users prefer to post a "gui update" message to the application event queue. I'm very new to threads and might know just enough to make in dangerous. :-) In fpGUI the application's "custom" event queue is a linked-list structure. If threads post messages to that queue, I obviously need to make the queue manipulation code thread safe. fpgApplication is a global singleton of the TfpgApplication class. fpgPostMessage() is a stand-alone function (not a member of TfpgApplication class). To make fpgPostMessage() thread save, I instantiated a TCriticalSection instance in the TfpgApplication class - in the constructor. I added two member functions .Lock and .Unlock which calls FCritSect.Enter and FCritSect.Leave respectively. In fpgPostMessage, I call fpgApplication.Lock, then manipulate the message queue and then call fpgApplication.Unlock. Is this the correct way to make my event queue thread save?

It is - theoretically. I use threads quite a lot and found it a constant source of problems, to create a deadlock here and there when using the messagequeue or to forget some lock/unlock somewhere. (you will need more than PostMessage for your queue! *bg*)

I prefer to implement the messaging in the object it belongs to - in this case on fpgApplication. The Queue will not be visible from outside, only funcions like PostMessage, GetMessage or DiscardMessage. This way you can make shure, the lock/unlock sequence will be used correctly...

Regards
Lukas
(Just seen that Vincent was a bit faster than me, but with quite the same thougts...)

-------------------------- software security networks Lukas Gradl <fpc#ssn.at> Eduard-Bodem-Gasse 9 A - 6020 Innsbruck Tel: +43-512-214040-0 Fax: +43-512-214040-21 --------------------------
Graeme Geldenhuys
to Lukas Gradl

Thanks to everybody that replied...

Lukas Gradl wrote
I prefer to implement the messaging in the object it belongs to - in this case on fpgApplication. The Queue will not be visible from outside, only funcions like PostMessage, GetMessage or DiscardMessage. This way you can make shure, the lock/unlock sequence will be used correctly...

Yes, this was going to be my next step. I have those functions stand-alone, which I would like to move into the TfpgApplication class with the TCriticalSection. Currently the actual message queue is local to a unit's implementation section, so the only access to it is via the global methods which have the Lock/Unlock code. But yes, encapsulating those into TfpgApplication is definitely my next step.

Regards,
- Graeme -

_______________________________________________
fpGUI - a cross-platform Free Pascal GUI toolkit
http://opensoft.homeip.net/fpgui/