0

I have a thread class to do some work then post a message to the main app thread and self terminate. What's the correct way to do this, do I need an OnTerminate procedure or is the Destructor enough?

(The threads List is a bit irrelevant to the question, I want to use it to check how many threads are still working)

FThread := TPicBuilder.Create(PicObject);
FThread.Resume;

.

Type TPicBuilder = class(TThread)
  protected
    procedure Execute; override;
  private
    MyPic: TBitmap32;
    procedure AddToPool;
    procedure RemoveFromPool;
    procedure MyTerminateHandler(Sender: TObject);
  public
    Constructor Create(Pic: Pointer);
    Destructor Destroy; override;
end;

Var PicThreads: TObjectList<TPicBuilder>;

implementation

Constructor TPicBuilder.Create(Pic: Pointer);
begin
  inherited Create(False);
  Self.FreeOnTerminate := True;
  Self.OnTerminate := MyTerminateHandler;
  MyPic:= TBitmap32(Pic);
  Synchronize(AddToPool);
end;

Destructor TPicBuilder.Destroy;
begin
  inherited Destroy;
  Synchronize(RemoveFromPool);
  // PostMessage(Application.Handle, WM_APicBuilt, 0 {some tag goes here with a pic ID}, 0); // should I call this here instead?
end;

procedure TPicBuilder.AddToPool;
begin
  PicThreads.Add(Self);
end;

procedure TPicBuilder.RemoveFromPool;
begin
  PicThreads.Remove(Self);
end;

procedure TPicBuilder.MyTerminateHandler(Sender: TObject);
begin
  PostMessage(Application.Handle, WM_APicBuilt, 0, 0);
end;

Procedure TPicBuilder.Execute;
begin
  // thread work
end;

Initialization
  PicThreads := TObjectList<TPicBuilder>.Create(False);

Finalization
  PicThreads.Free;

end.
hikari
  • 3,393
  • 1
  • 33
  • 72
  • 1
    The destructor will be called automatically since you are using `FreeOnTerminate=True`. Obviously, you do need to destroy what you create, whether manually or not. The `OnTerminate` handler is called automatically after `Execute` exits, before the destructor is called. You could avoid the synchronize used for `OnTerminate` by overriding `TThread.DoTerminate()` instead. So, what is the actual question? – Remy Lebeau Aug 11 '20 at 15:21
  • The question is whether I can omit the OnTerminate event and rely on just the Destructor (what's the purpose of the event if it's not needed?) – hikari Aug 11 '20 at 17:52
  • 1
    It would make more sense to me to PostMessage at the end of `TPicBuilder.Execute`. An object assigning its own event is... Unorthodox. Events are meant for the callers of your object. As Remy stated, you'd be better off overriding `TThread.DoTerminate()`. Also, `inherited Destroy;`should probably be at the end of the destructor. – Ken Bourassa Aug 11 '20 at 18:20
  • 1
    The destructor runs in whichever thread calls it, which for `FreeOnTerminate=true` means the worker thread itself. `OnTerminate` is always called in the main UI thread. I would not post a message at the end of `Execute()`, in case it exits due to exception. Better to override `DoTerminate()` instead, which is always called regardless of how `Execute()` exits (the inherited `DoTerminate()` calls `OnTerminate` via `Synchronize()`). – Remy Lebeau Aug 11 '20 at 20:12
  • I'm a bit confused with the termination flow, according to these comments: 1) Constructor (main thread) 2) Execute (worker thread) 3) OnTerminate event (main thread, could be any proc outside the thread object, which blocks the final thread termination until exit) 4) finally Destructor (worker thread due to self terminate). Is this correct?. Regarding avoiding the Syncronization, am I wrong in using it to add/remove the thread object from the List? – hikari Aug 11 '20 at 20:27
  • Also, using synchronise to let the main thread add/remove items to you ‘pool’ is not advisable. You would be better of using a sync object to guard the modification of the list. – R. Hoek Aug 22 '20 at 21:34

0 Answers0