Is there a way to use FreeOnTerminate other that setting it in the constructor (or before the thread starts / or in the rather complex manner below)?

The doc does not mention any limitations
https://www.freepascal.org/docs-html/rtl/classes/tthread.freeonterminate.html

However, setting FreeOnTerminate*after* Execute() has finished has no effect.

So code like:

   t := TThread.create(false); // create  not suspended
   // do something
   if foo then begin // decide we do not need the result
     t.FreeOnTerminate:= true;
     t.terminate;
  end;

This may fail. If the thread's execute already finished, then the thread is not destroyed.
The code cannot call WaitFor, because the thread could still be running for a long time. And even take a long time until it recognizes the "terminate" request.
So the app wants just to signal it, and tell it to clean up after itself.

But in the above code, if Execute() had already finished, t is never destroyed. (tested with 3.0.4)

One would have to test for Finished in addition. Only that Finished could change between it being queried and any subsequent action. So the only solution I can see is maybe

   t := TThread.create(false); // create  not suspended
    // do something
    if foo then begin // decide we do not need the result
     t.Suspend;
     if t.Finished then begin
       t.free; // Not sure, since it is finished, we would not need to Terminate, WaitFor  or Resume ?
       t := nil;
     end
     else begin
       t.FreeOnTerminate:= true;
       t.terminate;
       t.Resume;
       t := nil; // no longer save to access
    end;
   end;

1) Is there an easier way?
2) Should the doc have any hint towards this?
3) Should FreeOnTerminate maybe detect this situation itself?

The question is why you need to set FreeOnTerminate after starting a thread? FreeOnTerminate is designed for threads which you start and forget about them because accessing the thread object is dangerous for such threads.
If you need to access the thread object or control the thread's lifetime never use FreeOnTerminate. To stop such thread use the code like this:

t.Terminate;
tm:=GetTickCount64;
while not t.Finished do
   begin
     sleep(10);
     // Check for timeout, the thread does not respond
     if GetTickCount64 - tm > 10000 then
       begin
         // Do something such as try to kill the thread
         break;
       end;
   end;
if t.Finished then
   t.Free;

The thread may makes calls (several, one after the other) into a library, and each of those calls may not return for some time (to long for the main thread to wait, without the app becoming unresponsive). And the structure of that library may not be possible to change.

The main thread launches the thread as a request that it will need the data.
The main thread will have several different such workers (or other sources, that will return a result after some time).

Normally the main thread will collect all the info. But if (due to user abort, or error in one info-source) the info is no longer required, then the request should be aborted.

Of course the main thread could keep all the pending-abort thread objects, wait for them to stop, and deal with it. That however is much more work, given that depending on what happens next, the entire infra structure of the main thread may need to be freed, or may need to be used for the next request (and then would need extra space to hold the pending-abort).

Since a thread can destroy itself, it is reasonable to make use of it. Once it is no longer needed, it can be abandoned.

----
Besides, the documentation does not say that FreeOnTerminate is limited to be used in the thread construction. Especially since its effect is not due until "terminate"

Further the name does *not* indicate, if "OnTerminate" means when "execute" exits (the thread actually terminates), or it means when "terminate" is called, or both.

The  documentation also does *not* say, that "FreeOnTerminate" is not thread safe (on a method of TThread you would expect thread safety, unless otherwise documented).
Yet if FreeOnTerminate is used while the thread is running, the result is unpredictable....

I am not saying that FreeOnTerminate has to be changed (I am asking if it might be useful to change it).
But if not changed, then probably the doc, wants to be much more specific.

For me this is a given.

Almost by definition, changing anything in a thread after the constructor has returned, is dangerous. You should set up everything in the constructor.

You can try to do so later on, but then you're on your own.

Michael.

Then this TThread.FreeOnTerminate property should not really be public, if it's only to be used in 'private' context of the constructor? (or as Martin says, it should be documented as such?)

You can try to do so later on, but then you're on your own. Michael.

el es

I think FreeOnTerminate should not even exist. IMO it should simply be 'True'.

But I did not invent TThread.

(or as Martin says, it should be documented as such?)

This is a valid point and I intend to elaborate the TThread documentation somewhat, with this in mind.

Michael.

This is where I strongly disagree, to me it should always be False, because having things free up themselves at unpredictable times is a recipe for disaster which already hit me badly.
For instance, if a thread is still running when a DLL unloads, you get deadlocks. Of if a thread finishes after the memory manager has been unloaded, you are in for a hellish shutdown.

All this leads me to believe that FreeOnTerminate should always be left to False, even if I can understand it's usefulness in some very specific cases.

The opposite problem is just as bad: you cannot guarantee that you can shut down a thread properly, because you have no idea of the state it is on.

All this leads me to believe that FreeOnTerminate should always be left to False, even if I can understand it's usefulness in some very specific cases.

And now we have come to the root of the problem:

Threads are unpredictable, and hence evil in programming :-)

Michael.

El 13/12/18 a les 20:36, Martin Frb ha escrit:

I think that TThread should have a method to kill it for such cases, using, e.g., pthread_cancel under unix or TerminateThread under windows (though the latter is not a very good option according to the documentation).

Another possible solution for your use case could be to use the OnTerminate callback and ignore the result if it's not your thread, e.g.

FMyThread:=TMyThread.Create(true);
;
FMyThread.Start;

....

procedure TForm1.MyTerminate(Sender:TObject);
var t:tthread;
begin
   if t=FMyThread then
   begin
      //do something with the thread results
     FMyThread:=nil;
   end;
   t.free;
end;

The when you want to ignore the result of the thread, you just set FMyThread to nil.
There's a problem though: when the application terminates and the thread is still running, it will call MyTerminate possibly causing a segfault :-/ A solution to that could be to keep a list of running threads so that you can set their OnTerminate to nil when terminating the application and remove the thread of the list in MyTerminate.

Bye

Am Fr., 14. Dez. 2018, 10:31 hat Michael Van Canneyt geschrieben:

I disagree, because then you could never do a WaitFor(). Or provide the thread with new data through a method (that fills a queue that the thread processes) without risking an access violation.

Regards,
Sven

Michael Van Canneyt wrote

> (or as Martin says, it should be documented as such?) This is a valid point and I intend to elaborate the TThread documentation somewhat, with this in mind.

I did have an error in my initial example:

Obviously "t.terminate" is not save after "t.FreeOnTerminate:= true; "
because by that time t may be dangling. The only thing save to do with
"t" after at that point is "t:=nil".

But that aside, I think there are 2 thinks that can be made part of the documentation (they may be obvious, but "obvious" here is a relative term, depending on the readers experience)

1)
A thread that has its FreeOnTerminate property set to "True" must not be accessed in any for by any other thread.

An exception to this rule, is access to the thread from inside a synchronize event, that comes from the very thread itself. This is because the while the synchronize event is executed the thread is on hold and can not be destroyed.

[optional]
This is also true for any thread that at any time had its FreeOnTerminate property set to "True", and then reverted it back to false (from within the thread itself, as this the thread itself is the only place that is still allowed access).
Again the exception is that the property can be reverted inside a synchronize event, in which case the thread can be accessed again after that.

2)
The FreeOnTerminate property is evaluated once the thread's Execute method is exited. This is *before* "procedure DoTerminate; virtual;" and the "OnTerminate: TNotifyEvent" are executed. [[According to google/stackoverflow, this is the same in Delphi]]

Document relation to property "Finished" / see below

----
For (2) I have a question/request

The current FPC implementation (3.0.4) is
     FreeThread := Thread.FFreeOnTerminate;  // property is evaluated /
can no longer be changed
     Result := Thread.FReturnValue;
     Thread.FFinished := True;  // Marked as finished
     Thread.DoTerminate;
     if FreeThread then
       Thread.Free;
Could "Thread.FFinished := True;"  be moved to the top? (potentially
together with   "Result := Thread.FReturnValue;" so the order of those 2
remains).

The reason is, that then (on a suspended thread) it would be save to change "FreeOnTerminate" if "Finished = False".

This should be ok, the doc
https://www.freepascal.org/docs-html/rtl/classes/tthread.finished.html explicitly states that when it is true "the thread is still cleaning up (calling OnTerminate, etc)."

This is why all my threads are written to be "stoppable". This requires a bit of thinking when writing them, but it's not that hard.

Like all powerful tools, if you use them without knowing them, they can hurt you badly.
But I would not prevent anyone from using a pillar drill just because I once cut through my finger because I was careless when using it...

This can be solved with the OnTerminate event. As long as you didn't get that, the thread is still alive.

Michael.

Agreed, but the manufacturer can maybe be persuaded to make a more safe drill.

Michael.

Am Fr., 14. Dez. 2018, 11:23 hat Michael Van Canneyt geschrieben:

That won't help if multiple threads are involved, because OnTerminate will always arrive on the main thread and a thread using the just terminated thread might be in the middle of a method call to that thread.

Also the argument regarding WaitFor still stands.

Regards,
Sven

Am Fr., 14. Dez. 2018, 11:26 hat Luca Olivetti
geschrieben:

These functions have the potential to leave locks in a locked state which would lead to deadlocks. Thus ccoperative exits are always preferable.

Regards,
Sven

Michael Van Canneyt wrote

And now we have come to the root of the problem: Threads are unpredictable, and hence evil in programming :-)

Aye, but the evil is necessary, for example if one has to a program that needs to do GUI AND handle background communication/protocol tasks...

Even with TTimer to fire up periodically, the one main context can only do _one_ thing at a time.

Michael.

el_es

Luca Olivetti wrote

I think that TThread should have a method to kill it for such cases, using, e.g., pthread_cancel under unix

pthread_cancel() does not not kill a thread. Conceptually, it is exactly the same as calling TThread.terminate: it just sets a property for the thread to indicate someone wants it to stop. The thread will only stop the next time pthread_testcancel() gets subseaquently called from inside this "cancelled" thread, which should only be done at well-defined points so the chances at leaving stray locks etc behind are minimalised.

Jonas

El 14/12/18 a les 18:08, Jonas Maebe ha escrit:

Oh, I'm not familiar with those APIs (that's why I use freepascal ;-), I just looked at the implementation of a thread class in vdr (http://www.tvdr.de/) documented this way:

   void Cancel(int WaitSeconds = 0);
        ///< Cancels the thread by first setting 'running' to false, so that
        ///< the Action() loop can finish in an orderly fashion and then
waiting
        ///< up to WaitSeconds seconds for the thread to actually end.
If the
        ///< thread doesn't end by itself, it is killed.
        ///< If WaitSeconds is -1, only 'running' is set to false and
Cancel()
        ///< returns immediately, without killing the thread.

and the method uses pthread_cancel. So the documentation of that class is misleading.
I also read the manual page for pthread_cancel and even when using pthread_canceltype(PTHREAD_CANCEL_AYNCHRONOUS) "The thread can be canceled at any time. (Typically, it will be canceled immediately upon receiving a cancellation request, but the system doesn't guarantee this.)".

Bye

Luca Olivetti wrote

I also read the manual page for pthread_cancel and even when using pthread_canceltype(PTHREAD_CANCEL_AYNCHRONOUS) "The thread can be canceled at any time.  (Typically, it will be canceled immediately upon receiving a cancellation request, but the system doesn't guarantee this.)".

This also depends on the operating system. Some systems will never asynchronously cancel a thread, and will only do so at cancellation points (= places where pthread_testcancel gets called). This is perfectly valid according to the standard.

Jonas