View Issue Details

IDProjectCategoryView StatusLast Update
0034708LazarusLCLpublic2018-12-22 11:31
ReporterlainzAssigned ToZeljan Rikalo 
PrioritynormalSeverityminorReproducibilityunable to reproduce
Status resolvedResolutionfixed 
Platformi386OSWindowsOS Version10
Product Version2.1 (SVN)Product Build59627 
Target VersionFixed in Version 
Summary0034708: ImageList memory leak
Descriptionhttps://forum.lazarus.freepascal.org/index.php/topic,43525.msg305057.html#msg305057

TApplication.HandleException: EOutOfMemory
Out of memory
  Stack trace:
  $00413B9A
  $00413EDA
  $0061B2B2 ALLOCDATA, line 329 of include/imglist.inc
  $0061BF46 INTERNALINSERT, line 626 of include/imglist.inc
  $0061E112 ADDSLICE, line 1390 of include/imglist.inc
  $0062227A ADDNEWBTNIMAGE, line 2745 of include/imglist.inc
  $00621F76 GETIMAGEINDEX, line 2781 of include/imglist.inc
  $007F96CD GETIMAGEINDEX, line 290 of ideimagesintf.pas
  $008494C8
  $008663DE
  $00875CE8
  $0043A61E GETCHILDREN, line 1072 of include/customform.inc
  $008755B0
  $00875E7A
  $008760E7
  $00875F59
  $008785B8
Steps To ReproduceI have a big form with many image lists on it, when I'm using the IDE it crashes, the log file points that the leak comes from image list if I understand it correctly.
TagsNo tags attached.
Fixed in Revision59892
LazTarget-
WidgetsetWin32/Win64
Attached Files
  • testimglist.zip (226,268 bytes)
  • imglist-01.patch (519 bytes)
    Index: lcl/include/imglist.inc
    ===================================================================
    --- lcl/include/imglist.inc	(revision 59877)
    +++ lcl/include/imglist.inc	(working copy)
    @@ -326,9 +326,9 @@
       if n <> 0
       then Inc(ACount, FImageList.FAllocBy - n);
     
    -  SetLength(FData, ACount * FWidth * FHeight * SizeOf(FData[0]));
    +  SetLength(FData, ACount * FWidth * FHeight);
     
    -  Inc(FAllocCount, ACount);
    +  FAllocCount := ACount;
     end;
     
     procedure TCustomImageListResolution.CheckIndex(AIndex: Integer;
    
    imglist-01.patch (519 bytes)
  • ImageListBug.tar.gz (190,360 bytes)
  • imglist-02.patch (866 bytes)
    Index: lcl/include/imglist.inc
    ===================================================================
    --- lcl/include/imglist.inc	(revision 59890)
    +++ lcl/include/imglist.inc	(working copy)
    @@ -326,9 +326,9 @@
       if n <> 0
       then Inc(ACount, FImageList.FAllocBy - n);
     
    -  SetLength(FData, ACount * FWidth * FHeight * SizeOf(FData[0]));
    +  SetLength(FData, ACount * FWidth * FHeight);
     
    -  Inc(FAllocCount, ACount);
    +  FAllocCount := ACount;
     end;
     
     procedure TCustomImageListResolution.CheckIndex(AIndex: Integer;
    @@ -1461,7 +1461,7 @@
             ToR.AllocData(ToR.FCount);
             if ToR.FCount>0 then
             begin
    -          DataSize := ToR.FWidth * ToR.FHeight * SizeOf(FData[0]);
    +          DataSize := ToR.FWidth * ToR.FHeight * SizeOf(ToR.FData[0]);
               System.Move(FromR.FData[0], ToR.FData[0], ToR.FCount * DataSize);
             end;
           end;
    
    imglist-02.patch (866 bytes)

Activities

wp

2018-12-16 16:36

developer   ~0112606

Could you post a demo which shows the issue?

lainz

2018-12-16 18:49

reporter  

testimglist.zip (226,268 bytes)

lainz

2018-12-16 18:50

reporter   ~0112617

Last edited: 2018-12-16 18:52

View 2 revisions

Attached the demo. Open task manager on Windows and see how memory slowly grows. If time passes by it crashes when it's about 700 mb on my machine that has 8 gb.

Edit: Use the IDE like type something or save often, so memory grows.

Juha Manninen

2018-12-16 18:59

developer   ~0112618

007, your demo app does nothing. It only has an ImageList.
Are you sure it creates a leak for you?

wp

2018-12-16 19:54

developer   ~0112619

Using heaptrc I cannot detect a leak in your demo (Win 10/32 bit, Laz trunk, fpc 3.0.4/64 bit). And if it would leak Lazarus and myriads of other applications using a TImageList would leak as well. Please provide a demo which shows the issue with heaptrc. The memory usage in Windows can increase with time for many reasons.

lainz

2018-12-16 20:26

reporter   ~0112621

Hi, sorry you're right. I've removed the ImageList from my project and it stops working the IDE anyways.

This is what the last log gives (removing the image list from my project):

TApplication.HandleException: EOutOfMemory
Out of memory
  Stack trace:
  $004138CA
  $00413C0A
  $005D3075 TCUSTOMIMAGELISTRESOLUTION__ALLOCDATA, line 329 of ./include/imglist.inc
  $005D3980 TCUSTOMIMAGELISTRESOLUTION__INTERNALINSERT, line 626 of ./include/imglist.inc
  $005D529B TCUSTOMIMAGELIST__ADDSLICE, line 1390 of ./include/imglist.inc
  $005D85B8 ADDNEWBTNIMAGE, line 2745 of ./include/imglist.inc
  $005D8332 TLCLGLYPHS__GETIMAGEINDEX, line 2781 of ./include/imglist.inc
  $0076B93F TIDEIMAGES__GETIMAGEINDEX, line 290 of ideimagesintf.pas
  $007B99D8 TPKGCOMPONENT__IMAGEINDEX, line 4030 of C:/fpcupdeluxe/lazarus/packager/packagedefs.pas
  $007D65EE TCOMPONENTPALETTE__ONGETNONVISUALCOMPICON, line 1045 of componentpalette.pas
  $007E5EF8 TDESIGNER__DRAWNONVISUALCOMPONENT, line 3651 of C:/fpcupdeluxe/lazarus/designer/designer.pp
  $0043538B TCUSTOMFORM__GETCHILDREN, line 1072 of ./include/customform.inc
  $007E57C0 TDESIGNER__DRAWNONVISUALCOMPONENT, line 3554 of C:/fpcupdeluxe/lazarus/designer/designer.pp
  $007E608A TDESIGNER__DRAWNONVISUALCOMPONENTS, line 3672 of C:/fpcupdeluxe/lazarus/designer/designer.pp
  $007E62F7 TDESIGNER__DOPAINTDESIGNERITEMS, line 3735 of C:/fpcupdeluxe/lazarus/designer/designer.pp
  $007E6169 TDESIGNER__DRAWDESIGNERITEMS, line 3691 of C:/fpcupdeluxe/lazarus/designer/designer.pp
  $007E87C8 TFORMEDITOR__PAINTALLDESIGNERITEMS, line 98 of formeditor.pp

I think is not the image list in my project, but an image list is used by the IDE to store the icons of the components placed in the form. I have a lot of components (more than 60) placed in the form.

So the leak is in alloc data what I can understand.

Sorry I can't share the form that leaks, is a commercial application.

lainz

2018-12-16 20:30

reporter   ~0112622

To clarify: My app works fine, it has no leaks, is the IDE that stop working after a while working in that form.

When I say compoenents is like TBufDataset and the like, not visual controls.

Juha Manninen

2018-12-16 21:53

developer   ~0112623

Do you mean the IDE leaks memory? No it doesn't.
I have heaptrace and all other debug flags on always when testing it.
No leaks except OPM may have some rarely.

The problem is clearly somewhere else. You should brings such issues to forum or mailing list.

jamie philbrook

2018-12-16 22:14

reporter   ~0112624

Looks like he is using some data aware components, wonder if he has some
active components during design time that isn't making the IDE happy?

 Most likely has leaks in the components he installed that are not being
properly managing the resources when querying the design time properties to
avoid some activity.

 I know, I did that once! :(

lainz

2018-12-16 23:07

reporter   ~0112633

Ok. You're right is not a memory leak. Please you can update the title to out of memory error.

And why it always crashes and in the log is imglist allocation procedure?

lainz

2018-12-17 00:09

reporter   ~0112636

I can't replicate, memory grows in a form with a lot of components but not as much as in my form. Maybe as pointed a component that is using image list that is not the IDE.

I think I will make that form smaller, move everything that's a component to a data module and see in the forum.

Sorry, I can't provide a form that shows the behaviour. Please close this.

wp

2018-12-17 00:17

developer   ~0112637

Resolving, no change required. Please close.

lainz

2018-12-17 00:19

reporter   ~0112638

Thanks for your time. I will try to spot the problem and comment in the forum.

lainz

2018-12-20 16:02

reporter   ~0112754

Ondrej Pokorny said that has a solution for this

lainz

2018-12-20 16:04

reporter   ~0112755

https://forum.lazarus.freepascal.org/index.php/topic,43525.msg305549.html#msg305549

Ondrej Pokorny

2018-12-20 16:09

reporter  

imglist-01.patch (519 bytes)
Index: lcl/include/imglist.inc
===================================================================
--- lcl/include/imglist.inc	(revision 59877)
+++ lcl/include/imglist.inc	(working copy)
@@ -326,9 +326,9 @@
   if n <> 0
   then Inc(ACount, FImageList.FAllocBy - n);
 
-  SetLength(FData, ACount * FWidth * FHeight * SizeOf(FData[0]));
+  SetLength(FData, ACount * FWidth * FHeight);
 
-  Inc(FAllocCount, ACount);
+  FAllocCount := ACount;
 end;
 
 procedure TCustomImageListResolution.CheckIndex(AIndex: Integer;
imglist-01.patch (519 bytes)

Ondrej Pokorny

2018-12-20 16:09

reporter   ~0112756

Patch attached. 007 can you test it if it helps?

wp

2018-12-20 17:56

developer   ~0112760

Applied the patch (r59880). Thank you Ondrej - good to see you still around.

007, please test if it fixes the gradual increase of memory load.

lainz

2018-12-20 22:04

reporter   ~0112763

Last edited: 2018-12-20 22:07

View 2 revisions

Memory still increasing. Must be something else, I need to keep it running until crash to see if the log changes from previous intents, I will try this weekend.

wp

2018-12-20 23:47

developer   ~0112767

Did you ever test your application with heaptrc?

Check "Use Heaptrc unit" in "Project options" > "Compiler options" > "Debugging", compile and run the program, maybe until you notice memory increasing. Close the program, and you will see a list of all code lines with non-released memory allocations.

Zeljan Rikalo

2018-12-21 16:53

developer   ~0112781

Last edited: 2018-12-21 16:54

View 2 revisions

This is not good. r59880 introduced crashes. eg Qt4/Qt5 application with MDI windows cannot create simple MDI window at all - sigsegv. TQtWidget.LCLObject becomes nil inside TQtMainWindow.CreateWidget() for unknown reason.
EDIT: r59879 works fine, and non mdi windows works fine.

Zeljan Rikalo

2018-12-21 16:57

developer   ~0112783

#0 0x00000000004270e4 in SYSTEM_$$_MOVE$formal$formal$INT64 ()
0000001 0x00000000009068ea in ASSIGN (this=0x7fffc9b2b100, SOURCE=0x7ffff7e19800) at include/imglist.inc:1463

Zeljan Rikalo

2018-12-21 17:10

developer   ~0112784

Please test with r59888, I've fixed memory corruption when assigning imagelist.

Zeljan Rikalo

2018-12-21 17:15

developer   ~0112785

IMO, r59880 and r59888 should be reverted back. TImageList does not work as expected anymore.

wp

2018-12-21 17:18

developer   ~0112786

Last edited: 2018-12-21 17:20

View 2 revisions

Are you sure? DataSize is the size of an image in bytes here, and it is used to copy ToR.FCount images by a System.Move which requires the size in bytes, not in array element counts as in the location fixed by 59880.

What I think is wrong is that a few lines higher the assignment of ToR.Height is missing.

Zeljan Rikalo

2018-12-21 17:36

developer   ~0112787

Last edited: 2018-12-21 17:40

View 3 revisions

I'm pretty sure.
Edit: Problem is that r59880 WORKS ok only for win32.
All other ws crashes because of memory corruption (64bit tested only - gtk2, qt and qt5). With r59888 there's no more crashes but some bitmaps are invisible when using ImgList.GetBitmap().
And yes, I've tried to assign FHeight but same problem persists.
Updating my svn copy to r59879 fixes all problems.

wp

2018-12-21 17:45

developer   ~0112788

OK. Since I don't have the correct test program at the moment to see these issues (my test program is working) I pass this report over to you. Then you can revert anything needed.

Zeljan Rikalo

2018-12-21 17:52

developer  

ImageListBug.tar.gz (190,360 bytes)

Zeljan Rikalo

2018-12-21 17:53

developer   ~0112789

I've attached simple project which shows all problems as I mentioned in my comments. I'll revert 59880 and 59888 since it does not fix original issue of the reporter, and introduces crashes on non win32 widgetsets.

Zeljan Rikalo

2018-12-21 18:15

developer   ~0112790

r59880 & r59888 reverted in r59889

Ondrej Pokorny

2018-12-22 01:37

reporter   ~0112797

Zeljko, you talk nonsense. Werner is right.

My r59880 is absolutely OK, your r59888 is wrong.

BUT there was a third bug in the Assign method (SizeOf(ToR.FData[0]) <> SizeOf(FData[0]) !!!) See the second patch.

Btw. assignments of FWidth&FHeight are not necessary (they are always equal) but they definitely don't harm.

Ondrej Pokorny

2018-12-22 01:37

reporter  

imglist-02.patch (866 bytes)
Index: lcl/include/imglist.inc
===================================================================
--- lcl/include/imglist.inc	(revision 59890)
+++ lcl/include/imglist.inc	(working copy)
@@ -326,9 +326,9 @@
   if n <> 0
   then Inc(ACount, FImageList.FAllocBy - n);
 
-  SetLength(FData, ACount * FWidth * FHeight * SizeOf(FData[0]));
+  SetLength(FData, ACount * FWidth * FHeight);
 
-  Inc(FAllocCount, ACount);
+  FAllocCount := ACount;
 end;
 
 procedure TCustomImageListResolution.CheckIndex(AIndex: Integer;
@@ -1461,7 +1461,7 @@
         ToR.AllocData(ToR.FCount);
         if ToR.FCount>0 then
         begin
-          DataSize := ToR.FWidth * ToR.FHeight * SizeOf(FData[0]);
+          DataSize := ToR.FWidth * ToR.FHeight * SizeOf(ToR.FData[0]);
           System.Move(FromR.FData[0], ToR.FData[0], ToR.FCount * DataSize);
         end;
       end;
imglist-02.patch (866 bytes)

Zeljan Rikalo

2018-12-22 10:30

developer   ~0112799

Nonsense about what exactly ? There's memory corruption on linux 64bit , and yes I know that 59888 is wrong, that's why I reverted 59888 (wrong) and 59880 (mem corruption). Thanks for the second patch, I'll test it today.

Zeljan Rikalo

2018-12-22 10:43

developer   ~0112800

Please test and close if ok.

Ondrej Pokorny

2018-12-22 10:48

reporter   ~0112801

> Nonsense about what exactly ?

System.Move needs count of bytes, as Werner said.

The reason why r59880 worked only on win32 is because you compiled win32 for 32bit and other widgetset for 64bit. If you compiled with Qt for 32bit, it would work as well.

Because on line 1462:
32bit: SizeOf(ToR.FData[0]) = SizeOf(FData[0])
64bit: SizeOf(ToR.FData[0]) < SizeOf(FData[0])

Unfortunately there are 2 fields with the same name:
1.) TCustomImageListResolution.FData: array of TRGBAQuad;
2.) TCustomImageList.FData: TCustomImageListResolutions;

Maybe it would be worth to rename the latter to something else, e.g. FResolutions: TCustomImageListResolutions;

wp

2018-12-22 11:08

developer   ~0112806

Thank you.

Zeljan, I added also r59890 to the merge requests because I think that r59892 will create a merge conflict without it.

Zeljan Rikalo

2018-12-22 11:31

developer   ~0112807

@wp, thanks, I've forgot about r59890. I'll test all widgetsets I'm able to test today (win32,qt,qt5 on windows, gtk2,qt,qt5 on linux, qt5 and cocoa on mac) for this change and move it to merge list for RC4 if everything is ok.

Issue History

Date Modified Username Field Change
2018-12-16 16:10 lainz New Issue
2018-12-16 16:36 wp Note Added: 0112606
2018-12-16 18:49 lainz File Added: testimglist.zip
2018-12-16 18:50 lainz Note Added: 0112617
2018-12-16 18:52 lainz Note Edited: 0112617 View Revisions
2018-12-16 18:59 Juha Manninen Note Added: 0112618
2018-12-16 19:54 wp Note Added: 0112619
2018-12-16 20:26 lainz Note Added: 0112621
2018-12-16 20:30 lainz Note Added: 0112622
2018-12-16 21:53 Juha Manninen Note Added: 0112623
2018-12-16 22:14 jamie philbrook Note Added: 0112624
2018-12-16 23:07 lainz Note Added: 0112633
2018-12-17 00:09 lainz Note Added: 0112636
2018-12-17 00:17 wp LazTarget => -
2018-12-17 00:17 wp Note Added: 0112637
2018-12-17 00:17 wp Status new => resolved
2018-12-17 00:17 wp Resolution open => no change required
2018-12-17 00:17 wp Assigned To => wp
2018-12-17 00:19 lainz Note Added: 0112638
2018-12-17 00:19 lainz Status resolved => closed
2018-12-20 16:02 lainz Note Added: 0112754
2018-12-20 16:02 lainz Status closed => assigned
2018-12-20 16:02 lainz Resolution no change required => reopened
2018-12-20 16:04 lainz Note Added: 0112755
2018-12-20 16:09 Ondrej Pokorny File Added: imglist-01.patch
2018-12-20 16:09 Ondrej Pokorny Note Added: 0112756
2018-12-20 17:56 wp Note Added: 0112760
2018-12-20 17:56 wp Status assigned => feedback
2018-12-20 22:04 lainz Note Added: 0112763
2018-12-20 22:04 lainz Status feedback => assigned
2018-12-20 22:07 lainz Note Edited: 0112763 View Revisions
2018-12-20 23:47 wp Note Added: 0112767
2018-12-21 16:53 Zeljan Rikalo Note Added: 0112781
2018-12-21 16:53 Zeljan Rikalo Status assigned => feedback
2018-12-21 16:54 Zeljan Rikalo Note Edited: 0112781 View Revisions
2018-12-21 16:57 Zeljan Rikalo Note Added: 0112783
2018-12-21 17:10 Zeljan Rikalo Fixed in Revision => 59880,59888
2018-12-21 17:10 Zeljan Rikalo Note Added: 0112784
2018-12-21 17:10 Zeljan Rikalo Status feedback => resolved
2018-12-21 17:10 Zeljan Rikalo Resolution reopened => fixed
2018-12-21 17:10 Zeljan Rikalo Status resolved => feedback
2018-12-21 17:15 Zeljan Rikalo Note Added: 0112785
2018-12-21 17:18 wp Note Added: 0112786
2018-12-21 17:20 wp Note Edited: 0112786 View Revisions
2018-12-21 17:36 Zeljan Rikalo Note Added: 0112787
2018-12-21 17:39 Zeljan Rikalo Note Edited: 0112787 View Revisions
2018-12-21 17:40 Zeljan Rikalo Note Edited: 0112787 View Revisions
2018-12-21 17:45 wp Note Added: 0112788
2018-12-21 17:46 wp Assigned To wp => Zeljan Rikalo
2018-12-21 17:46 wp Status feedback => assigned
2018-12-21 17:52 Zeljan Rikalo File Added: ImageListBug.tar.gz
2018-12-21 17:53 Zeljan Rikalo Note Added: 0112789
2018-12-21 18:15 Zeljan Rikalo Note Added: 0112790
2018-12-22 01:37 Ondrej Pokorny Note Added: 0112797
2018-12-22 01:37 Ondrej Pokorny File Added: imglist-02.patch
2018-12-22 10:30 Zeljan Rikalo Note Added: 0112799
2018-12-22 10:43 Zeljan Rikalo Fixed in Revision 59880,59888 => 59892
2018-12-22 10:43 Zeljan Rikalo Note Added: 0112800
2018-12-22 10:43 Zeljan Rikalo Status assigned => resolved
2018-12-22 10:48 Ondrej Pokorny Note Added: 0112801
2018-12-22 11:08 wp Note Added: 0112806
2018-12-22 11:31 Zeljan Rikalo Note Added: 0112807