Issue with thread safety

[Scott-online]

First of all, my compliments to you for this excellent piece of code. It is really neat and quite useful.

There is however an issue when using this in a multi-threading UI environment. The critical section locking is not enough to protect user interface controls. As you know, the Delphi handles (UI) messages in one (main) thread. If you update a control from another thread, even when using (your own) locking mechanism, it is not guaranteed that the main thread is not also accessing the same control at the same time. So this would be only safe if MessageBus is the only one to update the UI. And this is, of course, very unlikely because a user is most likely interacting with the UI, hence the U in UI :-)

One possible solution is to use TThread.Synchronize (TThread.Queue) or even SendMessage (PostMessage) in case the sender is calling from another thread instead of the main thread. You might even implement your own message loop, to ensure this also works in non-UI projects.

In short, the best way would be to always run the Subscribe, Unsubscribe and Send code in the main thread. This would also eliminate the need for a locking mechanism.

Also keep in mind that it is not always safe to create an object (graphic, files access, communication etc) in one thread and destroying it in another thread. Currently MessageBus does not protect you from doing that.

Your thoughts on this?

Regards,
Scott

[THSoftPoland]

Hi Scott - yes, you absolutely right.

I prefer SendMessage, because Microsoft guarantee synchronization of threads. With PostMessage you cant`t uses aValue, because aValue can be just destroyet.
TThread.Synchronize is also good solution, but TThread.Queue is bad soultion (is like PostMessage, aValue can be just destroyet).

But with this methods (SendMessage, TThread.Synchronize), your MessageBus is waiting for executing code in MainThread context. To avoid this you can clone Data and send to MainThread, without blocking MessageBus, other receipients can do they work:

TMessageBus.Default.Subscribe(MSG_TEST1,
procedure(const aValue: String; aEnv: TMessageBus.TBusEnvir) // simple data or your Data can be cloned
var
aInternalData: String; // or your TObjectData
begin
aInternalData := aValue.Clone; // to obtain a copy of aValue !!!!!!!!!
TThread.CreateAnonymousThread( // totaly async and independent from MessageBus
procedure
begin
// nothing to do with this thread but:
// SendMessage(MainWindow.Handle, WM_YOURMESSAGE, WPARAM(aInternalData), 0); aInternalData.Free; or
TThread.Synchronize(nil, // Synchronize work ONLY in Main Thread (= VCL)
procedure
begin
label1.Caption := aInternalData;
// aInternalData.Free;
end;
end);
end);

// example of cloned Object:

TObjectData = class
  private
    fData: String;
  public
    property Data: string read fData;
    function Clone: TObjectData;      
end;

function TObjectData.Clone: TObjectData;
begin
  Result := TObjectData.Create;
  Result.fData := self.fData;
end;

Regards
THsoftPoland

PS: read below

In short, the best way would be to always run the Subscribe, Unsubscribe and Send code in the main thread. This would also eliminate the need for a locking mechanism.

Yes - this eliminate locking

Also keep in mind that it is not always safe to create an object (graphic, files access, communication etc) in one thread and destroying it in another thread. Currently MessageBus does not protect you from doing that.

Yes - you right. But Data is created and destroyed always in ONE thread (look for example Send and next is always Data.Free). can be used in other threads - see the code at the top of mail.