Free a TThread either automatically or manually

You can simplify this to:

procedure TMyThread.Execute;begin  // ... Do some processingend;procedure TMainForm.Create;begin  MyThread := TMyThread.Create(false);end;procedure TMainForm.Close;begin  if Assigned(MyThread) then    MyThread.Terminate;  MyThread.Free;end;

Explanation:

• Either use FreeOnTerminate or free the thread manually, but never do both. The asynchronous nature of the thread execution means that you run a risk of not freeing the thread or (much worse) doing it twice. There is no risk in keeping the thread object around after it has finished the execution, and there is no risk in calling Terminate() on a thread that has already finished either.

• There is no need to synchronize access to a boolean that is only written from one thread and read from another. In the worst case you get the wrong value, but due to the asynchronous execution that is a spurious effect anyway. Synchronization is only necessary for data that can not be read or written to atomically. And if you need to synchronize, don't use Synchronize() for it.

• There is no need to have a variable similar to MyThreadReady, as you can use WaitForSingleObject() to interrogate the state of a thread. Pass MyThread.Handle as the first and 0 as the second parameter to it, and check whether the result is WAIT_OBJECT_0 - if so your thread has finished execution.

BTW: Don't use the OnClose event, use OnDestroy instead. The former isn't necessarily called, in which case your thread would maybe continue to run and keep your process alive.

Have the main thread assign a handler to the worker thread's OnTerminate event. If the worker thread finishes first, then the handler can signal the main thread to free the thread. If the main thread finishes first, it can terminate the worker thread. For example:

procedure TMyThread.Execute;begin  ... Do some processing ...end;procedure TMainForm.Create;begin  MyThread := TMyThread.Create(True);  MyThread.OnTerminate := ThreadFinished;  MyThread.Resume; // or MyThread.Start; in D2010+end;const  APPWM_FREE_THREAD = WM_APP+1;procedure TMainForm.ThreadFinished(Sender: TObject);begin  PostMessage(Handle, APPWM_FREE_THREAD, 0, 0);end;procedure TMainForm.WndProc(var Message: TMessage);begin  if Message.Msg = APPWM_FREE_THREAD then    StopWorkerThread  else    inherited;end;procedure TMainForm.StopWorkerThread;begin  if MyThread <> nil then  begin    MyThread.Terminate;    MyThread.WaitFor;    FreeAndNil(MyThread);  end;end;procedure TMainForm.Close;begin  StopWorkerThread;end;

No, your code is not good (though it probably will work in 99.99% or even 100% cases). If you are planning to terminate work thread from main thread, don't set FreeOnTerminate to True (I don't see what are you trying to gain in the above code by setting FreeOnTerminate to True, it at least makes your code less understandable).

A more important situation with terminating work threads is that you are trying to close an application while work thread is in wait state. The thread will not be awaken if you just call Terminate, generally you should use additional syncronization object (usually event) to wake up the work thread.

And one more remark - there is no need for

  begin    MyThread.Terminate;    MyThread.WaitFor;    MyThread.Free;  end;

if you look at TThread.Destroy code, it calls Terminate and WaitFor, so

    MyThread.Free;

is enough (at least in Delphi 2009, have no Delphi 7 sources at hand to check).

Updated

Read mghie answer. Consider the following situation (better on 1 CPU system):

main thread is executing

procedure TMainForm.Close;begin  if not MyThreadReady then  begin    MyThread.Terminate;    MyThread.WaitFor;    MyThread.Free;  end;end;

it checked MyThreadReady value (it is False) and was switched off by scheduler.

Now scheduler switches to work thread; it executes

  Synchronize(ThreadFinished);

and forces scheduler to switch back to main thread. Main thread continues execution:

    MyThread.Terminate;   // no problem    MyThread.WaitFor;     // ???    MyThread.Free;

can you say what will happen at WaitFor? I can't (requires a deeper look into TThread sources to answer, but at first glance looks like a deadlock).

Your real error is something different - you have written an unreliable code and trying to find out is it correct or not. That is bad practice with threads - you should learn to write a reliable code instead.

As for resources - when the TThread (with FreeOnTerminate = False) is terminated the only resources that remains allocated is Windows thread handle (it does not use substantial Windows resources after thread is terminated) and Delphi TThread object in memory. Not a big cost to be on the safe side.