Thread Error: The Handle is Invalid (6) when trying to Free a suspended thread

Question

In a given example I am receiving an exception when calling AThread.Free.

program Project44;

{$APPTYPE CONSOLE}

uses
  SysUtils, Classes, Windows;

type
  TMyException = class(Exception);

var
  AThread: TThread;
begin
  AThread := TThread.Create(True);
  try
    AThread.FreeOnTerminate := True;
    //I want to do some things here before starting the thread
    //During the setup phase some exception might occur, this exception is for simulating purpouses
      raise TMyException.Create('exception');
  except
    AThread.Free; //Another exception here
  end;
end.

I have two questions:

1. How should I free AThread instance of TThread in a given example?

2. I don't understand, why TThread.Destroy is calling Resume before destroing itself. What is the point of this?

Solution 1

You can't set FreeOnTerminate to True and call Free on the thread instance. You have to do one or the other, but not both. As it stands your code destroys the thread twice. You must never destroy an object twice and of course when the destructor runs for the second time, errors occur.

What happens here is that since you created the thread suspended, nothing happens until you explicitly free the thread. When you do that the destructor resumes the thread, waits for it to complete. This then results in Free being called again because you set FreeOnTerminate to True. This second call to Free closes the handle. Then you return to the thread proc and that calls ExitThread. This fails because the thread's handle has been closed.

As Martin points out in the comment you must not create TThread directly since the TThread.Execute method is abstract. Also, you should not use Resume which is deprecated. Use Start to begin execution of a suspended thread.

Personally I don't like to use FreeOnTerminate. Using this feature results in the thread being destroyed on a different thread from which it was created. You typically use it when you want to forget about the instance reference. That then leaves you uncertain as to whether or not the thread has been destroyed when your process terminates, or even whether it is terminating and freeing itself during process termination.

If you must use FreeOnTerminate then you need to make sure that you don't call Free after having set FreeOnTerminate to True. So the obvious solution is to set FreeOnTerminate to True immediately before after calling Start and then forget about the thread instance. If you have any exceptions before you are ready to start then you can safely free the thread then since you FreeOnTerminate would still be False at that point.

Thread := TMyThread.Create(True);
Try
  //initialise thread object
Except
  Thread.Free;
  raise;
End;
Thread.FreeOnTerminate := True;
Thread.Start;
Thread := nil;

A more elegant approach would be to move all the initialisation into the TMyThread constructor. Then the code would look like this:

Thread := TMyThread.Create(True);
Thread.FreeOnTerminate := True;
Thread.Start;
Thread := nil;

Solution 2

The situation is very complicated in your case.

First, you does not actually free a suspended thread; a thread is resumed in destructor:

  begin
    Terminate;
    if FCreateSuspended then
      Resume;
    WaitFor;
  end;

Since Terminate is called before Resume, the Execute method never runs, and thread terminates immediately after being resumed:

  try
    if not Thread.Terminated then
    try
      Thread.Execute;
    except
      Thread.FFatalException := AcquireExceptionObject;
    end;
  finally
    Result := Thread.FReturnValue;
    FreeThread := Thread.FFreeOnTerminate;
    Thread.DoTerminate;
    Thread.FFinished := True;
    SignalSyncEvent;
    if FreeThread then Thread.Free;

Now look at the last line - you call destructor (Thread.Free) from destructor itself! Fantastic bug!

---

To answer your questions:

1. You just can't use FreeOnTerminate:= True in your code;
2. You should ask Embarcadero why TThread is designed so; my guess - some code (DoTerminate method) should be executed in thread context while thread terminates.

---

You can send a feature request to QC: add FFreeOnTerminate:= False to TThread.Destroy implementation:

destructor TThread.Destroy;
begin
  FFreeOnTerminate:= False;
// everything else is the same
  ..
end;

That should prevent recursive desctructor call and make your code valid.

Comments

[Martin James]

I thought about that. You're probably right, but Delphi thread control, especially with termination, has been, (and probably still is), such a mess that I did not dare post it. I looked very briefly at TThread in 'classes' and decided not to look in any further depth in case I found something.

[David Heffernan]

@MartinJames This is exactly what happens. The destructor runs twice. That never ends well.

[Wodzu]

@David If I do not call AThread.Free in this specific example, I am getting a memory leak. So how could I free the thread twice?

[David Heffernan]

@Wodzu Yes you are. FreeOnTerminate := True introduces an implicit call to Free. You then add an extra explicit call to Free.

[Sertac Akyuz]

Equally obviously, can set FreeOnTerminate False in the except handler.

[David Heffernan]

@Sertac Yes that would work too but I think moving code inside the thread constructor is really the best approach here.

[Wodzu]

Thank you Serg, regarding point 1: I belive I can, if I move it just before the Resume() or even immediately after, it will work. But thanks for the answer anyway, I understand the problem better now. +1

[David Heffernan]

After you call Start (please call Start rather than Resume) is not right. The thread might complete and then it will be too late to set FreeOnTerminate. You would then leak the thread and the OS handle.

[TLama]

+1, @David, there's no TThread.Start in D2009 (as the Q is tagged) yet ;) It was since D2010, so TThread.Resume here is correct.