A few years ago I posted in this mailing list about my implementation of creating thread through a simple method. I'm not sure who copied it into the RTL, but I was checking on it today and it's wrong.

Wrong in the fact that's its just not usable given the copied design. Here is how it should work.

procedure TSomeForm.DoWork(Thread: TSimpleThread);
begin
  while not Thread.Terminated do
    // DoIntensiveWork suppossably returns some text message
    Thread.Status := DoIntensiveWork;
  Thread.Status := 'Work done!'
end;
procedure TSomeForm.DoWorkStatus(Sender: TObject);
begin
  // Safely do something with the thread status text
  StatusMemo.Lines.Add(Status);
end;
procedure TSomeForm.IntensiveButtonClick(Sender: TObject);
begin
  // Stop any previous work
  if FWork  nil then
    FWork.Terminate;
  // Start some work
  FWork := TSimpleThread.Create(DoWork, DoWorkStatus);
end;
procedure TSomeForm.FormDestroy(Sender: TObject);
begin
  // Clean up
  FWork.Free;
end;

So in a nutshell, you can define work to be done on a thread without the need to define a TThread derived class. Status is optional. Freeing is optional.

Here's the problems with the RTL version.

A) TThread.ExecuteInThread returns a TThread
B) The actual instance returned by ExecuteInThread is TSimpleStatusProcThread
C) User has to know to typecast the return
D) Type TSimpleStatusProcThread has no accessible methods or properties E) Status cannot be set, terminated cannot be checked, and no synchronization is available

If anyone cares to fix this, for reference here is my class declaration. Usage is as simple as, FWork := TSimpleThread.Create(DoWork, DoWorkStatus); with DoWorkStatus being optional.

{ TThreadExecuteMethod is the method invoked when a TSimpleThread is created } TThreadExecuteMethod = procedure(Thread: TSimpleThread) of object; { TSimpleThread allows objects to create a threading method without defining a new thread class See also <link Overview.Codebot.System.TSimpleThread, TSimpleThread members> } TSimpleThread = class(TThread) private FExecuteMethod: TThreadExecuteMethod; FTempStatus: string; FStatus: string; FOnStatus: TNotifyEvent; procedure DoStatus; procedure SetStatus(const Value: string); protected { Sets FreeOnTerminate to True and executes the method } procedure Execute; override; public { Create immediately starts ExecuteMethod on a new thread } constructor Create(ExecuteMethod: TThreadExecuteMethod; OnStatus: TNotifyEvent = nil; OnTerminate: TNotifyEvent = nil); { Synchronize can be used by ExecuteMethod to invoke a method on the main thread } procedure Synchronize(Method: TThreadMethod); { You should only set status inside ExecuteMethod } property Status: string read FStatus write SetStatus; { Terminated is set to True after the Terminate method is called } property Terminated; end;

At the time I submitted my work and you OK-ed it.

Michael.

My bad. I think I looked at the declaration of the method signature and no further, so it's my bad.

Basically what needs to happen at least is that Status can be set, Synchronize should be accessible, and Terminated should be public.

Nicer to have would be the actual thread class is returned such that the user doesn't need to know he must typecast. Setting of Status outside of the thread context should be ignored or not allowed, reading Status should be guarded, and OnStatus/OnTermiante should execute in the context of the main thread.

I can submit an issue/patch if you want, as well as a test.

A follow up ... I think the best way to handle everything I mentioned is to just create an instance of this thread type, rather than using a class method on TThread.

Anthony Walter wrote

A follow up ... I think the best way to handle everything I mentioned is to just create an instance of this thread type, rather than using a class method on TThread.

Exactly.

The class methods are intended to be used as quick fire-and-forget methods.

There are some parametrizations possible by choosing the right overload, but you obviously do not get the full options you would have when you actually use a class.

I think that if you need more flexibility than these methods provide, you should simply write a TThread class.

Michael.

Okay, but the problem with the current implementation in Classes.pas is that Status cannot be used, Terminated cannot be checked, and there is no access to Synchronize. As such OnStatus is pointless, and the semantics of checking Terminated doesn't work. If that's how the these ExecuteInThread class methods on TThread are going work then they should be removed because they don't have significant function and likely would confuse users.

Eg. of confusion when trying to use ExecuteInThread Q: How does OnStatus work? A: It doesn't. Q: How can I check if my thread method is terminated? A: Forget about Thread.Terminated, you'll need to set a flag somewhere or create you own solution. Q: How can I suspend my method and safely join the main thread, because I normally use Synchronize? A: You'll have to do without Synchronize or maybe figure out a workaround.

So IMO ExecuteInThread should either be removed or these problems should be fixed.

Anthony Walter wrote

Okay, but the problem with the current implementation in Classes.pas is that Status cannot be used, Terminated cannot be checked, and there is no access to Synchronize.

All wrong. See below.

And why doesn't it ?

You must obviously pass a callback. Just as in asynchronous programming. Of course your function must call it from time to time.

Q: How can I check if my thread method is terminated?

By passing aOnTerminate ?

If you mean inside the thread, use TThread.CheckTerminated. This is a class function.

or use TThread.CurrentThread.Terminated;

A: Forget about Thread.Terminated, you'll need to set a flag somewhere or create you own solution. Q: How can I suspend my method and safely join the main thread, because I normally use Synchronize? A: You'll have to do without Synchronize or maybe figure out a workaround.

You can perfectly call Synchronize. It is a class method:

Procedure MyThreadHandlerFunction ;
begin
   TThread.Synchronize(CurrentThreead,SomeMethod);
end;

So IMO ExecuteInThread should either be removed or these problems should be fixed.

IMO you should check the documentation a little more thorough.

https://www.freepascal.org/docs-html/current/rtl/classes/tthread.executeinthread.html

It contains 4 examples that show how things work.

Michael.