Thread destroy never gets executed

Question

Download the source code with compiled executable (221 KB (226,925 bytes)): http://www.eyeclaxton.com/download/delphi/skeleton.zip

Why doesn't the Destroy destructor get called if I close the application (click the X button) before the thread has terminated? FastMM4 reports a memory leak with FPauseEvent event.

How should i destroy thread? If someone closes the application before the thread finishes.

unit SkeletonThread;

interface

uses
  Windows, Classes, SysUtils, SyncObjs;

type
  TOnInitialize = procedure(Sender: TObject; const AMaxValue: Integer) of object;
  TOnBegin = procedure(Sender: TObject) of object;
  TOnProgress = procedure(Sender: TObject; const APosition: Integer) of object;
  TOnPause = procedure(Sender: TObject; const APaused: Boolean) of object;
  TOnFinish = procedure(Sender: TObject) of object;
  TOnFinalize = procedure(Sender: TObject) of object;

  TMasterThread = class(TThread)
  private
    { Private declarations }
    FPaused: Boolean;
    FPosition: Integer;
    FMaxValue: Integer;
    FOnBegin: TOnBegin;
    FOnProgress: TOnProgress;
    FOnFinish: TOnFinish;
    FOnInitialize: TOnInitialize;
    FOnFinalize: TOnFinalize;
    FPauseEvent: TEvent;
    FOnPause: TOnPause;
    procedure BeginEvent();
    procedure ProgressEvent();
    procedure FinishEvent();
    procedure InitializeEvent();
    procedure FinalizeEvent();
    procedure PauseEvent();
    procedure CheckForPause();
  protected
    { Protected declarations }
    procedure DoInitializeEvent(const AMaxValue: Integer); virtual;
    procedure DoBeginEvent(); virtual;
    procedure DoProgress(const APosition: Integer); virtual;
    procedure DoPauseEvent(const APaused: Boolean); virtual;
    procedure DoFinishEvent(); virtual;
    procedure DoFinalizeEvent(); virtual;
  public
    { Public declarations }
    constructor Create(const CreateSuspended: Boolean; const theValue: Integer);
    destructor Destroy(); override;
    procedure Pause();
    procedure Unpause();
  published
    { Published declarations }
    property IsPaused: Boolean read FPaused write FPaused default False;
    property OnInitialize: TOnInitialize read FOnInitialize write FOnInitialize default nil;
    property OnBegin: TOnBegin read FOnBegin write FOnBegin default nil;
    property OnProgress: TOnProgress read FOnProgress write FOnProgress default nil;
    property OnPause: TOnPause read FOnPause write FOnPause default nil;
    property OnFinish: TOnFinish read FOnFinish write FOnFinish default nil;
    property OnFinalize: TOnFinalize read FOnFinalize write FOnFinalize default nil;
  end;

  TSkeletonThread = class(TMasterThread)
  private
    { Private declarations }
    procedure DoExecute(const theValue: Integer);
  protected
    { Protected declarations }
    procedure Execute(); override;
  public
    { Public declarations }
  published
    { Published declarations }
  end;

implementation

{ TMasterThread }
constructor TMasterThread.Create(const CreateSuspended: Boolean; const theValue: Integer);
begin
  inherited Create(CreateSuspended);
  Self.FreeOnTerminate := True;

  Self.FPosition := 0;
  Self.FMaxValue := theValue;
  Self.FPaused := False;
  Self.FPauseEvent := TEvent.Create(nil, True, True, '');
end;

destructor TMasterThread.Destroy();
begin
  FreeAndNil(FPauseEvent);
  if (Pointer(FPauseEvent) <> nil) then Pointer(FPauseEvent) := nil;
  inherited Destroy();
end;

procedure TMasterThread.DoBeginEvent();
begin
  if Assigned(Self.FOnBegin) then Self.FOnBegin(Self);
end;

procedure TMasterThread.BeginEvent();
begin
  Self.DoBeginEvent();
end;

procedure TMasterThread.DoProgress(const APosition: Integer);
begin
  if Assigned(Self.FOnProgress) then Self.FOnProgress(Self, APosition);
end;

procedure TMasterThread.ProgressEvent();
begin
  Self.DoProgress(Self.FPosition);
end;

procedure TMasterThread.DoFinishEvent();
begin
  if Assigned(Self.FOnFinish) then Self.FOnFinish(Self);
end;

procedure TMasterThread.FinishEvent();
begin
  Self.DoFinishEvent();
end;

procedure TMasterThread.DoInitializeEvent(const AMaxValue: Integer);
begin
  if Assigned(Self.FOnInitialize) then Self.FOnInitialize(Self, AMaxValue);
end;

procedure TMasterThread.InitializeEvent();
begin
  Self.DoInitializeEvent(Self.FMaxValue);
end;

procedure TMasterThread.DoFinalizeEvent();
begin
  if Assigned(Self.FOnFinalize) then Self.FOnFinalize(Self);
end;

procedure TMasterThread.FinalizeEvent;
begin
  Self.DoFinalizeEvent();
end;

procedure TMasterThread.DoPauseEvent(const APaused: Boolean);
begin
  if Assigned(Self.FOnPause) then Self.FOnPause(Self, APaused);
end;

procedure TMasterThread.PauseEvent();
begin
  Self.DoPauseEvent(Self.FPaused);
end;

procedure TMasterThread.Pause();
begin
  Self.FPauseEvent.ResetEvent();
  Self.FPaused := True;
  Self.Synchronize(Self.PauseEvent);
end;

procedure TMasterThread.Unpause();
begin
  Self.FPaused := False;
  Self.Synchronize(Self.PauseEvent);
  Self.FPauseEvent.SetEvent();
end;

procedure TMasterThread.CheckForPause();
begin
  if (not (Self.Terminated)) then Windows.Sleep(1);
  Self.FPauseEvent.WaitFor(INFINITE);
end;

{ TSkeletonThread }
procedure TSkeletonThread.DoExecute(const theValue: Integer);
var
  X: Integer;
begin
  Self.Synchronize(InitializeEvent);
  try
    Self.Synchronize(BeginEvent);
    try
      for X := 0 to (theValue - 1) do
      begin
    Self.CheckForPause();

    if (not Self.FPaused) and (not Self.Terminated) then
    begin
      Self.FPosition := Self.FPosition + 1;
      Self.Synchronize(ProgressEvent);
    end
    else begin
      Break;
    end;
      end;

      for X := Self.FPosition downto 1 do
      begin
    Self.CheckForPause();

    if (not Self.FPaused) and (not Self.Terminated) then
    begin
      Self.FPosition := X;
      Self.Synchronize(ProgressEvent);
    end
    else begin
      Break;
    end;
      end;
    finally
      Self.Synchronize(FinishEvent);
    end;
  finally
    Self.Synchronize(FinalizeEvent);
  end;
end;

procedure TSkeletonThread.Execute();
begin
  Self.DoExecute(Self.FMaxValue);
end;

end.

Not answering but just a thought. What's the point of using separate thread in your code if everything You do is executed in the main thread (You are using synchronize everywhere)? — Linas, Apr 05 '11 at 11:09
When you shut the app down are you calling Terminate on the thread and waiting for it to stop? — David Heffernan, Apr 05 '11 at 11:11
I bet David is correct. You probably just close the app without terminating the thread and waiting for it to actually stop. This is why FastMM reportst the leak. — Runner, Apr 05 '11 at 11:23
@Linas, I can see this as a form of very cumbersome co-routine — it lets two separate execution paths run cooperatively on a single thread. — Rob Kennedy, Apr 05 '11 at 13:57
In short, you could replace it with a TTimer, and that would save you from this running-but-worthless background TThread, and you would be just as multi-threaded as you were before. — Warren P, Apr 05 '11 at 15:52

score 2 · Accepted Answer · answered Apr 05 '11 at 11:29

2

You have to terminate the thread yourself (tell it to stop). One way is to use the Terminate procedure of the thread, but you have to check for this in the thread Execute method. Something like this:

procedure Execute;
begin
  inherited;

  while not Terminated do
  begin
    // do your job
  end;
end;

procedure TForm1.StopThread;
begin
  MyThread.Terminate;

  // wait and block until the scheduling thread is finished
  AResult := WaitForSingleObject(MyThread.Handle, cShutdownTimeout);

  // check if we timed out
  if AResult = WAIT_TIMEOUT then
    TerminateThread(MyThread.Handle, 0);
end;

Or you can use signalization build into windows so you do not have to loop.

procedure Execute;
begin
  inherited;

  while not Terminated do
  begin
    WaitStatus := WaitForSingleObject(FTermEvent, Max(0, SleepInterval));

    // check what was the cause for signalization
    if WaitStatus <> WAIT_TIMEOUT then
      Terminate;
  end;
end;

procedure TForm1.StopThread;
begin
  // Terminate the thread
  SetEvent(FTermEvent);
  // close the handle
  CloseHandle(FTermEvent);

  // wait and block until the scheduling thread is finished
  AResult := WaitForSingleObject(MyThread.Handle, cShutdownTimeout);

  // check if we timed out
  if AResult = WAIT_TIMEOUT then
    TerminateThread(MyThread.Handle, 0);
end;

Signalization can be very neat way of signaling for termination because you can use WaitForMultipleObjects and release the wait in different conditions. I used WaitForSingleObject to not complicate things to much.

Also be sure to set "FreeOnTerminate := True" in thread constructor. Oh and the hard termination at the end is optional of course. It can be dangerous. You know best yourself if you will use it or not. You can also wait for a longer period or infinite if you are sure the thread will stop eventually.

answered Apr 05 '11 at 11:29

Runner

6,073
26
38

Sure it does explain. It never gets called because the thread is not destroyed before you app is. Threads do not destroy by themselves when you close you app. Yes they eventually get cleaned up when the OS destroys the process but your debugger will never see that. – Runner Apr 05 '11 at 12:07
Hm, obviously the comment I was refering to was deleted :) – Runner Apr 05 '11 at 12:08
Okay, if you run the application in the delphi IDE add a break point in the first line of TMasterThread.Destroy(), click the "start" button in the main form and wait for the thread to finish you'll see that the program will stop at your break point. If you do the same thing and this time before the thread finishes just click the close button the break point will NOT be executed. – eyeClaxton Apr 05 '11 at 12:14
Yes, because the thread did not finish. Clicking close button did not destroy the thread, it is still running. So Destroy is never called. I don't see anything unusual here. It works as it should. – Runner Apr 05 '11 at 13:46
I am not aware that WaitFor has a timeout. If not it just cries for close hangups. And this was just a simple example, usually WaitForMultipleObjecs comes into the mix (because of pipes etc...). Otherwise yes this is basically the same as WaitFor does :) – Runner Apr 05 '11 at 16:04
If your program is written correctly then you don't hang when calling WaitFor – David Heffernan Apr 05 '11 at 16:11
Ah, you're right. TEvent and the other synchronization objects have a timeout in their WaitFor methods, but TThread doesn't. I wonder why. – Rob Kennedy Apr 05 '11 at 16:24
@David. In ideal world you can make that, however as a programmer you can be in situations when this is not possible. Just a small example. I use a IAX library written in C++. Its working thread somethimes hangs on exit. I have no control over the code and a timeout ensures that my application exits even if this happens. – Runner Apr 05 '11 at 17:19
@Runner I live in an ideal world!! ;-) – David Heffernan Apr 05 '11 at 17:25
@Rob. It is interesting. Either designer had some deeper insight, or was just plain sloppy :) – Runner Apr 05 '11 at 17:52

Thread destroy never gets executed

1 Answers1