Recent

Author Topic: Playing with thread, memory leaks  (Read 37752 times)

BlueIcaro

  • Hero Member
  • *****
  • Posts: 576
Playing with thread, memory leaks
« 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

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
Remenber, the lazarus wiki is your friend: http://wiki.lazarus.freepascal.org/Main_Page
General questions (several lenguages) http://wiki.lazarus.freepascal.org/

Fred vS

  • Hero Member
  • *****
  • Posts: 1670
    • miXimum is the DJ's best friend
Re: Playing with thread, memory leaks
« Reply #1 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.


« Last Edit: February 16, 2014, 09:10:11 pm by Fred vS »
I use Lazarus 1.8.0 32/64 and FPC 3.0.3 32/64 on Linux Mint Mate 17 32/64, Windows 10, Windows 7 32/64, Windows XP 32,  FreeBSD 64 and Mac OS X Snow Leopard 32.
Widgetset: fpGUI, MSEgui, Win32, GTK2, Qt, Carbon.

https://github.com/fredvs

BlueIcaro

  • Hero Member
  • *****
  • Posts: 576
Re: Playing with thread, memory leaks
« Reply #2 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
« Last Edit: February 16, 2014, 09:05:56 pm by BlueIcaro »
Remenber, the lazarus wiki is your friend: http://wiki.lazarus.freepascal.org/Main_Page
General questions (several lenguages) http://wiki.lazarus.freepascal.org/

eny

  • Hero Member
  • *****
  • Posts: 1585
Re: Playing with thread, memory leaks
« Reply #3 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;
All posts based on: Win10 (Win64); Lazarus 1.8.0 'stable' (#56594 win64) unless specified otherwise...

User137

  • Hero Member
  • *****
  • Posts: 1790
    • Nxpascal home
Re: Playing with thread, memory leaks
« Reply #4 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.
« Last Edit: February 16, 2014, 09:27:30 pm by User137 »

BlueIcaro

  • Hero Member
  • *****
  • Posts: 576
Re: Playing with thread, memory leaks
« Reply #5 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
Remenber, the lazarus wiki is your friend: http://wiki.lazarus.freepascal.org/Main_Page
General questions (several lenguages) http://wiki.lazarus.freepascal.org/

Fred vS

  • Hero Member
  • *****
  • Posts: 1670
    • miXimum is the DJ's best friend
Re: Playing with thread, memory leaks
« Reply #6 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
I use Lazarus 1.8.0 32/64 and FPC 3.0.3 32/64 on Linux Mint Mate 17 32/64, Windows 10, Windows 7 32/64, Windows XP 32,  FreeBSD 64 and Mac OS X Snow Leopard 32.
Widgetset: fpGUI, MSEgui, Win32, GTK2, Qt, Carbon.

https://github.com/fredvs

eny

  • Hero Member
  • *****
  • Posts: 1585
Re: Playing with thread, memory leaks
« Reply #7 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 :)
« Last Edit: February 16, 2014, 09:38:37 pm by eny »
All posts based on: Win10 (Win64); Lazarus 1.8.0 'stable' (#56594 win64) unless specified otherwise...

Fred vS

  • Hero Member
  • *****
  • Posts: 1670
    • miXimum is the DJ's best friend
Re: Playing with thread, memory leaks
« Reply #8 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 ?

I use Lazarus 1.8.0 32/64 and FPC 3.0.3 32/64 on Linux Mint Mate 17 32/64, Windows 10, Windows 7 32/64, Windows XP 32,  FreeBSD 64 and Mac OS X Snow Leopard 32.
Widgetset: fpGUI, MSEgui, Win32, GTK2, Qt, Carbon.

https://github.com/fredvs

BlueIcaro

  • Hero Member
  • *****
  • Posts: 576
Re: Playing with thread, memory leaks
« Reply #9 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
Remenber, the lazarus wiki is your friend: http://wiki.lazarus.freepascal.org/Main_Page
General questions (several lenguages) http://wiki.lazarus.freepascal.org/

engkin

  • Hero Member
  • *****
  • Posts: 2400
Re: Playing with thread, memory leaks
« Reply #10 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

Fred vS

  • Hero Member
  • *****
  • Posts: 1670
    • miXimum is the DJ's best friend
Re: Playing with thread, memory leaks
« Reply #11 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...)
« Last Edit: February 16, 2014, 09:48:22 pm by Fred vS »
I use Lazarus 1.8.0 32/64 and FPC 3.0.3 32/64 on Linux Mint Mate 17 32/64, Windows 10, Windows 7 32/64, Windows XP 32,  FreeBSD 64 and Mac OS X Snow Leopard 32.
Widgetset: fpGUI, MSEgui, Win32, GTK2, Qt, Carbon.

https://github.com/fredvs

BlueIcaro

  • Hero Member
  • *****
  • Posts: 576
Re: Playing with thread, memory leaks
« Reply #12 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
Remenber, the lazarus wiki is your friend: http://wiki.lazarus.freepascal.org/Main_Page
General questions (several lenguages) http://wiki.lazarus.freepascal.org/

eny

  • Hero Member
  • *****
  • Posts: 1585
Re: Playing with thread, memory leaks
« Reply #13 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.
All posts based on: Win10 (Win64); Lazarus 1.8.0 'stable' (#56594 win64) unless specified otherwise...

engkin

  • Hero Member
  • *****
  • Posts: 2400
Re: Playing with thread, memory leaks
« Reply #14 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?