Lazarus

Programming => Packages and Libraries => Topic started by: chrv on March 22, 2021, 09:32:10 am

Title: TATTabs memory leak
Post by: chrv on March 22, 2021, 09:32:10 am
Code below run fine. I can add several tabs at runtime.
Problem when checking for memory leak : 1 unfreed memory block by new tab created.
NB :
- no memory leak if no text is added to the tab
- same hapens with demo : ATFlatControls>app>demo_tabs

What Am i missing ?

Code: Pascal  [Select][+][-]
  1. uses
  2.   Classes, SysUtils, Forms, Controls, Graphics, Dialogs, StdCtrls, attabs;
  3. type
  4.   TForm1 = class(TForm)
  5.     Button1: TButton;
  6.     Edit1: TEdit;
  7.     procedure Button1Click(Sender: TObject);
  8.     procedure FormCreate(Sender: TObject);
  9.   private
  10.   public
  11.     AT : TATTabs;
  12.   end;
  13. var
  14.   Form1: TForm1;
  15. implementation
  16. {$R *.lfm}
  17. procedure TForm1.FormCreate(Sender: TObject);
  18. begin
  19.   AT := TATTabs.Create(Form1);
  20.   AT.parent := self;
  21.   AT.Align:=alTop;
  22. end;
  23. procedure TForm1.Button1Click(Sender: TObject);
  24. var
  25.   aTab : TATTabData;
  26. begin
  27.   aTab := TATTabData.Create(AT.Tabs);
  28.   aTab.TabCaption:=Edit1.Text;
  29. end;          
  30.  
Title: Re: TATTabs memory leak
Post by: AlexTP on March 22, 2021, 11:51:06 am
aTab := TATTabData.Create(AT.Tabs);

this allocates mem. and you do not free it. normally, I do
  data:= ATTabs1.GetTabData(nIndex);
  data.Caption:= 'smth';
Title: Re: TATTabs memory leak
Post by: chrv on March 22, 2021, 12:36:06 pm
Thanks for quick reply.

Your code is fine to change Tab caption but what about (dynamically) creating a new tab  (like in my little example program) ?
Title: Re: TATTabs memory leak
Post by: AlexTP on March 22, 2021, 02:02:03 pm
Method TATTabs.AddTab will do that alloc of Data.
Title: Re: TATTabs memory leak
Post by: chrv on March 22, 2021, 03:08:31 pm
I coverted my little demo with your suggestion.
It seems it does not change anything. Still memory leak

heaptrc : 1 unfreed memory blocks

Code: Pascal  [Select][+][-]
  1. unit Unit1;
  2. {$mode objfpc}{$H+}
  3. interface
  4. uses
  5.   Classes, SysUtils, Forms, Controls, Graphics, Dialogs, StdCtrls, attabs;
  6. type
  7.   TForm1 = class(TForm)
  8.     Button1: TButton;
  9.     Edit1: TEdit;
  10.     procedure Button1Click(Sender: TObject);
  11.     procedure FormCreate(Sender: TObject);
  12.   private
  13.   public
  14.     AT : TATTabs;
  15.   end;
  16.  
  17. var
  18.   Form1: TForm1;
  19.  
  20. implementation
  21. {$R *.lfm}
  22. procedure TForm1.FormCreate(Sender: TObject);
  23. begin
  24.   AT := TATTabs.Create(Form1);
  25.   AT.parent := self;
  26.   AT.Align:=alTop;
  27. end;
  28. procedure TForm1.Button1Click(Sender: TObject);
  29. //var
  30. //  aTab : TATTabData;
  31. begin
  32.   AT.AddTab(-1,Edit1.Text);
  33.   //aTab := TATTabData.Create(AT.Tabs);
  34.   //aTab.TabCaption:=Edit1.Text;
  35. end;
  36. end.
     

More information :
1) if  Edit.text ='' => no memory leak   
2) AT.AddTab(-1,'XXX') => no memory leak
Title: Re: TATTabs memory leak
Post by: AlexTP on March 22, 2021, 03:18:56 pm
According to your info,I added deallocation of Data's. Pls get GitHub version and test?
Title: Re: TATTabs memory leak
Post by: chrv on March 22, 2021, 07:30:53 pm
For me, it still leaking
Code: Pascal  [Select][+][-]
  1. destructor TATTabListCollection.Destroy;
  2. var
  3.   i : integer;
  4. begin
  5.   For i := Count-1 downto 0 do
  6.     Items[i].Free;
  7.   inherited Destroy;
  8. end;  
It seems Count allready = 0 at this point.
Title: Re: TATTabs memory leak
Post by: AlexTP on March 22, 2021, 08:39:23 pm
Yes, Count is 0, changed that place.
Leaking: I don't have an idea.
Title: Re: TATTabs memory leak
Post by: AlexTP on March 22, 2021, 08:49:27 pm
I run the demo_tabs from ATFlatControls, with 'heaptrc' checkbox in IDE options, and see NO leaks.
Quote
Heap dump by heaptrc unit of /home/user/cuda/atflatcontrols/app/demo_tabs/demo
12747 memory blocks allocated : 27977489/27999088
12747 memory blocks freed     : 27977489/27999088
0 unfreed memory blocks : 0
True heap size : 2686976
True free heap : 2686976
 

this is after fixing that demo_tabs.
Title: Re: TATTabs memory leak
Post by: tom.tom on July 22, 2021, 04:32:26 pm
Yeah, there is something strange with memory leak when assigning the dynamic string to the TabCaption property. It looks like a false positive report though...

No leak reported:
Code: Pascal  [Select][+][-]
  1.   for ids := 1 to CameraTabs.TabCount do
  2.   begin
  3.     tabData := CameraTabs.GetTabData(Pred(idx));
  4.     tabData.TabCaption := 'test';
  5.   end;
  6.  

[TabCount-1] leaks reported where the TabCaption is assigned:
Code: Pascal  [Select][+][-]
  1.   for idx:= 1 to CameraTabs.TabCount do
  2.   begin
  3.     tabData := CameraTabs.GetTabData(Pred(idx));
  4.     tabData.TabCaption := Format('%d: %s', [idx, 'test']);;
  5.   end;
  6.  

Isn't it because heaptrc gets confused because tabData is just a reference and never freed locally?

Title: Re: TATTabs memory leak
Post by: AlexTP on July 22, 2021, 07:59:04 pm
I tried to clear TabList collection in Destroy - updated ATTabs in GitHub. Pls test.
Title: Re: TATTabs memory leak
Post by: chrv on July 22, 2021, 11:52:35 pm
I never succeded to fine a working solution with TATTabs : allways memory leaks when dynamicaly creating Tabs.
Finaly i tested TJvTabBar (from Jvcllaz in Online Package Manager) ) : everything works without problem (no memory leaks).
Title: Re: TATTabs memory leak
Post by: AlexTP on July 22, 2021, 11:54:13 pm
@chrv,
ATTabs has a patch from today, to fix the leaks, pls try.
Title: Re: TATTabs memory leak
Post by: tom.tom on July 23, 2021, 05:02:29 pm
Still leaks (see the attachment).

Line 7709 of Unit1 is dynamic string assignment to the TabCaption property.
Title: Re: TATTabs memory leak
Post by: AlexTP on July 23, 2021, 07:01:43 pm
An idea of 'additional leak' is that you assign TATTabData.TabObject, but you don't free it.
Or are you sure it's leak of 'string' vars?
Title: Re: TATTabs memory leak
Post by: tom.tom on July 23, 2021, 09:11:35 pm
I'll prepare sample app for you to run and tell me if there is a leak at your end.

BTW - which OS and LAzarus version do you use?
Title: Re: TATTabs memory leak
Post by: AlexTP on July 23, 2021, 10:47:02 pm
Lazarus 2.3.0 r65368M FPC 3.2.1 x86_64-linux-gtk2
Title: Re: TATTabs memory leak
Post by: tom.tom on July 26, 2021, 12:32:47 pm
I use older version - Lazarus 1.8, FPC 3.0.4 and Windows and OSX.

Could you check if this simple code reports memory leaks when run and closed in your environment (tick the 'use heaptrc' in project options)?

In my case it reports the leak in unit1 line 40.


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, attabs;
  9.  
  10. type
  11.  
  12.   { TForm1 }
  13.  
  14.   TForm1 = class(TForm)
  15.     ATTabs1: TATTabs;
  16.     procedure FormCreate(Sender: TObject);
  17.   private
  18.  
  19.   public
  20.  
  21.   end;
  22.  
  23. var
  24.   Form1: TForm1;
  25.  
  26. implementation
  27.  
  28. {$R *.lfm}
  29.  
  30. { TForm1 }
  31.  
  32. procedure TForm1.FormCreate(Sender: TObject);
  33. var
  34.   iTmp : Integer;
  35.   data : TATTabData;
  36. begin
  37.   for iTmp := 0 to Pred(ATTabs1.TabCount) do
  38.   begin
  39.     data := ATTabs1.GetTabData(iTMp);
  40.     data.TabCaption := Format ('Tab #%d', [iTmp]);
  41.   end;
  42. end;
  43.  
  44. end.
  45.  
  46.  

Code: Pascal  [Select][+][-]
  1. object Form1: TForm1
  2.   Left = 741
  3.   Height = 332
  4.   Top = 405
  5.   Width = 625
  6.   Caption = 'Form1'
  7.   ClientHeight = 332
  8.   ClientWidth = 625
  9.   OnCreate = FormCreate
  10.   LCLVersion = '1.8.0.6'
  11.   object ATTabs1: TATTabs
  12.     Left = 0
  13.     Height = 35
  14.     Top = 0
  15.     Width = 625
  16.     Align = alTop
  17.     ParentColor = False
  18.     Tabs = <    
  19.       item
  20.       end    
  21.       item
  22.       end    
  23.       item
  24.       end    
  25.       item
  26.       end>
  27.     DoubleBuffered = True
  28.     OptButtonLayout = '<>,v'
  29.     OptShowModifiedText = '*'
  30.     OptShowPinnedText = '!'
  31.     OptHintForX = 'Close tab'
  32.     OptHintForPlus = 'Add tab'
  33.     OptHintForArrowLeft = 'Scroll tabs left'
  34.     OptHintForArrowRight = 'Scroll tabs right'
  35.     OptHintForArrowMenu = 'Show tabs list'
  36.     OptHintForUser0 = '0'
  37.     OptHintForUser1 = '1'
  38.     OptHintForUser2 = '2'
  39.     OptHintForUser3 = '3'
  40.     OptHintForUser4 = '4'
  41.   end
  42. end
  43.  
Title: Re: TATTabs memory leak
Post by: AlexTP on July 26, 2021, 01:44:24 pm
Yes I see that memleak, in this project. I looked in the code but couldn't find the reason, sorry. E.g. in this project (4 tabs) I see that TATTabData.Destroy is called 4 times on exiting.
Title: Re: TATTabs memory leak
Post by: tom.tom on July 26, 2021, 03:13:10 pm
What's that 37 bytes block? Is this TATTabData size?

Please notice, that if you change the dynamic string (Format(...)) to static - i.e.

data.TabCaption := 'Tab';

...there will be no leak reported.

It may be something to do with tracking that string reference not the TATabData as a whole....
Title: Re: TATTabs memory leak
Post by: AlexTP on July 26, 2021, 06:10:38 pm
Then it means that TabCaption value makes a leak. But again, I cannot find the reason..
Title: Re: TATTabs memory leak
Post by: AlexTP on September 10, 2021, 04:40:05 pm
It (mem leak) was fixed just now. See https://github.com/Alexey-T/ATFlatControls/issues/73

Updated in github.
TinyPortal © 2005-2018