unit Unit1;
{$mode objfpc}{$H+}
interface
uses
Classes, SysUtils, FileUtil, Forms, Controls, Graphics, Dialogs, StdCtrls;
type
{ TMyThread }
TMyThread = class(TThread)
protected
procedure Execute; override;
public
constructor Create(CreateSuspended: boolean);
end;
type
{ TForm1 }
TForm1 = class(TForm)
Button1: TButton;
procedure Button1Click(Sender: TObject);
procedure FormClose(Sender: TObject; var CloseAction: TCloseAction);
private
{ private declarations }
T: TMyThread;
public
{ public declarations }
end;
var
Form1: TForm1;
terminate_thread : boolean ; => here added
implementation
{ TForm1 }
procedure TForm1.Button1Click(Sender: TObject);
begin
T := TMyThread.Create(True);
terminate_thread := false; => here added
T.Start;
end;
procedure TForm1.FormClose(Sender: TObject; var CloseAction: TCloseAction);
begin
if t <> nil then terminate_thread := true; => here added
end;
{$R *.lfm}
{ TMyThread }
procedure TMyThread.Execute;
begin
repeat
Sleep(60);
until terminated_var = True;
terminate;
end,
end;
constructor TMyThread.Create(CreateSuspended: boolean);
begin
FreeOnTerminate := True;
inherited Create(CreateSuspended);
end;
end.
procedure TForm1.FormClose(Sender: TObject; var CloseAction: TCloseAction);
begin
if t <> nil then
begin
T.Terminate;
Sleep(120)
end;
end;
until Terminated = True;
but writeuntil Terminated;
unit Unit1;
{$mode objfpc}{$H+}
interface
uses
Classes, SysUtils, FileUtil, Forms, Controls, Graphics, Dialogs, StdCtrls;
type
{ TMyThread }
TMyThread = class(TThread)
protected
procedure Execute; override;
public
constructor Create(CreateSuspended: boolean);
destructor Destroy; override;
end;
type
{ TForm1 }
TForm1 = class(TForm)
Button1: TButton;
procedure Button1Click(Sender: TObject);
procedure FormClose(Sender: TObject; var CloseAction: TCloseAction);
procedure FormCloseQuery(Sender: TObject; var CanClose: boolean);
private
{ private declarations }
T: TMyThread;
public
{ public declarations }
end;
var
Form1: TForm1;
implementation
{ TForm1 }
procedure TForm1.Button1Click(Sender: TObject);
begin
T := TMyThread.Create(False); // If you don't start it suspended, no need to call Start
//T.Start;
end;
procedure TForm1.FormClose(Sender: TObject; var CloseAction: TCloseAction);
begin
FreeAndNil(T); // This is good because of no FreeOnTerminate
// But it could be moved to onDestroy rather, or right after T.WaitFor; for clarity
end;
procedure TForm1.FormCloseQuery(Sender: TObject; var CanClose: boolean);
begin
if t <> nil then begin
T.Terminate;
T.WaitFor; // Wait for thread to close
end;
//CanClose:=True; // Actually this is true by default already...
end;
{$R *.lfm}
{ TMyThread }
procedure TMyThread.Execute;
begin
repeat
Sleep(60);
until Terminated;
end;
constructor TMyThread.Create(CreateSuspended: boolean);
begin
FreeOnTerminate := False; // <-- Not always a good idea to have this True
// .. because you have a case where thread could be running while app tries to exit.
inherited Create(CreateSuspended);
end;
destructor TMyThread.Destroy;
begin
inherited Destroy;
end;
end.
WaitForThreadTerminate
T.WaitFor; // Wait for thread to close
Ooops, i did not know that, so no need for sleep(?) . (always difficult to know how many time is needed..) :oThat's why it's easier not to auto-free but call Terminate in combination with WaitFor.
As user137 suggested it might be easier to just not auto-free your thread but do it by code.
That way you can use T.WaitFor.
It's still not perfect... How about this?It seems to me that calling FreeAndNil is sufficient because it leads to WaitFor inside TThread.SysDestroyCode: [Select]..
procedure TForm1.FormClose(Sender: TObject; var CloseAction: TCloseAction);
begin
FreeAndNil(T); // This is good because of no FreeOnTerminate
..
procedure TForm1.FormCloseQuery(Sender: TObject; var CanClose: boolean);
begin
if t <> nil then begin
T.Terminate;
T.WaitFor; // Wait for thread to close
end;
..
Using sleep() is Always a bit of a gamble, but in TS's case not so much because he used a sleep in the thread without doing anything else
Do you mean that i can use WaitFor only if the thread has FreeOnTerminate := false ?You can also use it in case of FreeOnTerminate = true.
Eny: I don't want to start the thread as soon as I create it, because I have planned to pass to some parameters to my thread.I never said you should :)
It seems to me that calling FreeAndNil is sufficient because it leads to WaitFor inside TThread.SysDestroyCalling FreeAndNil() on a running Thread will lead to problems.
Calling FreeAndNil() on a running Thread will lead to problems.That what I thought until I checked the source code:
procedure TThread.SysDestroy;
begin
if FHandle<>0 then
begin
{..}
if not FFinished {and not Suspended} then
begin
Terminate;
{..}
if FInitialSuspended then
Start;
WaitFor;
end;
end;
FFatalException.Free;
FFatalException := nil;
end;
As you can see it calls Terminate and WaitFor. Maybe you have some specific objection to using FreeAndNil on a TThread?
The complexity lies in the fact that you must make sure the thread has not yet freed itself yet, when you call Waitfor.
if assisgned(MyTreads[x])
.If MyTreads[x].create() then MyArrayShortint[x] := 1.
If MyTreads[x].terminate then MyArrayShortint[x] := -1.
if MyArrayShortint[x] = 1 then MyArrayTreads[x].free....
And, before to try to free a member of array, i checkHow do you know that the execution will not shift to the thread you are trying to free exactly after it passed this check.Code: [Select]if MyArrayShortint[x]...
How do you know that the execution will not shift to the thread you are trying to free exactly after it passed this check.
TMyThread = class(TThread)
protected
procedure Execute; override;
public
isterminated :booelan; ====> this one
constructor Create(CreateSuspended: boolean);
destructor Destroy; override;
end;
procedure TMyThread.Execute;
begin
repeat
Sleep(60);
until terminated_var = True;
isterminated := true; ===> here
terminate;
end,
end;
As you can see it calls Terminate and WaitFor. Maybe you have some specific objection to using FreeAndNil on a TThread?My objection is that the program will crash horribly when calling FreeAndNil on a running thread.
My objection is that the program will crash horribly when calling FreeAndNil on a running thread.It seems to me that having FreeOnTerminate = true caused the crash after calling FreeAndNil.
Try the simple example TS gave and instead of terminate/waifor, call FreeAndNil.Could you change his TMyThread.Create to make sure FreeOnTerminate := False and check again.
Could be related to the usage of sleep though.
if t <> nil then
T.Terminate;
if t <> nil
Is not really useful, =>will always be true. if t <> nil then
T.Terminate;
Will give a error and a big boum. :-XNot if someone used FreeAndNil(t) or initialized t to nil in the form constructor. But again, I'm wondering about problems related to using FreeAndNil to terminate a thread gracefully. I did notice that I can not use it when FreeOnTerminate is true.Code: [Select]if t <> nil
Is not really useful, =>will always be true.
..or initialized t to nil in the form constructor...I have read somewhere that instance variables (e.g. form variables) are initialized to zero (the object instance is filled with zeroes via a call to fillchar).
I have read somewhere that instance variables (e.g. form variables) are initialized to zero
... using free or freeandnil will just free the warper object and will create a complete mess!Please correct me if I'm wrong. As long as FreeOnTerminate = false it is perfectly OK to use Free or FreeAndNil, and here is why:
procedure TObject.Free;
begin
// the call via self avoids a warning
if self<>nil then
self.destroy;
end;
destructor TThread.Destroy;
begin
if not FExternalThread or False then begin
SysDestroy;
..
inherited Destroy;
end;
procedure TThread.SysDestroy;
begin
if FHandle<>0 then
begin
{ Don't check Suspended. If the thread has been externally suspended (which is
deprecated and strongly discouraged), it's better to deadlock here than
to silently free the object and leave OS resources leaked. }
if not FFinished {and not Suspended} then
begin
Terminate;
{ Allow the thread function to perform the necessary cleanup. Since
we've just set Terminated flag, it won't call Execute. }
if FInitialSuspended then
Start;
WaitFor;
end;
end;
FFatalException.Free;
FFatalException := nil;
end;
procedure TThread.SysDestroy;
begin
..
// if someone calls .Free on a thread with not(FreeOnTerminate), there
// is no problem. Otherwise, FreeOnTerminate must be set to false so
// when ThreadFunc exits the main runloop, it does not try to Free
// itself again
FFreeOnTerminate := false;
{ you can't join yourself, so only for FThreadID<>GetCurrentThreadID }
{ and you can't join twice -> make sure we didn't join already }
if not FThreadReaped then
begin
Terminate;
if (FSuspendedInternal or FInitialSuspended) then
Resume;
WaitFor;
end;
end;
CurrentTM.SemaphoreDestroy(FSem);
..
end;
I have read somewhere that instance variables (e.g. form variables) are initialized to zero (the object instance is filled with zeroes via a call to fillchar).I confirm seeing that in the source code:
class function TObject.InitInstance(instance : pointer) : tobject; {$ifdef SYSTEMINLINE} inline; {$ENDIF}
begin
{ the size is saved at offset 0 }
fillchar(instance^, InstanceSize, 0);
...
end;
@ engkin :I agree with that. Where did you see me claiming the opposite?
when you call a method of TThread Instance it's executed in the current context NOT in the thread!!!
i repeat once more the TThread Class is just a wapper around OS or lib (dll) function that actually manages the threads on the computer.I agree with this one as well.
the thread are NOT pascal objects that you can manage ...Hmmm, it depends on what you mean by "manage".
freeandnil, destroy and free just deallocates the memory in the heap and runs some cleanup in the pascal hierarchy of classes...Obviously you did not read the source code I quoted above. FreeAndNil, Free, and Destroy do wait for the thread to terminate itself (by exiting its Execute method) before deallocating the memory in the heap and running cleanup code.
they cannot terminate a running. an OS thread terminates by exiting its main proc.Hopefully that happens when it checks its FTerminated which holds the value True after we call Terminate, Free, FreeAndNil, or Destroy from another thread.
thread all the methods must be called within the thread it self.. the code that you quoted is just fine if called from the thread it self..Just to be precise, you can call any of these methods from another thread, and yes they do not terminate the thread. They simply indicate that the thread needs to terminate itself by setting FTerminated to true and then they wait for it to happen using WaitFor.
So, you cannot terminate the thread from another thread
Constructor TMyGoodEndThread.Create();
begin
FreeOnTerminate := True;
...
end;
procedure TMyGoodEndThread.Execute;
begin
...
Terminate;
Waitfor;
end;
Constructor TMyGoodEndThread.Create();
begin
FreeOnTerminate := false;
...
end;
procedure TMyGoodEndThread.Execute;
begin
...
Terminate;
Waitfor;
FreeAndNil;
end;
Constructor TMyGoodEndThread.Create();
begin
FreeOnTerminate := False;
...
end;
procedure TMyGoodEndThread.Execute;
begin
...
Terminate;
Waitfor;
Destroy;
end;
... ?..
You don't call any of that in Thread.Execute.
Constructor TMyGoodEndThread.Create();
begin
FreeOnTerminate := True;
...
end;
procedure TMyGoodEndThread.Execute;
begin
while not terminated do begin
...
end;
end;
Constructor TMyGoodEndThread.Create();
begin
FreeOnTerminate := False;
...
end;
procedure TMyGoodEndThread.Execute;
begin
while not terminated do begin
...
end;
end;
procedure TForm1.CreateThread;
begin
thread:=TMyGoodEndThread.Create(false);
end;
procedure TForm1.CloseThread;
begin
FreeAndNil(thread);
end;
var
TerminateMyThread : boolean = false;
...
Constructor TMyGoodEndThread.Create();
begin
FreeOnTerminate := True;
...
end;
procedure TMyGoodEndThread.Execute;
begin
while TerminateMyThread = false do
begin
...
end;
Terminate;
WaitFor;
end;
with freeonterminate do not call waitfor or terminate ...
var
TerminateMyThread : boolean = false;
...
Constructor TMyGoodEndThread.Create();
begin
FreeOnTerminate := True;
...
end;
procedure TMyGoodEndThread.Execute;
begin
while TerminateMyThread = false do
begin
...
end;
Terminate;
end;
with freeonterminate do not call waitfor or terminate ...
with freeonterminate do not call waitfor or terminate
var
TerminateMyThread : boolean = false;
...
Constructor TMyGoodEndThread.Create();
begin
FreeOnTerminate := True;
...
end;
procedure TMyGoodEndThread.Execute;
begin
while TerminateMyThread = false do
begin
...
end;
...
///// => not needed "terminate" ?
end;
procedure TThread.Terminate;
begin
FTerminated := True;
end;
You don't need additional global variables for that purpose. You CAN use Terminate when freeonterminate is true. It will just instruct it to stop, and it will not terminate instantly. But you must be sure that the thread even exists while calling that Terminate, otherwise you could get SIGSEGV (calling class object function for nil object).
FreeOnTerminate := True;
then, when procedure TMyGoodEndThread.Execute;
arrives at end; the thread is automatically terminated, waitfor and freedandnil too.FreeOnTerminate := false;
then, when procedure TMyGoodEndThread.Execute;
arrives at end; the thread will terminate but waitfor and freedandnil should be called to avoid memory leak.So, to resume, ifRoughly yes. Terminated status will be true, and because of that, waitfor would do nothing even if it is called (propably is not called). Object is destroyed from memory but it doesn't automatically set any variable referring to it to nil. That you have to do yourself at the end of Execute (for example global variable "form1.thread:=nil;" if you use such... don't always have to use.).Code: [Select]FreeOnTerminate := True;
then, whenCode: [Select]procedure TMyGoodEndThread.Execute;
arrives at end; the thread is automatically terminated, waitfor and freedandnil too.
But ifYeah, again it is terminated, and it is your responsibility to free the thread. Again waitfor would do nothing on a terminated thread. The Waitfor is there to wait until the thread has finished its Execute method. You can call Start to reuse the same thread multiple times if you want aswell.Code: [Select]FreeOnTerminate := false;
then, whenCode: [Select]procedure TMyGoodEndThread.Execute;
arrives at end; the thread will terminate but waitfor and freedandnil should be called to avoid memory leak.
Hum, there is still a little shadow in that bright light...In theory you could define another instance variable that would be a pointer to the variable that contains the thread reference.
Does it exist ?: FreeAndNilOnTerminate := True;... :-[
PTMyThread = ^TMyThread;
TMyThread = class(TThread)
private
FThreadRef: PTMyThread;
public
//....
end;
about setting the onTerminate event and do the clean up in that event?
In theory you could define another instance variable that would be a pointer
(but is it safe to set to nil the thread by himself ?)
type
TMyGoodEndThread = class(TThread)
protected
...
procedure onTerminate;
...
public
constructor Create(CreateSuspended: boolean;
const StackSize: SizeUInt = DefaultStackSize);
...
end;
constructor TMyGoodEndThread.Create(CreateSuspended: boolean;
const StackSize: SizeUInt);
begin
inherited Create(CreateSuspended, StackSize);
FreeOnTerminate := false;
...
end;
procedure TMyGoodEndThread.onTerminate() ;
begin
FreeAndNil(self);
end;
Which variable the freeandnil(self) sets to nil?
type
TMyGoodEndThread = class(TThread)
protected
...
procedure onTerminate; =====> here added
...
public
Index: cardinal; ====> here added
constructor Create(CreateSuspended: boolean;
const StackSize: SizeUInt = DefaultStackSize);
...
end;
type
TArrayGoodEndThread = array of TMyGoodEndThread;
var
MyArrayGoodThread : TArrayGoodEndThread ;
constructor TMyGoodEndThread.Create(CreateSuspended: boolean;
const StackSize: SizeUInt);
begin
inherited Create(CreateSuspended, StackSize);
FreeOnTerminate := false;
...
end;
procedure CreateArrayGoodEndThread(indexarray :cardinal);
begin
setlength(MyArrayGoodThread, indexarray);
MyArrayGoodThread[indexarray] := TMyGoodEndThread.Create(true);
MyArrayGoodThread[indexarray].index := indexarray; =====> here added
end;
procedure TMyGoodEndThread.onTerminate() ;
begin
FreeAndNil(MyArrayGoodThread[index]);
end;
setlength(MyArrayGoodThread, indexarray+1);
2) You allocate 1 too few ;D Should be:
Code: [Select]
setlength(MyArrayGoodThread, indexarray+1);
Repor
if indexarray+1 > length(MyArrayGoodThread) then setlength(MyArrayGoodThread, indexarray+1);
;D ;D1) Can you delete indexes from between? If you move indexes backwards (to fill the empty gap of deleted thread), you have to update the index value within each thread that are moved too.