0
votes

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:

enter image description here

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?

1
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
@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
On a side note, you're not supposed to call inherited from within Execute.Jerry Dodge
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
@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

1 Answers

8
votes

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.