12

The app is written in Delphi XE.

I have two classes, a TBoss and TWorker, which are both based of of TThread. The TBoss is a single instance thread, which starts up and then will create about 20 TWorker threads.

When the boss creates a instance of TWorker it assigns it a method to call synchronize on, when the Worker has finished with what it's doing it calls this method which allows the Boss to access a record on the Worker.

However I feel this is a problem, calling synchronize appears to be locking up the whole application - blocking the main (ui) thread. Really it should just be synchronizing that worker to the boss thread....

Previously I used messages/packed records to send content between threads which worked well. However doing it this way is much cleaner and nicer.... just very blocking.

Is there a way to call Syncronize in the worker to only wait for the Boss thread?

My code:

    type 
      TWorker = class(TThread) 
      private 
        fResult : TResultRecord;
        procedure SetOnSendResult(const Value: TNotifyEvent);
        ....
        ....
      public
        property OnSendResult: TNotifyEvent write SetOnSendResult; 
        property Result : TResultRecord read fResult;
        ....
     end;

    ...
    ...
    procedure TWorker.SendBossResults; 
    begin 
      if (Terminated = False) then 
      begin 
        Synchronize(SendResult); 
      end; 
    end; 

    procedure TWorker.SendResult; 
    begin 
      if (Terminated = false) and Assigned(FOnSendResult) then 
      begin 
        FOnSendResult(Self); 
      end; 
    end;

Then in my Boss thread I will do something like this

    var 
      Worker  : TWorker; 
    begin 
      Worker              := TWorker.Create; 
      Worker.OnTerminate  := OnWorkerThreadTerminate; 
      Worker.OnSendResult := ProcessWorkerResults;

So my boss then has a method called ProcessWorkerResults - this is what gets run on the Synchronize(SendResult); of the worker.

    procedure TBoss.ProcessWorkerResults(Sender: TObject); 
    begin 
      if terminated = false then 
      begin 
        If TWorker(Sender).Result.HasRecord then
        begin
          fResults.Add(TWorker(Sender).Result.Items);
        end;
      end; 
    end;
bummi
  • 27,123
  • 14
  • 62
  • 101
Wizzard
  • 12,582
  • 22
  • 68
  • 101
  • 1
    which Delphi version is it (there are some new options since 2009)? – mjn Apr 08 '11 at 20:49
  • 1
    I would argue that using messages to pass data between threads, which requires no "application" synchronisation mechanism (it is in the threading "framework"), is much more elegant than using synchronisation primitives around shared data. – Misha Feb 10 '12 at 06:00

5 Answers5

10

Synchronize is specifically designed to execute code in the main thread; that's why it seems to lock everything up.

You can use several ways to communicate from the worker threads to the boss thread:

  • Add a callback to each worker thread, and assign it from the boss thread when it's created. It can pass back whatever as parameters, along with a thread ID or some other identifier.

  • Post a message from the worker thread to the boss thread using PostThreadMessage. The disadvantage here is that the boss thread has to have a window handle (see Classes.AllocateHWnd in the Delphi help and David Heffernan's comment below).

  • Use a good quality third-party threading library. See OmniThreadLibrary - it's free, OS, and extremely well written.

My choice would be the third. Primoz has done all the hard work for you. :)

After your comment, here's something along the lines of my first suggestion. Note that this is untested, since writing the code for a TBoss and TWorker thread + a test app is a little long for the time I have right this minute... It should be enough to give you the gist, I hope.

type 
  TWorker = class(TThread) 
  private 
    fResult : TResultRecord;
    fListIndex: Integer;
    procedure SetOnSendResult(const Value: TNotifyEvent);
    ....
    ....
  public
    property OnSendResult: TNotifyEvent write SetOnSendResult; 
    property Result : TResultRecord read fResult;
    property ListIndex: Integer read FListIndex write FListIndex;
    ....
  end;

type 
  TBoss=class(TThread)
  private
    FWorkerList: TThreadList; // Create in TBoss.Create, free in TBoss.Free
    ...
  end;

procedure TWorker.SendBossResults; 
begin 
  if not Terminated then
    SendResult; 
end;

procedure TBoss.ProcessWorkerResults(Sender: TObject); 
var
  i: Integer;
begin 
  if not terminated then 
  begin 
    If TWorker(Sender).Result.HasRecord then
    begin
      FWorkerList.LockList;
      try
        i := TWorker(Sender).ListIndex;
        // Update the appropriate record in the WorkerList
        TResultRecord(FWorkerList[i]).Whatever...
      finally
        FWorkerList.UnlockList;
      end;
    end;
  end; 
end;
Ken White
  • 123,280
  • 14
  • 225
  • 444
  • Take care with Classes.AllocateHWnd which is not thread-safe. You need to serialise calls to AllocateHWnd and DeallocateHWnd. I have no idea why the VCL doesn't do this and in my code I hook these routines to achieve thread-safety. – David Heffernan Apr 08 '11 at 22:07
  • @David: I knew that. :( I should have specifically mentioned it as another disadvantage. Thanks for the correction. – Ken White Apr 08 '11 at 22:29
  • Hi there, yea it seems that is what is causing my issue. However your first solution is "Add a callback to each worker thread" which is basically what I have done, so how does it differ? – Wizzard Apr 08 '11 at 22:30
  • You didn't add a callback - you added a method to be synchronize'd. There's a difference. A callback is similar to any event handler in Delphi (for instance, TNotifyEvent, which is called during a TButton.OnClick, a TForm.FormCreate, etc.). You can define your own method prototype to pass any params you want from the worker to the boss. – Ken White Apr 08 '11 at 22:32
  • Thanks, if you look at the code I posted my method was a TNotifyEvent - so would it just be a matter of removing the Synchronize call? So I wonder this might be the shortist route in fixing the issue. The third option looks pretty good to, but the first might be simplest... Do you know of a site/example code for this? – Wizzard Apr 08 '11 at 22:36
  • Thanks Ken. The biggest thing different is what you've done in the SendBossResults method - not called Synchronize. Everything else (except the ThreadList) is about the same... method type (call back) looks like the same and so would assignment correct? – Wizzard Apr 09 '11 at 00:45
  • 1
    @Wizzard: Yep. The only thing you have to worry about is TBoss getting multiple calls from different threads at the same time - that's why the move to TThreadList. You can also use other methods, for instance a TCriticalSection. See Delphi's `SyncObjs` unit for other possibilities. – Ken White Apr 09 '11 at 01:09
  • Ah ha!! Thats it... So I thought that it was thread safe / threads would queue to call that method but they wont you could have many threads using that callback at the same time which is why you used the TThreadList. I went with the critical section (bit more obvious whats happening...) looking back at the svn logs, the syncronize was added to the worker method because there was many access violations. Add this stopped that, but grinded the app down. Well done! :) – Wizzard Apr 09 '11 at 02:01
  • 3
    Please correct your answer - a thread does most certainly **not** need a window handle to be able to receive Windows messages. The API documentation you linked to says as much, the message is posted to a thread identified by id, not to a window handle, and the thread needs a message queue and has to process queued messages, but that's it. – mghie Apr 09 '11 at 07:10
8

You could use a thread safe queue. In DelphiXE there is the TThreadedQueue. If you don't have DXE, try OmniThreadLibray - this library is very good for all threading issues.

Eran
  • 387,369
  • 54
  • 702
  • 768
Daniele Teti
  • 1,764
  • 14
  • 20
  • The TThreadedQueue has issues in a multithreaded environment with either many consumers or many producers. Use a more lightweight queue instead. See [link](http://www.pascalgamedevelopment.com/showthread.php?4961-freepascal-Delphi-thread-safe-queue) – LU RD Apr 08 '11 at 22:27
  • I'm not sure how this answers the question "Is there a way to call Syncronize in the worker to only wait for the Boss thread?" - Can you explain? – Ken White Apr 09 '11 at 00:35
1

As I mentioned new options in Delphi 2009 and higher, here is a link to an example for Producer / Consumer communication between threads, based on the new objct locks, in my blog:

Thread Synchronization with Guarded Blocks in Delphi

In a note regarding the deprecated methods TThread.Suspend and TThread.Resume, The Embarcadero DocWiki for Delphi recommends that “thread synchronization techniques should be based on SyncObjs.TEvent and SyncObjs.TMutex.“ There is, however, another synchronization class available since Delphi 2009: TMonitor. It uses the object lock which has been introduced in this version ...

mjn
  • 36,362
  • 28
  • 176
  • 378
0

Solved!! (answer taken from the question)
The fixes made for this problem where two fold.
First remove the syncronization call in the TWorker SendBossResult method.

Second add a fProcessWorkerResult CritialSection to TBoss class. Create and Free this in create/destroy of the TBoss. In the ProcessWorkerResults method call fProcessWorkerResult.Enter and fProcessWorkerResult.leave around the code which needs to be safe from multiple worker results streaming in.

The above was the conclusion after Kens code and follow up comment. Many thanks kind sir, hats off to you!.

bummi
  • 27,123
  • 14
  • 62
  • 101
0

public properties of the TWorker class MUST have get and set methods, so you can use a Tcriticalsection to give the values of the properties. Otherwise, you´d be having thread-safe issues. Your example seems ok, but in the real world, with thousands of threads accessing to the same value would result in an read error. Use critical sections.. and you wouldn´t have to use any Synchronize. This way you avoid going to the message queues of windows and improve performance. Besides, if you use this code in a windows service app, (where windows messages aren´t allowed), this example wouldn´t work. The synchronize method doesn´t work unless there´s access to the windows message queue.

Elmo
  • 6,409
  • 16
  • 72
  • 140
Emi
  • 9
  • 1