Lazarus

Programming => LCL => Topic started by: BlueIcaro on February 16, 2014, 08:42:13 pm

Title: Playing with thread, memory leaks
Post by: BlueIcaro on February 16, 2014, 08:42:13 pm
Hi, I started to play with Thread I did a small example, following the article in the wiki http://wiki.lazarus.freepascal.org/Multithreaded_Application_Tutorial (http://wiki.lazarus.freepascal.org/Multithreaded_Application_Tutorial)

But I got a leak memory when I close the program. The memory leaks points to line 45
Code: Pascal  [Select]
  1. T := TMyThread.Create(True);
  2.  

Here is my code:
Code: Pascal  [Select]
  1. unit Unit1;
  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(CreateSuspended: boolean);
  19.   end;
  20.  
  21. type
  22.  
  23.   { TForm1 }
  24.  
  25.   TForm1 = class(TForm)
  26.     Button1: TButton;
  27.     procedure Button1Click(Sender: TObject);
  28.     procedure FormClose(Sender: TObject; var CloseAction: TCloseAction);
  29.   private
  30.     { private declarations }
  31.     T: TMyThread;
  32.   public
  33.     { public declarations }
  34.   end;
  35.  
  36. var
  37.   Form1: TForm1;
  38.  
  39. implementation
  40.  
  41. { TForm1 }
  42.  
  43. procedure TForm1.Button1Click(Sender: TObject);
  44. begin
  45.   T := TMyThread.Create(True);
  46.   T.Start;
  47. end;
  48.  
  49. procedure TForm1.FormClose(Sender: TObject; var CloseAction: TCloseAction);
  50. begin
  51.   if t <> nil then
  52.     T.Terminate;
  53. end;
  54.  
  55.  
  56.  
  57. {$R *.lfm}
  58.  
  59. { TMyThread }
  60.  
  61. procedure TMyThread.Execute;
  62. begin
  63.   repeat
  64.     Sleep(60);
  65.   until Terminated = True;
  66.  
  67. end;
  68.  
  69. constructor TMyThread.Create(CreateSuspended: boolean);
  70. begin
  71.   FreeOnTerminate := True;
  72.   inherited Create(CreateSuspended);
  73.  
  74. end;
  75.  
  76. end.
  77.  

Any idea?

Thanks

/BlueIcaro

P.D. Using Lazarus 1.014, W7/64
Title: Re: Playing with thread, memory leaks
Post by: Fred vS on February 16, 2014, 08:54:36 pm
Hello.

I think your thread has never terminated.

Could you do like that :

Code: [Select]

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.


Title: Re: Playing with thread, memory leaks
Post by: BlueIcaro on February 16, 2014, 09:03:06 pm
Thanks Fred_Vs for your answer.
Before you wrote. I modify the code and now I don't have any memory leak, but I have to click twice the close button to close my application.
Here is my new code:
Code: Pascal  [Select]
  1. unit Unit1;
  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(CreateSuspended: boolean);
  19.     destructor Destroy; override;
  20.   end;
  21.  
  22. type
  23.  
  24.   { TForm1 }
  25.  
  26.   TForm1 = class(TForm)
  27.     Button1: TButton;
  28.     procedure Button1Click(Sender: TObject);
  29.     procedure FormClose(Sender: TObject; var CloseAction: TCloseAction);
  30.     procedure FormCloseQuery(Sender: TObject; var CanClose: boolean);
  31.   private
  32.     { private declarations }
  33.     T: TMyThread;
  34.   public
  35.     { public declarations }
  36.   end;
  37.  
  38. var
  39.   Form1: TForm1;
  40.  
  41. implementation
  42.  
  43. { TForm1 }
  44.  
  45. procedure TForm1.Button1Click(Sender: TObject);
  46. begin
  47.   T := TMyThread.Create(True);
  48.   T.Start;
  49. end;
  50.  
  51. procedure TForm1.FormClose(Sender: TObject; var CloseAction: TCloseAction);
  52. begin
  53.  
  54.   FreeAndNil(T);
  55. end;
  56.  
  57. procedure TForm1.FormCloseQuery(Sender: TObject; var CanClose: boolean);
  58. begin
  59.     if t <> nil then
  60.     T.Terminate;
  61.     Application.ProcessMessages;
  62.     CanClose:=True;
  63. end;
  64.  
  65. {$R *.lfm}
  66.  
  67. { TMyThread }
  68.  
  69. procedure TMyThread.Execute;
  70. begin
  71.   repeat
  72.     Sleep(60);
  73.   until Terminated = True;
  74.  
  75. end;
  76.  
  77. constructor TMyThread.Create(CreateSuspended: boolean);
  78. begin
  79.   FreeOnTerminate := True;
  80.   inherited Create(CreateSuspended);
  81.  
  82. end;
  83.  
  84. destructor TMyThread.Destroy;
  85. begin
  86.   inherited Destroy;
  87. end;
  88.  
  89. end.
  90.  

If I use your code I got the same result, I have to click twice the close button in my form

Any idea?

/Blueicaro
Title: Re: Playing with thread, memory leaks
Post by: eny on February 16, 2014, 09:12:08 pm
The problem lies in the fact that you don't give the Thread time to free.
Calling Terminate does not do that.

In your particular case this works:
Code: [Select]
procedure TForm1.FormClose(Sender: TObject; var CloseAction: TCloseAction);
begin
  if t <> nil then
  begin
    T.Terminate;
    Sleep(120)
  end;
end; 

BTW, don't write
Code: [Select]
until Terminated = True;but write
Code: [Select]
until Terminated;
Title: Re: Playing with thread, memory leaks
Post by: User137 on February 16, 2014, 09:24:45 pm
It's still not perfect... How about this?
Code: [Select]
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.
Title: Re: Playing with thread, memory leaks
Post by: BlueIcaro on February 16, 2014, 09:26:22 pm
Thanks Eny, for your help. With a delay time it's wotks fine.

Looking for more info I found
Quote
WaitForThreadTerminate

I wrote:
Code: Pascal  [Select]
  1. WaitForThreadTerminate(t.ThreadID, 0);
  2.  
Now I playing with this function.

/BlueIcaro

P.D User137 You wrote your answer while I was writting. I going to test it
Title: Re: Playing with thread, memory leaks
Post by: Fred vS on February 16, 2014, 09:30:31 pm
Quote
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..)  :o
Title: Re: Playing with thread, memory leaks
Post by: eny on February 16, 2014, 09:34:24 pm
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.

If you want to have the thread auto-free itself you have to set up some sync mechanism to know when the thread actually can be terminated and/or freed (something along the lines FredVS suggested, but I would not use a global variable but the OnTerminate event for that).

Ooops, i did not know that, so no need for sleep(?) . (always difficult to know how many time is needed..)  :o
That's why it's easier not to auto-free but call Terminate in combination with WaitFor.
Caling WaitFor on an already terminated thread will make the application freeze.

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 :)
Title: Re: Playing with thread, memory leaks
Post by: Fred vS on February 16, 2014, 09:40:14 pm
Quote
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.

Do you mean that i can use WaitFor only if the thread has FreeOnTerminate := false ?

Title: Re: Playing with thread, memory leaks
Post by: BlueIcaro on February 16, 2014, 09:41:46 pm
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.
But first I like learn, I don't like copy code without understand it. That's because I play with small codes.

Well, I make some more test. I think the code has not ane leak memory.

I wrote here, may be can help anyone:

Code: Pascal  [Select]
  1. unit Unit1;
  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(CreateSuspended: boolean);
  19.     destructor Destroy; override;
  20.   end;
  21.  
  22. type
  23.  
  24.   { TForm1 }
  25.  
  26.   TForm1 = class(TForm)
  27.     Button1: TButton;
  28.     procedure Button1Click(Sender: TObject);
  29.     procedure FormClose(Sender: TObject; var CloseAction: TCloseAction);
  30.     procedure FormCloseQuery(Sender: TObject; var CanClose: boolean);
  31.     procedure FormDestroy(Sender: TObject);
  32.   private
  33.     { private declarations }
  34.     T: TMyThread;
  35.   public
  36.     { public declarations }
  37.   end;
  38.  
  39. var
  40.   Form1: TForm1;
  41.  
  42. implementation
  43.  
  44. { TForm1 }
  45.  
  46. procedure TForm1.Button1Click(Sender: TObject);
  47. begin
  48.   T := TMyThread.Create(True);
  49.   T.Start;
  50. end;
  51.  
  52. procedure TForm1.FormClose(Sender: TObject; var CloseAction: TCloseAction);
  53. begin
  54.  
  55. end;
  56.  
  57. procedure TForm1.FormCloseQuery(Sender: TObject; var CanClose: boolean);
  58. var
  59.   R: DWord;
  60. begin
  61.   if t <> nil then
  62.   begin
  63.     T.Terminate;
  64.      T.WaitFor;
  65.   end;
  66. end;
  67.  
  68. procedure TForm1.FormDestroy(Sender: TObject);
  69. begin
  70.   FreeAndNil(T);
  71. end;
  72.  
  73. {$R *.lfm}
  74.  
  75. { TMyThread }
  76.  
  77. procedure TMyThread.Execute;
  78. begin
  79.   repeat
  80.     Sleep(10);
  81.   until Terminated;
  82. end;
  83.  
  84. constructor TMyThread.Create(CreateSuspended: boolean);
  85. begin
  86.   FreeOnTerminate := False;
  87.   inherited Create(CreateSuspended);
  88. end;
  89.  
  90. destructor TMyThread.Destroy;
  91. begin
  92.   inherited Destroy;
  93. end;
  94.  
  95. end.
  96.  

/BlueIcaro
Title: Re: Playing with thread, memory leaks
Post by: engkin on February 16, 2014, 09:44:49 pm
It's still not perfect... How about this?
Code: [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;
..
It seems to me that calling FreeAndNil is sufficient because it leads to WaitFor inside TThread.SysDestroy
Title: Re: Playing with thread, memory leaks
Post by: Fred vS on February 16, 2014, 09:45:23 pm
Quote
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

And @ BlueCaro : maybe time to study and use TEvent ? ( i do not use sleep() for pausing threads, TEvent is much better, it does not wait that your sleep() has terminated...)
Title: Re: Playing with thread, memory leaks
Post by: BlueIcaro on February 16, 2014, 09:56:29 pm
Fred_Vs, Thanks for the information. I didn't know about TEvent. I'll take a look.
I put sleep  in the thread to make a small example. My "future" thread will download a file from internet, I think I'll not need Sleep.

/BlueIcaro
Title: Re: Playing with thread, memory leaks
Post by: eny on February 16, 2014, 09:57:23 pm
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.
Just don't do anything else and call WaitFor immediately.
The complexity lies in the fact that you must make sure the thread has not yet freed itself yet, when you call Waitfor.

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 :)
Btw you could also add the paramters to a custom Create method.

It seems to me that calling FreeAndNil is sufficient because it leads to WaitFor inside TThread.SysDestroy
Calling FreeAndNil() on a running Thread will lead to problems.
Title: Re: Playing with thread, memory leaks
Post by: engkin on February 16, 2014, 10:08:39 pm
Calling FreeAndNil() on a running Thread will lead to problems.
That what I thought until I checked the source code:
FreeAndNil(t)
 -->
t.Free
 --> 
if self<>nil then
  self.destroy;
 --> 
    TThread.SysDestroy;
Code: [Select]
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?
Title: Re: Playing with thread, memory leaks
Post by: Fred vS on February 16, 2014, 10:15:53 pm
Quote
The complexity lies in the fact that you must make sure the thread has not yet freed itself yet, when you call Waitfor.

Yep, that is the Big problem of Threads and array of Threads.

You cannot know if the thread has terminated, even with
Code: [Select]
if assisgned(MyTreads[x]).
It is the reason why i use a "twins" array of shortint, who vary in length with ThreadArray.

Code: [Select]
If MyTreads[x].create() then  MyArrayShortint[x] := 1.
If MyTreads[x].terminate  then  MyArrayShortint[x] := -1.

And, before to try to free a member of array, i check
Code: [Select]
if MyArrayShortint[x] = 1 then  MyArrayTreads[x].free....
Title: Re: Playing with thread, memory leaks
Post by: engkin on February 16, 2014, 10:26:40 pm
And, before to try to free a member of array, i 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.
Title: Re: Playing with thread, memory leaks
Post by: Fred vS on February 16, 2014, 10:38:56 pm
Quote
How do you know that the execution will not shift to the thread you are trying to free exactly after it passed this check.

This is to check mainly if the thread.create has append.

After that , because in the Thread-class there is :

Code: [Select]
TMyThread = class(TThread)
  protected
    procedure Execute; override;
  public
    isterminated :booelan;  ====> this one
     constructor Create(CreateSuspended: boolean);
    destructor Destroy; override;
  end;

And in thread.execute :

Code: [Select]
procedure TMyThread.Execute;
begin
  repeat
    Sleep(60);
until terminated_var = True;
 isterminated := true; ===> here
terminate;
end,
end;

If the thread was created, i can check isterminated even if thread has terminated (and so free the thread if needed)
Title: Re: Playing with thread, memory leaks
Post by: eny on February 16, 2014, 10:50:15 pm
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.
Try the simple example TS gave and instead of terminate/waifor, call FreeAndNil.
Could be related to the usage of sleep though.
Title: Re: Playing with thread, memory leaks
Post by: engkin on February 16, 2014, 11:37:22 pm
@eny, LOL I see your objection clearly.

@Fred, Thanks.
Title: Re: Playing with thread, memory leaks
Post by: engkin on February 17, 2014, 12:44:27 am
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 be related to the usage of sleep though.
Could you change his TMyThread.Create to make sure FreeOnTerminate := False and check again.
Title: Re: Playing with thread, memory leaks
Post by: Fred vS on February 17, 2014, 01:03:35 am
@ engkin :
Quote
if t <> nil then
    T.Terminate;

Code: [Select]
if t <> nil Is not really useful, =>will always be true.

Also, if the thread was not created,
Code: [Select]
if t <> nil then
    T.Terminate;
Will give a error and a big boum.  :-X
Title: Re: Playing with thread, memory leaks
Post by: engkin on February 17, 2014, 01:44:30 am
Code: [Select]
if t <> nil Is not really useful, =>will always be true.
Not 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.
Title: Re: Playing with thread, memory leaks
Post by: eny on February 17, 2014, 06:58:21 pm
..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).
This is however not documented in the expected location (http://www.freepascal.org/docs-html/ref/refse32.html#x71-810006.2).
Title: Re: Playing with thread, memory leaks
Post by: Blestan on February 17, 2014, 08:10:32 pm
it's a bad idea or even impossible ( on linux or some others unices)  to terminate  a thread from another thread ...
do not forget that the pascal TThreadd class is just a warper around  OS ( or lib functions) ... the thread main function is passed to the  OS like a callback and executed in new thread ... the only way to terfinate a thread. gracefully is to check in the execute method  periodicly for. the  terminated property .... and just exit the function ASAP ... using free or freeandnil will just free the warper object and will create a complete mess!
Title: Re: Playing with thread, memory leaks
Post by: howardpc on February 17, 2014, 08:16:23 pm
I have read somewhere that instance variables (e.g. form variables) are initialized to zero

I don't know where it can be found in the documentation, but in an fpc mailing list message (28/11/12) Jonas confirmed that global variables and static global arrays are always initialized to zero.
Title: Re: Playing with thread, memory leaks
Post by: engkin on February 17, 2014, 11:56:01 pm
... 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:

When you call Free (or FreeAndNil) it calls Destroy:
Quote from: rtl\inc\objpas.inc
      procedure TObject.Free;
        begin
           // the call via self avoids a warning
           if self<>nil then
             self.destroy;
        end;

Destroy calls SysDestroy before it frees the object:
Quote from: rtl\objpas\classes\classes.inc
destructor TThread.Destroy;
begin
  if not FExternalThread or False then begin
    SysDestroy;
..
  inherited Destroy;
end;

SysDestroy, on Windows, terminates the thread gracefully. It makes sure it's not finished, if not it calls terminate and then WaitFor.
Quote from: rtl\win\tthread.inc
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;

SysDestroy, on Linux, does the same thing in a better way, in my opinion:
Quote from: rtl\unix\tthread.inc
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;
Title: Re: Playing with thread, memory leaks
Post by: engkin on February 18, 2014, 12:09:56 am
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:
rtl\inc\objpas.inc
Code: [Select]
      class function TObject.InitInstance(instance : pointer) : tobject; {$ifdef SYSTEMINLINE} inline; {$ENDIF}
        begin
           { the size is saved at offset 0 }
           fillchar(instance^, InstanceSize, 0);
...
        end;
Title: Re: Playing with thread, memory leaks
Post by: Blestan on February 18, 2014, 01:43:01 pm
@ engkin :
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.
the thread are NOT pascal objects that you can manage ... freeandnil, destroy and free just deallocates the memory in the heap and runs some cleanup in the pascal hierarchy of classes... they cannot terminate a running.
an OS thread terminates by exiting its main proc.
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..
So, you cannot terminate the thread from another thread
Title: Re: Playing with thread, memory leaks
Post by: engkin on February 18, 2014, 03:00:45 pm
@ engkin :
when you call a method of TThread Instance it's executed in the current context NOT in the thread!!!
I agree with that. Where did you see me claiming the opposite?

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..
So, you cannot terminate the thread from another thread
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.
Title: Re: Playing with thread, memory leaks
Post by: Fred vS on February 18, 2014, 03:11:28 pm
Hello.
Im a bit confused...
In conclusion, what is the best way to terminate a thread  ?

Method 1 :
Code: [Select]
Constructor TMyGoodEndThread.Create();
begin
FreeOnTerminate := True; 
...
end;

procedure TMyGoodEndThread.Execute;
begin
...
Terminate;
Waitfor;   
end;

Method 2 :
Code: [Select]
Constructor TMyGoodEndThread.Create();
begin
FreeOnTerminate := false; 
...
end;

procedure TMyGoodEndThread.Execute;
begin
...
Terminate;
Waitfor; 
FreeAndNil; 
end;

Method 3 :
Code: [Select]
Constructor TMyGoodEndThread.Create();
begin
FreeOnTerminate := False; 
...
end;

procedure TMyGoodEndThread.Execute;
begin
...
Terminate;
Waitfor;   
Destroy;
end;

Method 4 :
Code: [Select]
... ?..
Many thanks.
Title: Re: Playing with thread, memory leaks
Post by: User137 on February 18, 2014, 03:32:35 pm
You don't call any of that in Thread.Execute. If you want that thread frees itself when it exits, then you use FreeOnTerminate := True; BUT it means that you cannot use thread.WaitFor; from anywhere for that thread.

As it came apparent, FreeAndNil contains all these; Terminate, WaitFor and Destroy. But the above note applies here too, you cannot use FreeAndNil(thread) if FreeOnTerminate := True;.

Basically FreeOnTerminate is for thread to be standalone. It will not allow any outside control telling it to free itself. You can call thread.Terminate from outside though, but you can't wait for it to close, or know when it did, except with using global variables.
Title: Re: Playing with thread, memory leaks
Post by: Fred vS on February 18, 2014, 03:37:50 pm
Quote
You don't call any of that in Thread.Execute.

OK, now im totally confused. :-[

What is "any of that" for you ?

Thanks..
Title: Re: Playing with thread, memory leaks
Post by: User137 on February 18, 2014, 03:49:37 pm
Working example 1:
Code: [Select]
Constructor TMyGoodEndThread.Create();
begin
FreeOnTerminate := True; 
...
end;

procedure TMyGoodEndThread.Execute;
begin
  while not terminated do begin
    ...
  end;
end;

Working example 2:
Code: [Select]
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;
Title: Re: Playing with thread, memory leaks
Post by: Fred vS on February 18, 2014, 04:10:56 pm
@ User137

Ok, thanks for your light...

Hum, sorry for the next stupid question :

Is that code OK ? :

Code: [Select]
var
TerminateMyThread : boolean = false;
...
Constructor TMyGoodEndThread.Create();
begin
FreeOnTerminate := True; 
...
end;

procedure TMyGoodEndThread.Execute;
begin
  while TerminateMyThread = false do
begin
    ...
  end;

Terminate;
WaitFor;

end;


Title: Re: Playing with thread, memory leaks
Post by: Blestan on February 18, 2014, 04:19:47 pm
with freeonterminate do not call waitfor or terminate ...

when the while loop is exited the thread will be terminated

any cleanup must be done here ... after the main loop

the internal execute call you execute then self.free

and the internal execute is passed to function BeginThread(
  ThreadFunction: TThreadFunc;
  p: pointer;
  var ThreadId: TThreadID;
  const stacksize: SizeUInt;
  creationflags: dword;
):TThreadID;

with creation flags CREATE_SUSPENDED or 0 to execute immediatly or later
to be executed in new created OS thread
Title: Re: Playing with thread, memory leaks
Post by: Fred vS on February 18, 2014, 06:15:15 pm
Quote
with freeonterminate do not call waitfor or terminate ...

Ok...

Hum, if i use external variabe : TerminateMyThread it is because the (array of) threads are inside a library.

And for foreign-language it is difficult to deal with threads inside a library.
So, using a "simple" variable (like boolean or integer), it is much easier than call Thread.terminate.

So is that code OK ?(is WaitFor done automatically by terminate ?)
Code: [Select]
var
TerminateMyThread : boolean = false;
...
Constructor TMyGoodEndThread.Create();
begin
FreeOnTerminate := True; 
...
end;

procedure TMyGoodEndThread.Execute;
begin
  while TerminateMyThread = false do
begin
    ...
  end;

Terminate;

end;

Thanks
Title: Re: Playing with thread, memory leaks
Post by: Fred vS on February 18, 2014, 06:25:00 pm
Quote
with freeonterminate do not call waitfor or terminate ...

Re-Oooops :
Quote
with freeonterminate do not call waitfor or terminate

Do you mean that the thread will terminate automaticaly with that code, without using terminate : ?
Code: [Select]
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;
Title: Re: Playing with thread, memory leaks
Post by: User137 on February 18, 2014, 06:35:11 pm
Terminate is just a procedure that sets variable, that's all:
Code: [Select]
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).
Title: Re: Playing with thread, memory leaks
Post by: Fred vS on February 18, 2014, 06:47:04 pm
@ User137 : many thanks for that clear explain. (and i learn much things today, re-thanks).

So, to resume, if
Code: [Select]
FreeOnTerminate := True; then, when
Code: [Select]
procedure TMyGoodEndThread.Execute;
arrives at end; the thread is automatically terminated, waitfor and freedandnil too.

But if
Code: [Select]
FreeOnTerminate := false; then, when
Code: [Select]
procedure TMyGoodEndThread.Execute;
arrives at end; the thread will terminate but waitfor and freedandnil should be called to avoid memory leak.

Yes/No ?

Title: Re: Playing with thread, memory leaks
Post by: User137 on February 18, 2014, 06:58:37 pm
So, to resume, if
Code: [Select]
FreeOnTerminate := True; then, when
Code: [Select]
procedure TMyGoodEndThread.Execute;
arrives at end; the thread is automatically terminated, waitfor and freedandnil too.
Roughly 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.).
(And here is another danger when exiting the application. If form1 is freed before thread, you'll get a SIGSEGV trying to set that global var.)

But if
Code: [Select]
FreeOnTerminate := false; then, when
Code: [Select]
procedure TMyGoodEndThread.Execute;
arrives at end; the thread will terminate but waitfor and freedandnil should be called to avoid memory leak.
Yeah, 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.
Title: Re: Playing with thread, memory leaks
Post by: Fred vS on February 18, 2014, 07:02:07 pm
@ User137 : perfect and many thanks.  ;)
Title: Re: Playing with thread, memory leaks
Post by: Fred vS on February 18, 2014, 09:39:47 pm
Hum, there is still a little shadow in that bright light...

Does it exist ?:   FreeAndNilOnTerminate := True;...  :-[

If no, I would not be against it...  :-X
Title: Re: Playing with thread, memory leaks
Post by: taazz on February 18, 2014, 11:33:05 pm
sorry this can not exist, you see it is one thing to set an object to nill and a completely different to know how many reference variables are there and how many of them are linked to the thread object. How about setting the onTerminate event and do the clean up in that event?
Title: Re: Playing with thread, memory leaks
Post by: eny on February 18, 2014, 11:53:21 pm
Hum, there is still a little shadow in that bright light...

Does it exist ?:   FreeAndNilOnTerminate := True;...  :-[
In theory you could define another instance variable that would be a pointer to the variable that contains the thread reference.
At the end of the thread's execute method you could set the thread reference to nil.

The type definition would be something like this
Code: [Select]
  PTMyThread = ^TMyThread;
  TMyThread = class(TThread)
  private
    FThreadRef: PTMyThread;
  public
     //....
  end;

 >:D
Title: Re: Playing with thread, memory leaks
Post by: Fred vS on February 19, 2014, 01:53:37 am
Quote
about setting the onTerminate event and do the clean up in that event?

Excellent idea  ;D

I will try that... (but is it safe to set to nil the thread by himself ?)

Quote
In theory you could define another instance variable that would be a pointer

Yep, nice too, i will try that too.

Write you later...

Many thanks.
Title: Re: Playing with thread, memory leaks
Post by: taazz on February 19, 2014, 02:13:06 am
(but is it safe to set to nil the thread by himself ?)


The onTerminate event is execute inside the main thread after the execute method returned and before the thread is freed (when the FreeOnTerminate is set). It is the equivalent of the freeandnil setting the variable to nil before freeing the object.
Title: Re: Playing with thread, memory leaks
Post by: Fred vS on February 19, 2014, 02:36:35 pm
@ Taazz : here your excellent idea...

Hum, i have test it and i do not hear any aaaaargh and do not see smoke or fire in my computer...

But it seems so nice and simple that i do not believe it, is that code ok ?

Code: [Select]
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;
Title: Re: Playing with thread, memory leaks
Post by: taazz on February 19, 2014, 03:08:07 pm
nope that's not it. Which variable the freeandnil(self) sets to nil? What happens if there more than one variables with that thread as value?
Title: Re: Playing with thread, memory leaks
Post by: Fred vS on February 19, 2014, 03:11:13 pm
Quote
Which variable the freeandnil(self) sets to nil?

Hum, self is the thread himself (No/Yes) ?
Title: Re: Playing with thread, memory leaks
Post by: taazz on February 19, 2014, 03:15:37 pm
you never check the thread it self for nil though you check the variables if they are nil, yes?
Title: Re: Playing with thread, memory leaks
Post by: Fred vS on February 19, 2014, 03:29:53 pm
Ok, i will try to explain a other way (using Taazz idea).

This one use array, is it better ?

Code: [Select]
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;
Title: Re: Playing with thread, memory leaks
Post by: User137 on February 19, 2014, 04:55:29 pm
Array is fine, but there is at least 1 point you need to note:
1) 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.

2) You allocate 1 too few  ;D  Should be:
Code: [Select]
setlength(MyArrayGoodThread, indexarray+1);
Title: Re: Playing with thread, memory leaks
Post by: Fred vS on February 19, 2014, 09:34:08 pm
Quote
2) You allocate 1 too few  ;D  Should be:
Code: [Select]

setlength(MyArrayGoodThread, indexarray+1);

Repor

No : that is the right code :

Code: [Select]
if indexarray+1 > length(MyArrayGoodThread) then setlength(MyArrayGoodThread, indexarray+1);  ;D ;D

Otherwise, you will have trouble with already created thread.

But, good, you are attentive..  :-*

Quote
1) 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.

Hum, i decide to not to play with moving index.
Too much problems, with few advantages...
If the thread was freed and assigned to nil, it is not very heavy inside the array...
And if the new length of array contain unassigned thread, it is not a big problem because nothing was created after setlength().
For that, i use a "twins" array of integer who vary in length with the thread array.
And when a thread is created => assign 1 to that twins{x].
So i can check in the thread array if thread{x] was already created.


PS : @ Taazz : part of you is inside uos now... :-X