How do I terminate a thread that doesn't have an Execute method ?

[Rollo62 502]

3 minutes ago, Dalija Prasnikar said:

If you know absolutely nothing about threads, then Sleep can be a first step towards solution. It will at least give you a simple proof of concept code. If you need to learn everything at once it can be overwhelming.

Yes, but why hiding TEvent information, when this was made especially suited for Threads, not the Sleep method.
This might avoid that someone, who is new to threads, that he is stepping into the wrong direction from the first step.

I personally like to lay all info on the table, with all pro's and con's, where the user can decide well on his own.

I think, to refine a wrong, too early decision later on can be a lot harder than necessary.

[Dalija Prasnikar 1341]

12 minutes ago, Rollo62 said:

I think, to refine a wrong, too early decision later on can be a lot harder than necessary.

Depends on the decision. Replacing Sleep with event later on is trivial. 

 

In this case we don't even know whether we need to have a thread that will sleep or wait on an even in the first place. This is the "wrong direction" I am talking about.

[Rollo62 502]

17 minutes ago, Dalija Prasnikar said:

Depends on the decision. Replacing Sleep with event later on is trivial.

That might be the case, but I see another typical problem on the horizon.
I call it the "case solved" problem, where one might think a case is solved once for all, never considering that there may lie any hidden problems inside.
Especially with threads, this might lead to long and painful problem searches, if you are too sure about your initial solution.

 

But you'right, depends on the case, which we don't really have seen right now.

 

[Remy Lebeau 1260]

8 hours ago, dormky said:

Well, it seems that no matter what I do, the timer's callback is always executed in the main thread.

That means you are creating the TTimer in the main thread to begin with, not in the background thread.

8 hours ago, dormky said:

Anyone have a recommendation for a TTimer equivalent that has its callback executed in the thread it was created in ?

That's exactly how TTimer already works.  Its constructor creates a hidden window in the calling thread, which when activated will then receive timer messages from that same thread.  So the thread that creates the TTimer must have a message loop to receive and dispatch those timer messages.  So, you might think about moving the creation of the TTimer into the background thread's Execute() method, and that may work 99% of the time, but know that the creation of that hidden window is not done in a thread-safe manner, so you really should not be using TTimer in a background thread at all.

 

If you really need a thread-based timer, you could just use the Win32 timeSetEvent() function, which is a multimedia timer that runs its callback in a background thread that the OS manages,

 

If you really want to use a custom TThread class, then using TEvent is the simplest option that allows you to terminate the thread on demand, eg:

uses
  ..., Classes, SyncObjs;

type
  TTimerThread = class (TThread)
  private
    fEvent: TEvent;
    fInterval: LongWord;
    fOnElapsed: TNotifyEvent;
  protected
    procedure Execute; override;
    procedure TerminatedSet; override;
  public
    constructor Create(AInterval: LongWord; AOnElapsed: TNotifyEvent); reintroduce;
    destructor Destroy;
  end;
  
constructor TTimerThread.Create(AInterval: LongWord; AOnTimer: TNotifyEvent);
begin
  inherited Create(False);
  fInterval := AInterval;
  fOnElapsed := AOnElapsed;
  fEvent := TEvent.Create;
end;

destructor TTimerThread.Destroy;
begin
  fEvent.Free;
  inherited Destroy;
end;

procedure TTimerThread.TerminatedSet;
begin
  fEvent.SetEvent;
end;

procedure TTimerThread.Execute;
begin
  while fEvent.WaitFor(fInterval) = wrTimeout do
    fOnElapsed(Self);
end;

 

Edited January 15 by Remy Lebeau

[Dalija Prasnikar 1341]

@Remy Lebeau fEvent should be released after inherited destructor runs as thread might still be running at that time if thread is not waited for before calling Free (which is not common pattern in Delphi code) 

 

destructor TTimerThread.Destroy; 
begin 
  inherited Destroy; 
  fEvent.Free; 
end;

 

[Kas Ob. 91]

12 hours ago, Dalija Prasnikar said:

@Remy Lebeau fEvent should be released after inherited destructor runs as thread might still be running at that time if thread is not waited for before calling Free (which is not common pattern in Delphi code) 

If TThread destructor being called while the thread is running then it is best to make sure the event handled closed and triggered (aka freed), preventing the thread from blocking on it, other wise it will go performing undefined behavior.

[Dalija Prasnikar 1341]

4 minutes ago, Kas Ob. said:

If TThread destructor being called while the thread is running

Default TThread destructor will wait for thread to finish, so there will be no undefined behavior. That is why this inherited destructor needs to run first. Otherwise, you must always wait for a thread before you call Free on it. Because it is easy to forget calling WaitFor before calling Free every time you use a thread somewhere, it is much safer to call the inherited destructor first, because that is something you need to write only once.

[Kas Ob. 91]

1 minute ago, Dalija Prasnikar said:

Default TThread destructor will wait for thread to finish, so there will be no undefined behavior. That is why this inherited destructor needs to run first. Otherwise, you must always wait for a thread before you call Free on it. Because it is easy to forget calling WaitFor before calling Free every time you use a thread somewhere, it is much safer to call the inherited destructor first, because that is something you need to write only once.

Well, yes, but waiting on event that will not be triggered because the thread is trying to call the inherited destructor, for me is undefined behavior, will if finish or will it not and when ?

Because the event is not triggered nor freed, TThread.Free in my opinion should never ever called, all and every TThread by design should clean up after themselves, well that is my humble opinion, the whole point of leaving a class/creature meant to run only once such as thread which can effecting the whole process, should be considered as falling knife, draw the path/loop or what ever then let it run its course cleaning and exiting gracefully.

[Dalija Prasnikar 1341]

3 minutes ago, Kas Ob. said:

Well, yes, but waiting on event that will not be triggered because the thread is trying to call the inherited destructor, for me is undefined behavior, will if finish or will it not and when ?

Event will be triggered when TerminatedSet is called, which will be called as part of the thread cleanup process triggered by inherited destructor. There is no undefined behavior. Thread will finish, there was only a possible race condition where fEvent could have been released while background thread was still using it.

 

[Kas Ob. 91]

5 minutes ago, Dalija Prasnikar said:

there was only a possible race condition where fEvent could have been released while background thread was still using it.

Undefined behavior ?!!

[Dalija Prasnikar 1341]

9 minutes ago, Kas Ob. said:

TThread.Free in my opinion should never ever called, all and every TThread by design should clean up after themselves, well that is my humble opinion, the whole point of leaving a class/creature meant to run only once such as thread which can effecting the whole process, should be considered as falling knife, draw the path/loop or what ever then let it run its course cleaning and exiting gracefully.

Self destructing threads, while useful, have their own issues. They cannot be terminated directly, waited for, or properly cleaned up during the application shutdown and can cause various issues and exceptions during the shutdown. Basically, you lose proper application cleanup. 

 

Actually, you can do proper cleanup with such threads, too, but only if you add plenty of code that will also have to fiddle around with Windows API, and when doing all that, you are effectively rewriting infrastructure that is already in place inside TThread class. And then you are back to the square one, where you need to handle thread cleanup in proper order, basically achieving nothing comparing to using thread class that you need to manually Free - which again will work properly, if you pay little attention.

[Dalija Prasnikar 1341]

4 minutes ago, Kas Ob. said:

Undefined behavior ?!!

I said "was possible race condition", which is fixed if you call inherited thread destructor before you call fEvent.Free.

[Kas Ob. 91]

4 minutes ago, Dalija Prasnikar said:

elf destructing threads, while useful, have their own issues. They cannot be terminated directly, waited for, or properly cleaned up during the application shutdown and can cause various issues and exceptions during the shutdown. Basically, you lose proper application cleanup. 

 

Actually, you can do proper cleanup with such threads, too, but only if you add plenty of code that will also have to fiddle around with Windows API, and when doing all that, you are effectively rewriting infrastructure that is already in place inside TThread class. And then you are back to the square one, where you need to handle thread cleanup in proper order, basically achieving nothing comparing to using thread class that you need to manually Free - which again will work properly, if you pay little attention.

I agree on that, but about the part of plenty of code, well, that part is relative

for example, the thread in question above should have TerminatedSet called in the destructor before the inherited Destroy this will make sure the thread is not blocking on an event that might not be triggered or will take long time to trigger, this is equal or the same as freeing the event (before the the inherited) and forget about it.

[Dalija Prasnikar 1341]

3 minutes ago, Kas Ob. said:

for example, the thread in question above should have TerminatedSet called in the destructor before the inherited Destroy this will make sure the thread is not blocking on an event that might not be triggered or will take long time to trigger, this is equal or the same as freeing the event (before the the inherited) and forget about it.

The TThread destructor will call all that. I suggest you look at the source code. After that inherited destructor call is finished, the background thread will no longer run and it will be safe to call fEvent.Free. 

[Kas Ob. 91]

5 minutes ago, Dalija Prasnikar said:

After that inherited destructor call is finished, the background thread will no longer run and it will be safe to call fEvent.Free. 

Dalija , may be you are low on coffee but for sure i am ! 

Thread.Free is a blocking call and will not return until the Thread.Execute is finished and exited, so if an event is still holding the thread execution then fEvent.Free will not be reached until it is set/triggered, hence the the need to either trigger the event in the destructor by destroying/freeing it or trigger it explicitly before the inherited destructor then freeing it.

[Dalija Prasnikar 1341]

4 minutes ago, Kas Ob. said:

Thread.Execute is finished and exited,

Which will happen during the cleanup in TThread destructor because of fEven.SetEvent which is called in TerminatedSet. Please read everything I posted before. If you don't trust me, then run the code yourself. We are running in circles now.

[Remy Lebeau 1260]

15 minutes ago, Kas Ob. said:

Thread.Free is a blocking call and will not return until the Thread.Execute is finished and exited, so if an event is still holding the thread execution then fEvent.Free will not be reached until it is set/triggered, hence the the need to either trigger the event in the destructor by destroying/freeing it or trigger it explicitly before the inherited destructor then freeing it.

The event is signaled when TerminatedSet() is called. TerminatedSet() is called by the Terminated property setter. If the thread is still running when the TThread object is being destroyed, the inherited destructor will set the Terminated property and wait for the thread to finish running. 

Edited January 16 by Remy Lebeau