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?

Kikuyu

First, are you getting any errors/warnings for this? TThread.Execute is abstract in D2009. IME, you should get a warning about constructing instances with abstract methods. Normally, TThread.Execute is overridden in a TThread descendant class and it's that descendant that is instantiated. I've never tried to create an instance of TThread directly - I'm fairly sure that some exception would be raised on the constructing thread, the constructed thread or both. – Siblee

You could wait to set FreeOnTerminate until just before you call Resume. – Cindycine

I guess destroy calls resume because if a thread was suspended it can't be destroyed properly in such a state. – Softboiled

@ldsandon TThread.Destroy calls Terminate, Resume and then WaitFor. – Relativize

@MartinJames yes, I am getting a warning, but since TThread.Resume is never called in this example it is not a problem. The same exception occurs when I am creating a proper descendant class of TThread. Good point, but the error has nothing to do with an instance of TThread. – Kikuyu

I'm with @Marcus Adams. do "some things" and then use FreeOnTerminate, Resume. – Comprise

@Kikuyu Thread.Resume is indeed called. It happens when you call AThread.Free. – Relativize

@MarcusAdams yes, that is a nice workaround, thanks. – Kikuyu

Creating the thread suspended is begging for trouble. If you do the setup phase prior to creating the thread and later create it non-suspended, all problems are gone. The FreeOnTerminate can be set in your TThread.Create constructor. – Highminded

@LURD The easy way to achieve that is simply to move all the setup into the constructor. – Relativize

@DavidHeffernan, yes that's a good option. Sometimes it can break what you want to be visible in the thread unit, though. – Highminded

Just had a problem with FreeOnTerminate being set in the thread constructor. The problem is that if an exception occurs in the constructor and FreeOnTerminate has been set, the destructor gets called twice (once because the constructor failed and apparently also once because FreeOnTerminate was true). – Hendrik

Answer 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;

Relativize

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. – Siblee

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

@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? – Kikuyu

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

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

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

Answer 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.

Diadiabase

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 – Kikuyu

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. – Relativize

+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. – Millstone