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;
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?
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.