Why this thread never end?

[Edson]

Maybe I'm doing something wrong. This code ends normally the most of times, but some times it never ends.

Code: Pascal  [Select][+][-]

1. unit Unit1;
2. {$mode objfpc}{$H+}
3. interface
4. uses
5.   Classes, SysUtils, Forms, Controls, LCLProc;
6. type
7.   TForm1 = class(TForm)
8.     procedure FormCreate(Sender: TObject);
9.     procedure FormDestroy(Sender: TObject);
10.     procedure tTerminate(Sender: TObject);
11.   end;
12.  
13.   TMyThread = class(TThread)
14.   protected
15.     procedure Execute; override;
16.   public
17.     constructor Create;
18.   end;
19.  
20. var
21.   Form1: TForm1;
22.   t : TMyThread;
23.   Killed : boolean;
24.  
25. implementation
26. {$R *.lfm}
27. constructor TMyThread.Create;
28. begin
29.   FreeOnTerminate := True;
30.   inherited Create(false);
31. end;
32.  
33. procedure TMyThread.Execute;
34. begin
35.   sleep(3000);
36. end;
37.  
38. procedure TForm1.tTerminate(Sender: TObject);
39. begin
40.   Killed := true;
41. end;
42.  
43. procedure TForm1.FormCreate(Sender: TObject);
44. begin
45.   Killed := false;
46.   t := TMyThread.Create;
47.   t.OnTerminate:=@tTerminate;
48. end;
49.  
50. procedure TForm1.FormDestroy(Sender: TObject);
51. begin
52.   if not Killed then begin
53.     t.Terminate;
54.     t.WaitFor;
55.     Application.MessageBox('I was alive. Bye ...','');
56.   end;
57. end;
58. end.
59.  

Does someone have any idea?

EDIT:
It happens, when the form is closed before 3 seconds.

[aidv]

What exactly happens when you destroy the form before 3 seonds?

Are you sure you're actually destroying the form?

What are you waiting for when calling "t.WaitFor;"?

And for the grand finalé: I think that the problem is "sleep(3000)".

When "sleeping", then thread does no process any calls, it's more or less halted,
therefor "t.Terminate" might not work.

I think, I am not sure, but I think that that is how it works.

[Edson]

What exactly happens when you destroy the form before 3 seonds?

In sometimes, the program ends showing the message "I was alive. Bye "
In other times (10% to 20%), the form freezes. The program never ends. I have to stop it from Lazarus.

Are you sure you're actually destroying the form?

Yes. The event FormDestroy is executed always.

What are you waiting for when calling "t.WaitFor;"?

Wait for the thread ends.

[cdbc]

Hi

Code: Pascal  [Select][+][-]

1. constructor TMyThread.Create;
2. begin
3.   FreeOnTerminate := True;
4.   inherited Create(false); // inherited Create(true);
5. end;

Change this and this:

Code: Pascal  [Select][+][-]

1. procedure TForm1.FormCreate(Sender: TObject);
2. begin
3.   Killed := false;
4.   t := TMyThread.Create(true);
5.   t.OnTerminate:=@tTerminate;
6.   T.Start;
7. end;

Try this, it should work :-)
Regards Benny

[kapibara]

I noticed that you have the constructor of TMyThread start the thread immediately:

Code: Pascal  [Select][+][-]

1. inherited Create(false);

My guess is that if you end the program before the 3 second sleep is over, then OnTerminate will be unassigned and never called, so Killed will never become True.

So, since you are assigning an event handler for the OnTerminate event you should create the thread in a suspended state, assign the handler and then start the thread yourself:

Code: Pascal  [Select][+][-]

1. inherited Create(true);

Code: Pascal  [Select][+][-]

1. procedure TForm1.FormCreate(Sender: TObject);
2. begin
3.   Killed := false;
4.   t := TMyThread.Create;
5.   t.OnTerminate:=@tTerminate;
6.   t.Start;
7. end;

EDIT: Benny was faster. :-)

EDIT2: Why did the app hang? Was it because t.Terminate was called but not assigned? If the thread is finished, WaitFor returns at once. You could make a test:

Code: Pascal  [Select][+][-]

1. procedure TForm1.FormDestroy(Sender: TObject);
2. begin
3. if not Killed then begin
4.   if assigned(t.OnTerminate) then
5.     t.Terminate
6.   else
7.     Application.MessageBox('Gee, I was not assigned!','');
8.   t.WaitFor;
9.   Application.MessageBox('I was alive. Bye ...','');
10. end;

[Edson]

Hi @cdbc, @kapibara.

I have created the thread suspended, like you suggested, but the result is the same. 

In fact, The original code was exactly like you suggested. I just simplified it just for test.

I have verified that the program hangs (when it hangs) in "WaitFor".

EDIT: I have verified too, that the desctructor of the thread always execute. The problem come later in "WaitFor".

[rvk]

Mmm, I might see a few potential problems here.

First... you have an Execute which will only do a sleep(3000); So your thread WILL terminate after 3 seconds. The OnTerminate will be executed. If you want the thread to wait you need to build in a loop. But after 3 seconds OnTerminate will execute and you Killed will be set to true.

Next... you have set FreeOnTerminate to true. This will destroy the thread right after the t.Terminate (i.e. OnTerminate). You can't use t.WaitFor after the destruction of t. I'm not sure how it is here on timing but using FreeOnTerminate = true is very dangerous if you want to do a waitfor like this.

Try removing the FreeOnTerminate (or set it to false) and call t.free after the t.WaitFor;

[eny]

Code: Pascal  [Select][+][-]

1. unit ufrmMain;
2.  
3. {$mode objfpc}{$H+}
4.  
5. interface
6.  
7. uses
8.   Classes, SysUtils, FileUtil, Forms, Controls, Graphics, Dialogs, StdCtrls;
9.  
10. type
11.  
12.   { TMyThread }
13.  
14.   TMyThread = class(TThread)
15.   protected
16.     procedure Execute; override;
17.   public
18.     constructor Create;
19.   end;
20.  
21.   { TfrmMain }
22.  
23.   TfrmMain = class(TForm)
24.     Button1: TButton;
25.     procedure FormCreate(Sender: TObject);
26.     procedure FormDestroy(Sender: TObject);
27.   private
28.     { private declarations }
29.     thrd: TMyThread;
30.   end;
31.  
32.  
33. var
34.   frmMain: TfrmMain;
35.  
36. implementation
37.  
38. {$R *.lfm}
39.  
40. { TMyThread }
41.  
42. constructor TMyThread.Create;
43. begin
44.   // Do nót free on terminate
45.   inherited Create(false);
46. end;
47.  
48. procedure TMyThread.Execute;
49. const C_DURATION = 3000;
50.       C_MSEC     = 100;
51. var iSleep: integer;
52. begin
53.   // Do some waiting...
54.   iSleep := C_DURATION div C_MSEC;
55.   while (iSleep > 0) and not Terminated do
56.   begin
57.     sleep(C_MSEC);
58.     dec(iSleep)
59.   end;
60. end;
61.  
62. { TfrmMain }
63.  
64. procedure TfrmMain.FormCreate(Sender: TObject);
65. begin
66.   thrd := TMyThread.Create;
67. end;
68.  
69. procedure TfrmMain.FormDestroy(Sender: TObject);
70. begin
71.   thrd.Terminate;
72.   thrd.WaitFor;
73.   thrd.Free;
74. end;
75.  
76. end.

[Edson]

Hi @rvk, @eny

Using  FreeOnTerminate=FALSE, solves the problem, like you suggested. 

It seem's it's no secure to use FreeOnTerminate=TRUE for controlling the end of a thread.

Is it necessary to ensure all the threads are finished when leaving the main program?

Thanks.

[garlar27]

Try not to use Sleep() in threads, it eats the resources just to do nothing. Instead use

RTLeventWaitFor(FThreadRTLEvent); // FThreadRTLEvent : PRTLEvent;
or
RTLeventWaitFor(FThreadRTLEvent, ATimeOut);

to sleep a thread

and
   RTLeventSetEvent(FThreadRTLEvent);

to wake up the thread
Search for using threads in the wiki you'll find more examples and a good explanation.

[kapibara]

Is it necessary to ensure all the threads are finished when leaving the main program?

Thats the question. Threads that would try to use a resource that was destroyed when the main program ended definitely have to be prevented somehow. It might well be the simplest, and good practice, to terminate threads before allowing the app to be closed. Has anyone heard about a hard a fast rule here?

One time I wanted to know what happened inside a thread while it was running, so I wrote a callback that updated the GUI. When the app was closed, the thread sometimes used the callback so sometimes there was an access violation. I solved this by terminating all such threads before letting the app close. First one loop calling terminate on all threads, then a second loop calling WaitFor on all. It was agreed on StackOverflow that this is how you wait for multiple threads under Linux. Under Windows, I think you can use WaitForMultipleObjects". (Doesn't exist under Linux).

[Edson]

Try not to use Sleep() in threads, it eats the resources just to do nothing.

I believed sleep() was more intelligent. I haven't seen the code generated. Anyway it's a good suggestion.

One time I wanted to know what happened inside a thread while it was running, so I wrote a callback that updated the GUI. When the app was closed, the thread sometimes used the callback so sometimes there was an access violation.

I made a test(*), closing the main program, before the thread ends (in Windows). The result was that the thread seems never finishes, like if it went away to the heaven.

(*) My test consisted in a simple thread, with a sleep() and a routine to create a file after the delay. The file was never created.

[rvk]

I made a test(*), closing the main program, before the thread ends (in Windows). The result was that the thread seems never finishes, like if it went away to the heaven.

(*) My test consisted in a simple thread, with a sleep() and a routine to create a file after the delay. The file was never created.

You should always build in a loop and test for Terminated. I think (am not sure) the thread is signaled to terminate when the main program ends. But it's up to the thread to actually terminate.

So something like eny already showed you:
http://forum.lazarus.freepascal.org/index.php/topic,29877.msg189492.html#msg189492

Never just only have sleep(100000000000);

Code: Pascal  [Select][+][-]

1. procedure TMyThread.Execute;
2. const C_DURATION = 3000;
3.       C_MSEC     = 100;
4. var iSleep: integer;
5. begin
6.   // Do some waiting...
7.   iSleep := C_DURATION div C_MSEC;
8.   while (iSleep > 0) and not Terminated do
9.   begin
10.     sleep(C_MSEC);
11.     dec(iSleep)
12.   end;
13. end;

[kapibara]

Like Garlar27 suggested, using an RTLevent to pause/unpause the threads is more flexible. Then you have control when to UnPause and can free the threads without much delay. Sleep()'ing threads can't be gotten rid of until they wake up themselves.

Trace through the Pause and UnPause procedures of TManagerThread below for an example. (Disregard the other code, I just used it for some test). Note that it is in the loop of the threads Execute procedure that the pausing actually takes place by reacting to the msPaused flag.

Code: Pascal  [Select][+][-]

1. unit uThreads;
2.  
3. {$mode objfpc}{$H+}
4.  
5. interface
6.  
7. uses
8.   Classes, SysUtils, uThreadjob;
9.  
10. type
11.   TManagerStatus = (msNotStarted, msRunning, msPaused);
12.  
13.   { TWorkerThread }
14.  
15.   TWorkerThread = class(TThread)
16.   private
17.     FThreadJob: TThreadJob;
18.   protected
19.     procedure Execute; override;
20.   public
21.     property Job: TThreadJob read FThreadJob write FThreadJob;
22.   end;
23.  
24.   { TManagerThread }
25.  
26.   TManagerThread = class(TThread)
27.   private
28.     FStatus: TManagerStatus;
29.     FPauseEvent: PRTLEvent;
30.     FJobNumber: Integer;
31.   protected
32.     procedure Execute; override;
33.   public
34.     constructor Create;
35.     procedure Pause;
36.     procedure UnPause;
37.     property  Status: TManagerStatus read FStatus;
38.   end;
39.  
40.   var
41.     ManagerThread: TManagerThread;
42.  
43. implementation
44.  
45. uses
46.   uMain, uMyResource;
47.  
48. { TManagerThread }
49.  
50. procedure TManagerThread.Execute;
51. var
52.   aJobItem: TJobItem;
53.   aThreadJob: TThreadJob;
54.   aWorkerThread: TWorkerThread;
55. begin
56.   FStatus:= msRunning;
57.   FJobNumber:= 1;
58.   while not Terminated do
59.   begin
60.     if Status = msPaused then
61.       RTLeventWaitFor(FPauseEvent);
62.  
63.     aJobItem:= TJobItem.Create('Job ' +IntToStr(FJobNumber));
64.  
65.     aThreadJob:= TThreadJob.Create(FJobNumber);
66.     aThreadJob.JobItem:= aJobItem;
67.  
68.     aWorkerThread:= TWorkerThread.Create(True);
69.     aWorkerThread.Job:= aThreadJob;
70.     aWorkerThread.Start;
71.  
72.     Inc(FJobNumber);
73.     Sleep(Random(500));//Wait a while before starting new job
74.   end;
75. end;
76.  
77. constructor TManagerThread.Create;
78. begin
79.   inherited Create(True);
80.   FPauseEvent:= RTLEventCreate;
81.   FStatus:= msNotStarted;
82. end;
83.  
84. procedure TManagerThread.Pause;
85. begin
86.   FStatus:= msPaused;  //Execute loop must call waitfor, else gui freeze
87. end;
88.  
89. procedure TManagerThread.UnPause;
90. begin
91.   RTLeventSetEvent(FPauseEvent);
92.   FStatus:= msRunning;
93. end;
94.  
95. { TWorkerThread }
96.  
97. procedure TWorkerThread.Execute;
98. begin
99.   FreeOnTerminate:=True;
100.   if not Terminated then
101.   begin
102.     Job.Execute;
103.     Job.JobItem.Free;
104.     Job.Free;
105.     Terminate;
106.   end;
107. end;
108.  
109. initialization
110.   Randomize;
111.   ManagerThread:= TManagerThread.Create;
112.  
113. end.