CODEWITHSUNDEEP
multithreadingdelphi
john_who_is_doe
john_who_is_doe

Reputation: 389

Thread termination (again...)

When i stop thread 'srch_slave_thread' with setting the termination flag to true, (srch_slave_thread.terminate) freeing the thread stops at the 'inherited' line in the destructor, why? Is it a WaitFor that it hangs on? If i comment out 'inherited' in the destructor, the thread stops and free itself.

After taking out 'inherited' and debuging the code: Why the thread jumps into the DoTerminate method after calling the destructor?

Thank You.

Tsrch_slave_thread = class(TThread)   
private
  FSW: TStopWatch;
protected
  procedure Execute; override;
public
  SimpleEvent: TSimpleEvent;
  procedure DoTerminate; override;
  ...
  constructor Create; 
  destructor Destroy; override;
end;

Creating an event obj. in the constructor >>

constructor Tsrch_slave_thread.create;
begin
  inherited create(true);   
  Fsw := TStopWatch.Create;
  SimpleEvent := TSimpleEvent.Create;
end;

Jumps here after calling destructor? >>

procedure Tsrch_slave_thread.DoTerminate;
begin
  inherited;
  self.simpleEvent.SetEvent; 
end;

Thread hangs at inherited in the destructor >>

destructor Tsrch_slave_thread.destroy;
begin
  self.SimpleEvent.free;
  inherited;
end;

Create the thread here >>

function TMaster.th_slvsearch_start: integer; 
begin
  if not Assigned( Fslave_search_thread ) then begin
    Fslave_search_thread := TFslave_search_thread.create;
    ... 
  end
  else begin  
    ...
    exit;
  end;  
  with Fslave_search_thread do
  begin    
    master := self;  
    master_HWND := self.fMsgHandlerHWND;
    FreeOnTerminate := false;
    OnTerminate := slvsrch_termination;
    start;         
  end;
end;

Stoping the thread starts here >>

procedure TMaster.th_slvsearch_stop;
begin
  Fslave_search_thread.Terminate; 
end;

Thread .Execute >>

procedure Tsrch_slave_thread.Execute;
var
  text_orig: string;
  activesearch: integer;
begin
  FSW.Start;
  while not terminated do begin
    activesearch := master.CMD_LISTCNT; 
    //stopper refresh
    synchronize(procedure begin         
        with self.master do
          Fmasternode.text := FmasterDat.MstrName + ' (' + floattostr(Fsw.ElapsedMilliseconds / 1000) + 'sec - Searching)';
    end);
    if (SimpleEvent.WaitFor(2000) <> wrTimeOut) or (activesearch <> 1) then break;  
  end;
  FSW.Stop;  
end;

OnTerminate event handler >>

procedure TMaster.slvsrch_termination(Sender: TObject);
begin
  if Assigned( Fslave_search_thread ) then
  begin
    self.FLastSearchTime := Fslave_search_thread.FSW.ElapsedMilliseconds / 1000;
    Fslave_search_thread.Free; 
    Fslave_search_thread := nil;
    self.Factive_slv_search := 0;
  end;
  ...
end;

Upvotes: 1

Views: 267

Answers (1)

David Heffernan
David Heffernan

Reputation: 613511

You are calling Free on the thread object from its OnTerminate event handler. That alone is a mistake because you cannot free an object whilst it is firing an event, because when the event returns you are now executing code in a object that has been destroyed.

But that's not your immediate problem. You've deadlocked the thread because of the call to WaitFor in TThread.Destroy. The thread cannot finish because it is waiting on itself. The call stack on the thread looks like this:

Synchronize(CallOnTerminate)
DoTerminate
ThreadProc

So the thread is waiting on the main thread to finish executing CallOnTerminate. Over on the main thread, inside your OnTerminate handler is the call to Free. Which in turn leads to a call to WaitFor. Consequently the main thread is waiting for the thread to finish. That's your classic deadlock. Thread A waiting on thread B, and thread B waiting on thread A.

The moral of the story: never call Free on a thread inside its OnTerminate event handler.

The solution is probably to set FreeOnTerminate to True and in the OnTerminate handler set Fslave_search_thread to nil.

Upvotes: 6

Related Questions