5

I encounter an awkward problem. In my app I often do

TThread.createAnonymousThread(
  procedure
    ....
  end).start

The problem I have is that when I close the main form of my app, then sometime some of those AnonymousThread are still alive after the Tform.destroy finished . Is their a way in my Tform.destroy to wait that all those AnonymousThread (created a little everywhere in the whole app) are successfully terminated before to continue ?

I found this way to list all running thread (from How can I get a list with all the threads created by my application) :

program ListthreadsofProcess;

{$APPTYPE CONSOLE}

uses
  PsAPI,
  TlHelp32,
  Windows,
  SysUtils;

function GetTthreadsList(PID:Cardinal): Boolean;
var
  SnapProcHandle: THandle;
  NextProc      : Boolean;
  TThreadEntry  : TThreadEntry32;
begin
  SnapProcHandle := CreateToolhelp32Snapshot(TH32CS_SNAPTHREAD, 0); //Takes a snapshot of the all threads
  Result := (SnapProcHandle <> INVALID_HANDLE_VALUE);
  if Result then
  try
    TThreadEntry.dwSize := SizeOf(TThreadEntry);
    NextProc := Thread32First(SnapProcHandle, TThreadEntry);//get the first Thread
    while NextProc do
    begin
      if TThreadEntry.th32OwnerProcessID = PID then //Check the owner Pid against the PID requested
      begin
        Writeln('Thread ID      '+inttohex(TThreadEntry.th32ThreadID,8));
        Writeln('base priority  '+inttostr(TThreadEntry.tpBasePri));
        Writeln('');
      end;

      NextProc := Thread32Next(SnapProcHandle, TThreadEntry);//get the Next Thread
    end;
  finally
    CloseHandle(SnapProcHandle);//Close the Handle
  end;
end;

begin
  { TODO -oUser -cConsole Main : Insert code here }
  GettthreadsList(GetCurrentProcessId); //get the PID of the current application
  //GettthreadsList(5928);
  Readln;
end.

but it's look like that in this list their is some threads that are not really made by my code and that those threads never close. For example for a blank project this is the list of threads :

enter image description here

zeus
  • 12,173
  • 9
  • 63
  • 184
  • If you don't preserve the return value from CreateAnonymousThread, you've lost any chance to detect its termination (unless the threads in some way increase a global variable on startup and decreases it on termination). – HeartWare May 11 '22 at 13:53
  • There's lots of ways to do this. The way I do it, is to make a "list of threads" and when the application tries to close, before you allow it to close, you either purposely terminate, or wait for the threads that are still running at that moment, to terminate first. – Freddie Bell May 11 '22 at 13:53
  • In any other case you'd also store valuable information to a variable - why not doing that with your threads, too? Add them to a list per creation, and remove them per destruction. – AmigoJack May 11 '22 at 13:54
  • Take a look at Parallel Programming Library, https://docwiki.embarcadero.com/RADStudio/Sydney/en/Using_TTask_from_the_Parallel_Programming_Library and see the TTask.WaitForAll method – Marcodor May 11 '22 at 14:04
  • When using `TTask.Run` it automatically waits on application shutdown for all running tasks to finish. If that is too late and you want to wait earlier like on your form destroy or close then you have to store the ITask references and the aforementioned way to wait on them. – Stefan Glienke May 11 '22 at 14:17
  • You could also use `TCountdownEvent` for counting active anonymous threads. – Dalija Prasnikar May 11 '22 at 14:37
  • I updated the question with a way to list all running threads in my app, but now I don't know how to detect the anonymous thread created by my code – zeus May 11 '22 at 14:42
  • @zeus, taking a thread list snapshot does not guarantee that all are still alive in the next millisecond, imo wrong way. – Marcodor May 11 '22 at 15:39
  • *sometimes some of those AnonymousThread are still alive* sounds suspicious to me. When or why do you expect those tasks to terminate? Maybe it’s an error in the task scheduler? – Hartmut Braun May 11 '22 at 16:29
  • The threads are not finished with their work - if you close an application while some thread is still running it just stops - windows terminates the thread at some indeterministic point and you might have a data loss or worse. This fact makes using `CreateAnonymousThread` with fire and forget a possibly huge problem. – Stefan Glienke May 11 '22 at 16:57
  • @StefanGlienke yes exactly :( this why would like when I destroy the main form to wait that all threads are finished before closing the app. I can retrieve the list of thread via CreateToolhelp32Snapshot but the problem is that this api list ALSO some thread not made by my code and thoses threads never finish so not possible to wait them – zeus May 11 '22 at 19:47

3 Answers3

9

Core problem you are facing does not come from the anonymous threads as such, but from self-destroying anonymous threads - the ones that have FreeOnTerminate set.

In order to wait on a thread, you need to have reference to a thread or its handle (Windows platform). Because you are dealing with self-destroying threads, taking reference to a thread is not an option, because as soon as you start the thread you are no longer allowed to touch that reference.

Delphi RTL does not perform any cleanup for the self destroying anonymous threads during application shutdown, so those threads will be just killed by the OS after your application main form is destroyed, hence your problem.

One of the solutions that will allow you to wait for anonymous threads, that does not require any complicated housekeeping and maintaining any kind of lists, and that also requires minimal changes to the code that can be done with simple find and replace, is using TCountdownEvent to count threads.

This requires replacing TThread.CreateAnonymousThread with constructing custom thread class TAnoThread.Create (you can add static factory method if you like, instead of directly calling constructor) that will have same behavior as anonymous thread, except its instances will be counted and you will be able to wait on all such threads to finish running.

type
  TAnoThread = class(TThread)
  protected
    class var
      fCountdown: TCountdownEvent;
    class constructor ClassCreate;
    class destructor ClassDestroy;
  public
    class procedure Shutdown; static;
    class function WaitForAll(Timeout: Cardinal = INFINITE): TWaitResult; static;
  protected
    fProc: TProc;
    procedure Execute; override;
  public
    constructor Create(const aProc: TProc);
  end;

class constructor TAnoThread.ClassCreate;
begin
  fCountdown := TCountdownEvent.Create(1);
end;

class destructor TAnoThread.ClassDestroy;
begin
  fCountdown.Free;
end;

class procedure TAnoThread.Shutdown;
begin
  fCountdown.Signal;
end;

class function TAnoThread.WaitForAll(Timeout: Cardinal): TWaitResult;
begin
  Result := fCountdown.WaitFor(Timeout);
end;

constructor TAnoThread.Create(const aProc: TProc);
begin
  inherited Create(True);
  fProc := aProc;
  FreeOnTerminate := True;
end;

procedure TAnoThread.Execute;
begin
  if fCountdown.TryAddCount then
    try
      fProc;
    finally
      fCountdown.Signal;
    end;
end;

And then you can add following code in your form destructor or any other appropriate place and wait for all anonymous threads to finish running.

destructor TForm1.Destroy;
begin
  TAnoThread.Shutdown;
  while TAnoThread.WaitForAll(100) <> wrSignaled do
    CheckSynchronize;
  inherited;
end;

Principle is following: countdown event is created with value 1, and when that value reaches 0, event will be signaled. To initiate shutdown, you call Shutdown method which will decrease initial count. You cannot call this method more than once because it would mess up the count.

When anonymous thread Execute method starts, it will first attempt to increase the count. If it cannot do that, it means countdown event is already signaled and thread will just terminate without calling its anonymous method, otherwise anonymous method will run and after it finishes count will be decreased.

If anonymous threads use TThread.Synchronize calls, you cannot just call WaitForAll because calling it from the main thread will deadlock. In order to prevent deadlock while you are waiting for the threads to finish, you need to call CheckSynchronize to process pending synchronization requests.

This solution counts all threads of the TAnoThread class regardless of whether they are self-destroying or not. This can be easily changed to count only those that have FreeOnTerminate set.

Also, when you call Shutdown, and you still have some running threads, new threads will still be able to start at that point because countdown even is not signaled. If you want to prevent new threads from that point on, you will need to add a Boolean flag that will indicate you have initiated shutdown process.

Dalija Prasnikar
  • 27,212
  • 44
  • 82
  • 159
  • 4
    "*Because you are dealing with self-destroying threads, taking reference to a thread is not an option, because as soon as you start the thread you are no longer allowed to touch that reference*" - you can if you assign an `OnTerminate` handler to the thread before `Start()`'ing it. Then you can save a reference, and clear the reference when the thread is about to self-destroy. Just don't call `WaitFor()` on a self-destroying thread (it will crash), but if you need to wait on such a thread then you can save the thread's `Handle` before `Start()`, and then you can use OS wait functions on it. – Remy Lebeau Jun 10 '22 at 16:09
  • @RemyLebeau Yes, there are some other possibilities for handling self-destroying threads. But all those are way more convoluted than simply not using self-destroying threads and manually handling thread lifetime. And in both cases amount of house keeping required (storing handles or thread references) is basically the same. Since the idea was to solve problem with cleanup on shutdown with the solution that requires minimal adaptations for existing code, I haven't covered all other possible approaches and I avoided explanations that would make answer more complicated. – Dalija Prasnikar Jun 10 '22 at 17:36
  • 1
    For more info on TCountdownEvent see the blog article: https://www.ideasawakened.com/post/test-driving-delphi-s-tcountdownevent – Darian Miller Jun 11 '22 at 02:33
4

Reading all threads from the process and then trying to figure out which ones to wait for sounds like the path to a lot of pain.

If you really don't want to store the references of the anonymous threads (which then by the way should not be FreeOnTerminate as that would cause dangling references if the thread ends before you wait for it) then build a wrapper around TThread.CreateAnonymousThread or TTask.Run which does the storage internally and encapsulates the WaitFor. You could even be fancy and add an additional group key so you can create and wait for different set of threads/tasks instead of all.

Stefan Glienke
  • 20,860
  • 2
  • 48
  • 102
  • This isn't really an answer, it's just a long comment. – Freddie Bell May 11 '22 at 15:08
  • 1
    Because I am not spoon-feeding the solution? – Stefan Glienke May 11 '22 at 15:59
  • No, because you didn't provide even 1 line of code. – Freddie Bell May 11 '22 at 16:01
  • 6
    Since when does answering a question on SO require posting some code? I explained a possible solution in simple words – Stefan Glienke May 11 '22 at 16:02
  • I know. But your words required a base-knowledge equivalent to create the answer I gave below, and clearly the OP doesn't have that. – Freddie Bell May 11 '22 at 16:03
  • @FreddieBell Strictly speaking `TThread.CreateAnonymousThread` and `TTask.Run` and `WaitFor` (and `FreeOnTerminate`) is code already. – AmigoJack May 11 '22 at 17:06
  • @AmigoJack Very funny. – Freddie Bell May 11 '22 at 17:13
  • yes building a wrapper around TThread.CreateAnonymousThread and using TcountDownEvent (or similar counter) like @Dalija Prasnika suggest is an option but I would like to find a solution that will avoid to update all my existing code – zeus May 11 '22 at 19:56
  • 1
    @zeus there is no way you can solve your problem without touching existing code. Making custom thread class that will handle all details and behaves like anonymous thread class in other aspects sounds like the least intrusive solution. Then you can simply rename `TThread.CreateAnyonymousThread` to `TMyThread.CreateAnonymousThread` – Dalija Prasnikar May 12 '22 at 05:54
  • 2
    @zeus However, plain waiting for a threads or countdown event to finish in the context of main thread will deadlock if you are using Synchronize in those threads. So any solution will open up another can of worms. – Dalija Prasnikar May 12 '22 at 05:58
  • @DalijaPrasnikar YES that a pain in any case :( in the final onDestroy of my form I do ugly stuff like "while do sleep(100); checksynchronize; end ... ugly i know but my first target is in DEBUG to close the app in a proper way to check for memory leak and for that i need to wait that all thread are finished and released – zeus May 12 '22 at 07:28
3

Push in a thread-safe list the TThread references returned by CreateAnonymousThread, make sure to keep it in sync when a thread terminates and implement your own WaitForAll.

Or, consider to use TTask class for a simple parallel threads management. It have already implemented WaitForAll method.

Sample code took from Delphi help:

procedure TForm1.MyButtonClick(Sender: TObject);
var 
  tasks: array of ITask; 
  value: Integer; 
begin 
  Setlength (tasks ,2); 
  value := 0; 

 tasks[0] := TTask.Create (procedure () 
  begin 
   sleep(3000);
   TInterlocked.Add (value, 3000); 
  end); 
 tasks[0].Start; 

 tasks[1] := TTask.Create (procedure () 
   begin 
   sleep (5000);
   TInterlocked.Add (value, 5000);
 end); 
 tasks[1].Start; 

 TTask.WaitForAll(tasks); 
 ShowMessage ('All done: ' + value.ToString); 
end;
Marcodor
  • 4,578
  • 1
  • 20
  • 24
  • thanks @Marcodor, that possible but I would prefer a solution that not make me touch all the existing code in my app. – zeus May 11 '22 at 19:50
  • :) Maybe the solution I look simply do not exist :( – zeus May 12 '22 at 04:44
  • @zeus, if you still want to stick with `CreateAnonymousThread` then a thread list or a thread counter with a `SetEvent` when all threads are finished, seems the way. But anyway you'll have to touch a bit the code ;) however, if possible, i'd go with `TTask`'s as they ofer more control, take into account cpu resources, and you don't have to reinvent your own thread pool/manager – Marcodor May 12 '22 at 09:25