Waiting for multiple threads

[John Kouraklis 94]

Hi,

 

I've got an app that launches a few threads. Each thread does different things but they all write in the same log file. Some threads check the internet too. 

 

Everything works well in the normal course of the application.

 

The problem I have is when the user tries to shut down the app. What happens is that those threads that are attempting a web query or are writing in the log file raise an AV complaining that another process is attempting to write the log file. The writing process to log file is thread-safe for sure. 

 

Then, I created a global var to indicate the background running tasks. I use Atomic Inc/Dec to change the value and in FormCloseQuery I am waiting for the value to get to zero before the form is allowed to close.

 

But the problem with the log file still appears.

 

What's the strategy to shut down a multi-thread app?

 

I also contemplated the idea of having another global var to indicate that the app is shutting down and then exit from the threads but this will pollute the code and it does not feel right.   

 

 

[FredS 138]

29 minutes ago, John Kouraklis said:

global var to indicate that the app is shutting down

I use a global TEvent that is set at shutdown, all my wait functions check that and all threads check it either through wait or in their execute methods.

 

[vfbb 285]

1 hour ago, John Kouraklis said:

What's the strategy to shut down a multi-thread app?

 

This problem is very common because without proper care the threads end in a different order than the units.

The rule to avoid this is: each unit that creates each thread has to guarantee that the thread it created will be canceled and completely terminated until the finalization of the unit.

How to do this? There are several possibilities, I will quote one:

 

1) Create a way to cancel each thread (it is almost always possible through a simple Boolean + TEvent.SetEvent, but each case must be evaluated)
2) Create a unique TEvent to signal the end of the thread; Just run the thread code in a try finaly FFinishEvent.SetEvent;
3) At the end of the unit, cancel the thread, and give FFinishEvent.WaitFor;

[Vincent Parrett 754]

.NET has a nice construct for this, Cancellation Tokens. I created a delphi implementation a while ago 

 

https://github.com/VSoftTechnologies/VSoft.CancellationToken

 

It's an abstraction around an event, where the calling thread owns the CancellationTokenSource (which has the cancel method) and the threads are passed the CancellationToken - which has the IsCancelled method you can interrogate, and the Handle that can be passed into api calls that take waithandles (like WaitforMultipleObjects).  

 

I have used the cancellation tokens in this library to make http calls cancellable 

 

https://github.com/VSoftTechnologies/VSoft.HttpClient

 

It's also used in my https://github.com/VSoftTechnologies/VSoft.Awaitable async/await library (a abstraction over OmniThreadLibrary) 

 

All of the above are used in https://github.com/DelphiPackageManager/DPM - any methods that are potentially long running or might need to be cancelled take in an ICancellationToken  - so for example in the command line tool, invocking Ctrl+C does this

 

class procedure TDPMConsoleApplication.CtrlCPressed;
begin
  FLogger.Information('Ctrl-C detected.');
  FCancellationTokenSource.Cancel;
end;

That's all that's needed (from the outside at least) to cancel the task - and then in the tasks we pass the cancellation token

 

function TInstallCommand.Execute(const cancellationToken : ICancellationToken) : TExitCode;
begin
  // code deleted for brevity.
  if not FPackageInstaller.Install(cancellationToken, TInstallOptions.Default) then
    result := TExitCode.Error
  else
    result := TExitCode.OK;
end;

and in long running methods or tight loops 

 

  for platform in platforms do
  begin
    if cancellationToken.IsCancelled then
      exit(false);
...

or

 

  objHandles[0] := processInfo.hProcess;
  objHandles[1] := cancellationToken.Handle;

  { Wait for Something interesting to happen }
  waitRes := WaitForMultipleObjects(2, @objHandles, False, timeoutDuration);

 

Edited March 14, 2021 by Vincent Parrett
added more info

[Edwin Yip 154]

7 hours ago, FredS said:

I use a global TEvent that is set at shutdown, all my wait functions check that and all threads check it either through wait or in their execute methods.

 

I think this the standard and easy way. But don't forget to pass the TEvent object to each threads, avoid accessing any global vars in anywhere including inside the a thread.

[Dalija Prasnikar 1399]

3 hours ago, Edwin Yip said:

I think this the standard and easy way. But don't forget to pass the TEvent object to each threads, avoid accessing any global vars in anywhere including inside the a thread.

Not using global vars is good advice, but your particular advice here is wrong for several reasons.

 

First, accessing global state (and changing it) is always a problem when it comes to thread safety, because one thread can change that state and interfere with the other using that same state. Think of global TFormatSettings variable. Having formatting functions that works directly with global setting is not thread safe because different threads can change settings as they please. On the other hands functions that use format settings passed as parameter are safe. But the crucial thing here is not passing as parameter alone, but what happens when you pass it in this particular case. TFormatSettings is a record and when you pass it as parameter function will get a local copy of the whole record. That is what it makes it safe.

 

When you pass object instance as parameter, function will not get local copy of the object, just the reference. If the object is not thread safe and does not implement thread safety (synchronization) mechanisms that would allow safe access from multiple threads, then using parameter or global object directly is equally bad, and will not work.

 

However, when it comes to synchronization objects, including TEvent, their whole purpose is to be shared between multiple threads. And if you need to orchestrate threads on application level, the only way you can do that is through global object. Yes, you can still write code in such manner that it does not access global object directly, but that code will not be "more thread safe", but more flexible and allows you to pass different TEvent instances to different threads that may have different purpose and possible different synchronization events.

 

Using global locks, events and other synchronization objects is perfectly fine.

 

[John Kouraklis 94]

Thanks for the suggestions. 

 

I created an array of TEvents and each event is passed to each thread.

 

Then the TEvent.WaitFor...... method seems to do the job perfectly well.

 

[Edwin Yip 154]

@Dalija Prasnikar,

I don't follow your reasoning.

 

As you said, my advise of not accessing a global TEvent variable is to gain flexibility, but not thread-safety - the thread-safety has been provided by TEvent which is a 'sync object'.

 

So I really don't understand why you said the advise is wrong while you agree on the effects of the advise...

[John Kouraklis 94]

8 hours ago, Vincent Parrett said:

 

https://github.com/VSoftTechnologies/VSoft.CancellationToken

 

I use just a boolean flag but this seems more flexible and proper. I'll try it. Thanks

[Dalija Prasnikar 1399]

2 hours ago, Edwin Yip said:

As you said, my advise of not accessing a global TEvent variable is to gain flexibility, but not thread-safety - the thread-safety has been provided by TEvent which is a 'sync object'.

 

So I really don't understand why you said the advise is wrong while you agree on the effects of the advise...

I am sorry if I misunderstood your point.

 

You didn't explicitly said why are you suggesting passing TEvent instead of using global access and phrase "Don't forget" can be easily interpreted that if you use global objects directly, code will not work properly. At least that is how I have read it.

Edited March 14, 2021 by Dalija Prasnikar

[Guest]

hi @John Kouraklis

 

maybe my post about Threads "waiting for..." can have some relevance for you?

• here im using TASK.RUN to call 3 threads and, on the end, verifying if some is running yet...
• ok, can not be exactly what you do in your code, but It can help in some?

 

hug

 

[Guest]

17 hours ago, John Kouraklis said:

What's the strategy to shut down a multi-thread app?

There is no one solution to solve this, only your experience and better design.

 

One thing though, there is this suggestions, don't look at threads as simple creatures that work when you unleash them, and finished when they decide it is enough, on contrary see them as delicate creatures just like classes, means you design them with clear path, this is a start point, this is end point, and in between you can put events like progress.. also control procedure like terminate.

 

May be an example will be add some help here too, imagine you have application that will open a file parse the text and ensure that each line that ends with CRLF should be replaced with "."+CRLF, simple right ?

But what if that file was n lines with overall 1gb of data, here your very simple VCL application will block while it does parse that file for long time, this is normal and expected, now you are facing two problems, 

1) the application looks like not responsive.

2) what if the user wanted to close or abort, he wanted to turn the PC off.

 

We are still talking single thread (the main onw in VCL app), both above problems need to be solved, so 

1) adding progress bar that your will be nice, simple the loop that iterate the lines will update a TProgressBar like every 50k lines, but will this work ? most likely no, as updating progress bar will draw but the messages from the OS to repaint will be blocked hence it will still looks like stuck.

2) Your application will not be able to process Close/Terminate.. as long the iteration loop busy.

adding Application.ProcessMessage here is not ideal but will solve 2 and complete 1 above.

 

Now you want to do it better and move the parsing loop into background thread, by simply moving the code to a thread, but two things to keep in mind updating the progress bar and accepting close/terminate, and both will be handled as the same as with the main, except you have even more approaches to use, you can use PostMessage or Synchronize ... for progress bar (aka call back and updating status/progress), also you can check for local field to terminate and break form the loop the same way and in the same place you used Application.ProcessMessages.

 

As for global or not, i will count to 1000 before write code for a thread that will access any foreign data, so it is better to use events for call back from there you can send message or synchronize, with this you are writing clean code and separated logically, also the thread can have a procedure (kill/terminate/exit switch) that will update a local field to decide for exiting.

 

This goes for TThread or a class that encapsulate some threading model, or whatsoever, the idea is when you design a thread start with these four points with the simplest and clearest way in your mind, Start, Update, Terminate and Exit.

 

In short threads themselves should be control terminate and exit, like if they doing socket operations then a simple call from anywhere, should be enough to close the socket, hence will break even blocking operation that might take long time, another one, in case of looping/processing big data, then break the loop into two outer and inner, the inner one will go full speed not checking for anything, but the outer will check for termination and report updating,.. I/O operations overlapped or not also can be interrupted, drawing the same, all this terminate call can be interfaced with single call like EnoughForNow that can be called form anywhere means should be thread safe.

 

 

 

[Edwin Yip 154]

2 hours ago, Dalija Prasnikar said:

I am sorry if I misunderstood your point.

 

You didn't explicitly said why are you suggesting passing TEvent instead of using global access and phrase "Don't forget" can be easily interpreted that if you use global objects directly, code will not work properly. At least that is how I have read it.

No problem. Human's language is sometimes ambiguous, the Pascal language is not, fortunately.

[John Kouraklis 94]

@Kas Ob.Thanks for the detailed example.

 

I have a class that manages all the thread so it was easy to keep track of TEvent for each threads as they are not scattered everywhere in the code.

 

I haven't looked at the ability to cancel the thread yet. I think @Vincent Parrett's approach looks interesting

[Fr0sT.Brutal 900]

Usually there's a list of all running threads in process (anon threads are simple but lead to hell) and stopping them involves accessing every instance anyway so there's no big difference between local or global stop events, just the stopper code:
 

SetEvent(GlobalStopEvent)
for thr in Threads do
  thr.WaitFor

vs

for thr in Threads do
  SetEvent(thr.StopEvent)
for thr in Threads do
  thr.WaitFor

For the best flexibility the stop event could be optionally passed to a thread or created locally if not specified.

[FredS 138]

8 hours ago, Fr0sT.Brutal said:

Usually there's a list of all running threads in process

Where is that?

 

As for GlobalStopEvent, I simply use a TThread Helper with a private class FShutdownEvent.

 

Every thread uses TWait.For<whatnot> which allows multiple handles and always adds the ShutdownEvent, within the Thread's Execution one can call TWait.IsShutdown without waiting..

Very close to:

 

Edited March 15, 2021 by FredS

[Fr0sT.Brutal 900]

30 minutes ago, FredS said:

Where is that?

I meant, multithread apps usually have one. Otherwise they will have big troubles on closing