Jump to content
JackT

Destroying a TThread recreating TThread the destroying again hangs Process

Recommended Posts

Has anyone come across an issue where by you create a thread using TThread then end it by calling Terminate on it, and then recreate the thread again,  then terminate the 2nd instance of the thread using the terminate only to find that the 2nd instance on termination gets stuck in an infinite loop after it exits the Execute block.

 

The program appears to be stuck in ntdll.dll and the whole program has hung not just the terminating thread.

 

So to be clear all the code in the Execute block has finished executing.

 

I am a bit stuck - sometimes I can manage to create and destroy the thread 3 times.

 

I think this old post might describe the issue I am having. I Know it's on the FPC forum but it specifically mentions Delphi in the post.

 

https://gitlab.com/freepascal.org/fpc/documentation/-/issues/34530

 

 

Share this post


Link to post
25 minutes ago, JackT said:

Has anyone come across an issue where by you create a thread using TThread then end it by calling Terminate on it, and then recreate the thread again,  then terminate the 2nd instance of the thread using the terminate only to find that the 2nd instance on termination gets stuck in an infinite loop after it exits the Execute block.

 

The program appears to be stuck in ntdll.dll and the whole program has hung not just the terminating thread.

 

So to be clear all the code in the Execute block has finished executing.

 

I am a bit stuck - sometimes I can manage to create and destroy the thread 3 times.

 

I think this old post might describe the issue I am having. I Know it's on the FPC forum but it specifically mentions Delphi in the post.

 

https://gitlab.com/freepascal.org/fpc/documentation/-/issues/34530

 

 

Never had such issues.

 

I use a lot of TThreads (form 5 to 50), all manually created and destroyed in my applications. But "FreeOnTerminate" is always set to FALSE. Never used it with True sets, except in anonymous threads.

And I don't create and destroy most threads continuously, but leave them in a WaitFor state, until I use them again.

 

I'll try to do some tests in one my application left them to "autofree" ....

Share this post


Link to post
37 minutes ago, JackT said:

Has anyone come across an issue where by you create a thread using TThread then end it by calling Terminate on it, and then recreate the thread again,  then terminate the 2nd instance of the thread using the terminate only to find that the 2nd instance on termination gets stuck in an infinite loop after it exits the Execute block.

 

It would help if you can post the code example here. With threads even slight change in code can make a difference. 

 

But the example in the link shows waiting for self destroying thread and this is something which cannot be done with such thread. Hanging is expected behavior.

  • Like 1

Share this post


Link to post

Do not use "FreeOnTerminate" and free it in your code after your "waitfor".

 

Alternatively do not use "WaitFor", use the event "OnTerminate" of the thread to proceed your work. This event is called from MainThraed

Share this post


Link to post

The actual call that hangs in TThread is EndThread which calls the windows function ExitThread. I think ExitThread hangs if there are still open handles to the Thread.

To close a handle to a thread code must use the windows CloseHandle function. It would be handy to get a count of open handles to the thread before ExitThread is called, and a list of objects with open handles.

In that way I could discover which object has on open handle. It's just a bit weird I can exit the first instance of my threaded class, but subsequent instances hang the program on exit from Execute.

I think reusing the same thread and suspending it would be a great work around for what I am trying to do. There is no real need to destroy it and recreate it constantly. The code inside Execute method is very complex which is why I haven't posted it. 

Share this post


Link to post

You don't need to post the code within Execute. We just need to know how are you creating and waiting for a thread. Is it a self destroying thread, or not. If it is self destroying thread with FreeOnTerminate set to True, then you cannot wait for it. If you need a waitable thread, just use a thread with manual memory management and free it when you are done. There is nothing wrong with creating and destroying thread when you need one. The only problem with recreation is that creating and starting a thread takes some small amount of time, and if you need to do that frequently (multiple times in a second) then such code will have some small impact on performance.

Share this post


Link to post

For my tasks, "FreeOnTerminate" is always set to False in all my applications.

 

There are a lot of reasons for that (I repeat, for my uses), one of them is that I need a waitable thread 99% of the times.

If your thread release a resources, then you must use the "waitfor" or similar since the thread can be busy before free it or resuse it (and this is the real reason why I cannot use FreeOnTerminate).

 

And even if there's no need to release resources, there's no way to know when a thread with FreeOnTerminate set to True will actually terminate.
By running tests under load and in a 64-bit Windows environment (my applications generate an average load of around 75% on 32-thread Intel i9 processors), I found that low-priority threads take a long time to release. In some cases, it can take up to a few tens of milliseconds, so without a WaitFor, it's a risk.

 

 

Share this post


Link to post
3 hours ago, DelphiUdIT said:

And even if there's no need to release resources, there's no way to know when a thread with FreeOnTerminate set to True will actually terminate.

Yes, there is - use the TThread.OnTerminate event to signal your waiting code, such as via a TEvent object.  Just know that the event handler is called in the main thread, so if that will cause a sync issue, you can always override the TThread.DoTerminate() method to call the OnTerminate handler directly (or do anything else) without syncing it, as DoTerminate() is called in the worker thread and calls the OnTerminate handle via TThread.Synchronize() by default. 

Edited by Remy Lebeau
  • Thanks 1

Share this post


Link to post
20 hours ago, Olli73 said:

Do not use "FreeOnTerminate" and free it in your code after your "waitfor".

 

16 hours ago, Dalija Prasnikar said:

If it is self destroying thread with FreeOnTerminate set to True, then you cannot wait for it.

Great, correct and make sense, but the documentation should say so, also that is bad design in the first place, and i don't want to go there, but have to say you don't design such always loaded foot gun and remove the safety.

 

Someone should open a ticket and request to make WaitFor raise an exception in case the FreeOnTerminate is true, doesn't that make more sense ?

Share this post


Link to post
3 hours ago, Kas Ob. said:

Great, correct and make sense, but the documentation should say so, also that is bad design in the first place, and i don't want to go there, but have to say you don't design such always loaded foot gun and remove the safety.

I guess it was not anticipated that developers would store references to self destroying threads. And without having a reference you cannot wait for such thread. References to self destroying threads basically become invalid the moment the thread is started and they can only be used for initial configuration for threads that were created in suspended state. Once thread starts running, you should not touch the reference anymore. 

 

From documentation: 

 

https://docwiki.embarcadero.com/Libraries/Athens/en/System.Classes.TThread.CreateAnonymousThread 

 

Quote

The thread is also marked as FreeOnTerminate, so you should not touch the returned instance after calling Start. It is possible that the instance had run and then has been freed before other external calls or operations on the instance were attempted.

 

https://docwiki.embarcadero.com/Libraries/Athens/en/System.Classes.TThread.FreeOnTerminate

 

Quote

 

Warning: When FreeOnTerminate is true, the Execute method may run and then free the thread before your application can execute the next line of code. Thus, you should not call any methods of the thread object when FreeOnTerminate is true unless you create the thread in a suspended state.

FreeOnTerminate is best left for threads that simply perform a task and then naturally terminate. If you want to communicate with the thread or otherwise interact with it, including telling it when to terminate, FreeOnTerminate should never be used.

 

 

 

I am not going to discuss about how one can potentially use anonymous threads beyond their intended purpose: being fire and forget kind of thread, because such code can be very fragile and cause more problems than it solves.

 

And like I mentioned earlier in this thread, without having a very specific code and a use case, it can be hard to discuss potential problems and solutions because in multithreading even slight changes in code and how it is used can make a huge difference and break it.

Edited by Dalija Prasnikar
  • Like 1

Share this post


Link to post
1 hour ago, Dalija Prasnikar said:

The thread is also marked as FreeOnTerminate, so you should not touch the returned instance after calling Start.

So what is/was the point from design point to return TThread ? (shouldn't be different creature with single method "Start")

 

I know the details, i pointing the failure to protect the developer by design and depending on well you have been warned (somewhere).

 

IMHO, once FreeOnTerminate is true then most methods should raise exception to make sure it will work always as expected, not sometimes, and here i am pointing to the fact FreeOnTerminate was runtime adjustable at anytime, while it should fixed since the creating that TThread, again it is my opinion.

Share this post


Link to post
6 hours ago, Kas Ob. said:

So what is/was the point from design point to return TThread ? (shouldn't be different creature with single method "Start")

Because TThread can be self destroying or not based on the value in FreeOnTerminate property which can be configured after construction. Besides that property, there are others that can be configured before thread starts running, 

 

Also how would you prevent developers from taking reference of any abstract class that is meant to be extended and is therefore publicly accessible?

Share this post


Link to post
11 hours ago, Dalija Prasnikar said:

Also how would you prevent developers from taking reference of any abstract class that is meant to be extended and is therefore publicly accessible?

By not returning TThread in the first place, and using smaller creature (TAnonThread, TThreadWorker, TThreadedRoutine....) with only can-be-used properties (with getter/setter) before start,

now let see what one can use before start,... hmmm... nothing !, except priority and name and a maximum of three events OnBeforeExecute, OnDoneExecute and OnTerminate, everything else is useless and dangerous from this point after the method "Start"

 

CreateAnonymousThread is mute and return a class that can not be extended, so fundamentally is wrong to return a class that has dangerous publics, it should return a class that can't be abused, right ?

 

(by the way i don't use anonymous thread and hate them more than anything, their exceptions are anonymous and their call stack is unreadable and different platform like ARM their exceptions are fatal and time wasting),

 

Now back to FreeOnTerminate, the danger of using FreeOnTerminate is coming from it allowed to be used and might work, but really put the undefined behavior in hidden state that might fail in the future or with different parameters like parsing smaller files than expected or under different OS state, what i am saying is that if WaitFor worked most the time but hid that dangerous behavior then the the design should be fixed, and it should raised exception with FreeOnTerminate is True on lets say WaitFor, this will prevent the working state (the dangerous state) from stay hidden and force the developer to rethink their design, so no more it it is working fine then it will work forever fine.

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×