A proper way of destroying a TThread object

Question

This question may seem trivial, but I hope you won't ignore it.
Before destroying a TThread object it is usually necessary to wait until the thread that called the TThread.Execute() method finishes, for only then can we be sure that, for instance, the objects destroyed inside the class's destructor are no longer accessed. Therefore it is necessary to call Terminate to set the Terminated flag that the thread has to check to know whether to exit or not, and then call the WaitFor() method.

Because the thread may be suspended, I think it is good to resume it before calling WaitFor, as otherwise the calling thread would be deadlocked. And because the thread can be suspended multiple times, it should be resumed the same number of times, right?

while Suspended do
  Resume;

If the thread was created suspended, we do not have to worry that the TThread.Execute() method will be called when we resume the thread only to terminate it - it won't (please correct me if I'm wrong).

What I've stated suggests using the following lines of code for each TThread object being freed:

MyThread.Terminate;
while MyThread.Suspended do
  MyThread.Resume;
MyThread.WaitFor;
MyThread.Free;

Unfortunately, when we destroy our application that has created multiple threads, writing such a piece of code for each TThread object being destroyed unnecessarily makes the code very long and maybe even opaque.

Therefore I came to a conclusion that all these could be put inside an overriden destructor of the TThread class thanks to which it would be enough to call MyThread.Free (or MyThread.Terminate if MyThread.FreeOnTerminate is set) without caring about whether the destroyed object is a TThread object or not:

destructor TMyThread.Destroy;
begin
  //if FreeOnTerminate, the calling thread cannot wait for itself
  if GetCurrentThreadId <> ThreadId then
  begin
    Terminate;
    while Suspended do
      Resume;
    WaitFor;
  end;

  {free all objects created in this class}

  inherited Destroy;
end;

Forgive me asking such a basic question. I would like, however, to get to know your opinions about this way - I hope an universal way - of destroying TThread objects. I ask this questions for I learned from my workmates' codes that they usually used the first example of code to destroy such objects, but they never used to check whether the threads being waited for were not suspended which I considered a bit dangerous if the threads might be suspended somewhere in the code. Therefore I tried to find a universal way of destroying the objects of this class that would make the code clearer and safer. I hope I didn't make it worse - what do you think?

Thanks for your suggestions in advance.

Answer 1

Much of what your suggesting is already performed in the TThread.Destroy destructor, and invoking TMyThread.free will do just what your suggesting. To cleanup any objects owned by the thread class, you can perform that in the OnTerminate event, which will get invoked as part of the thread shutdown logic.

Answer 2

There is no universal way to stop a thread, just as there is no universal way to (gracefully) stop a process. Each one is different.

For some threads, it's sufficient to set its Terminated property via the Terminate method. Other threads, however, call functions like GetMessage or MsgWaitForMultipleObjects, which will block until something happens, such as a message arriving or a kernel handle becoming signaled. TThread.Terminate can't make either of those things happen, so it can't make those threads stop running. When I've written threads like those, I've provided my own functions for notifying them to stop running. I might call PostThreadMessage to force a message onto the thread's queue, or I might signal the event that the thread class has provided for notifying it of a request to terminate.

Don't worry about resuming a suspended thread. You really shouldn't be suspending them anyway. The only safe way to suspend a thread is for the thread to suspend itself, and once you have that, you're guaranteed to have at least two threads controlling when the thread runs: the thread itself to suspend it, and at least one other thread to resume it again. A thread should be in control of its own execution.

It would be great if TThread.Terminate were virtual. Then each thread class could provide a custom way to notify itself that it should stop running. Some could just set Terminated, and others could post themselves messages, signal events, or do whatever else they need. As it is, though, the non-virtual method doesn't work well with threads that spend a lot of their time waiting for other things. The current way only works for threads that are able to frequently poll their Terminated properties.

Some threads have their FreeOnTerminate properties set. For those threads, your code is not safe. Technically, it's not safe to call any methods on such objects since the thread could terminate at any time. But even if you know the thread is still running and the thread object still exists, the object will definitely stop existing sometime after the call to Terminate. You can't call WaitFor on a free-on-terminate thread object, and you definitely can't call Free.