Delphi TThread handle error

Question

I am reading "Delphi High performance" and there is something that I am missing. Given this code as test:

type TTest = class(TThread)
  private
    amemo: TMemo;
  public
    constructor Create(ss: boolean; memo: TMemo);
  protected
    procedure Execute; override;
end;

constructor TTest.Create(ss: boolean; memo: TMemo);
begin
  inherited Create(ss);
  FreeOnTerminate := true;
  amemo := memo;
end;

procedure TTest.Execute;
var i: uint32;
begin
  inherited;
  i := 0;

  while not Terminated do
    begin
      Inc(i);
      Synchronize(procedure
                  begin amemo.Lines.Add(i.ToString) end);
      Sleep(1000);
    end;

end;

Very simply, this thread prints some numbers in a memo. I start the thread suspended and so I have to call this piece of code:

procedure TForm1.Button1Click(Sender: TObject);
begin
  thread := TTest.Create(true, Memo1);
  thread.Start;
end;

I have always stopped the thread calling thread.Terminate; but reading the book I see that Primoz stops a thread like this:

procedure TForm1.Button2Click(Sender: TObject);
begin
  thread.Terminate;
  thread.WaitFor; //he adds this method call
  //FreeAndNil(thread)
  //there is the above line as well in the code copied from the book but I have removed it since I have set FreeOnTerminate := true (so I dont have to worry about freeing the obj).
end;

At this point, if I run the code using only Terminate I have no problems. If I run the code with Terminate + WaitFor I get this error:

I have read more coding in delphi too and I see that Nick Hodges just makes a call to Terminate;. Is calling Terminate; enough to safey stop a thread? Note that I've set FreeOnTerminate := true so I don't care about the death of the object. Terminated should stop the execution (what is inside execute) and so it should be like this:

Call Terminated
Execute stops
Thread stops execution
Thread is now free (FreeOnTerminate := true)

Please tell me what I'm missing.

Note. In the book the thread doesn't have FreeOnTerminate := true. So the thread needs to be freed manually; I guess that this is the reason why he calls

thread.Terminate;
thread.WaitFor;
FreeAndNil(thread)

I agree on Terminate (stop the thread= and FreeAndNil (free the object manually) but the WaitFor?

Once you start a thread with FreeOnTerminate true then you must not use a reference to the thread again because you can't guarantee that it is valid. — David Heffernan, Jun 30 '18 at 17:03
@DavidHeffernan I think I've got it. When I call thread.Terminate; the thread dies and then, since FreeOnTerminate is true, the object is freed. Since the the object is free calling WaitFor won't work of course. (which is what you're saying) — Raffaele Rossi, Jun 30 '18 at 17:09
So I think that Terminate + WaitFor has to be used when FreeOnTerminate is false (and of course remember to free the assigned object) — Raffaele Rossi, Jun 30 '18 at 17:10
On a side note, you're not supposed to call `inherited` from within `Execute`. — Jerry Dodge, Jun 30 '18 at 18:01
@ruff The thread might terminate before you call terminate. If you must keep a reference you need to nil it in On Terminate. — David Heffernan, Jun 30 '18 at 21:09
WaitFor() should not be used with FreeOnTerminate=True, because the thread handle is closed when the thread object is freed, which is what causes the "handle is invalid" exception. FreeOnTerminate=True should only be used for "start-and-forget" type of threads. — Remy Lebeau, Jun 30 '18 at 21:19
@David I'm curious when that happened. I just tested and it is in fact ignored. But I do recall in the past it would raise `Abstract error` as it's flagged as `virtual; abstract;`. I can't find any information about it changing. — Jerry Dodge, Jun 30 '18 at 21:25
@JerryDodge LURD provided the distinction here: https://stackoverflow.com/questions/29480402/how-can-i-use-a-dictionary-stringlist-inside-execute-of-a-thread-delphi/29516640#comment47233755_29516640 ... IMHO Superfluous **inherited** should still be deleted otherwise brain has to do a double-take every time the code is read. It's unfortunate that the IDE inserts it in places it does not belong. — Disillusioned, Jul 01 '18 at 01:51
@Craig Indeed, it should be very trivial to exclude `inherited` in `override` and especially `abstract` methods. The fact that the compiler ignores this case tells me a work-around was implemented instead of an actual fix :-/ — Jerry Dodge, Jul 01 '18 at 01:53

score 8 · Accepted Answer · answered Jul 01 '18 at 06:47

8

Please tell me what I'm missing.

The documentation for FreeOnTerminate explicitly says that you cannot use the Thread in any way after Terminate.

That includes your WaitFor call, which would work on a possibly already free'd object. This use-after-free can trigger the error above, among other even more "interesting" behaviours.

answered Jul 01 '18 at 06:47

Turbo J

7,563
1
23
43

6

The documentation is too lenient. You can't really access a `FreeOnTerminate=True` thread for **any reason**. The thread might terminate itself (crash, etc) before you have even had a chance to call `Terminate` at all. You should use `FreeOnTerminate=True` only for "start-and-forget" threads that you don't care about accessing after starting them. But once you need to access them for **any reason**, `FreeOnTerminate=True` becomes dangerous to use – Remy Lebeau Jul 01 '18 at 20:58

Delphi TThread handle error

1 Answers1