View Issue Details

IDProjectCategoryView StatusLast Update
0035684LazarusLCLpublic2019-10-07 09:43
ReporterserbodAssigned ToJuha Manninen 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Platformx86_64OSWindowsOS Version7
Product Version2.0.2Product Build 
Target VersionFixed in Version 
Summary0035684: Memory leak on TImageList.GetIcon()
DescriptionBoth memory and GDI objects (handles) leaks when icon assigned from TImageList to TImage.
Steps To Reproduce1. Create application with visual form
2. Add TImage
3. Add TImageList with 2 icons
3. Add TTimer with Interval=1
4. In OnTimer put code:
====
  ImageList1.GetIcon(Image1.Tag, Image1.Picture.Icon);
  Image1.Tag := Image1.Tag + 1;
  if Image1.Tag >= 2 then Image1.Tag := 0;
====
5. Run program and look at resource counters in system task manager
TagsNo tags attached.
Fixed in Revisionr61624
LazTarget-
WidgetsetWin32/Win64
Attached Files
  • GetIconMemleak.zip (69,846 bytes)
  • imglist.inc.patch (446 bytes)
    Index: lcl/include/imglist.inc
    ===================================================================
    --- lcl/include/imglist.inc	(revision 60715)
    +++ lcl/include/imglist.inc	(working copy)
    @@ -580,6 +580,8 @@
         ListImg.Free;
       end;
       Image.Handle := CreateIconIndirect(@IconInfo);
    +  if IconInfo.hbmColor<>0 then DeleteObject(IconInfo.hbmColor);
    +  if IconInfo.hbmMask<>0  then DeleteObject(IconInfo.hbmMask);
     
       RawImg.FreeData;
     end;
    
    imglist.inc.patch (446 bytes)
  • imglist.inc-2.patch (823 bytes)
    Index: lcl/include/imglist.inc
    ===================================================================
    --- lcl/include/imglist.inc	(revision 60715)
    +++ lcl/include/imglist.inc	(working copy)
    @@ -565,8 +565,8 @@
       GetRawImage(Index, RawImg);
       RawImg.PerformEffect(AEffect, True);
     
    +  FillChar(IconInfo, sizeof(TIconInfo), 0);
       IconInfo.fIcon := True;
    -  IconInfo.hbmMask := 0;
       if not CreateCompatibleBitmaps(RawImg, IconInfo.hbmColor, IconInfo.hbmMask, True)
       then begin
         // bummer, the widgetset doesn't support our 32bit format, try device
    @@ -580,6 +580,10 @@
         ListImg.Free;
       end;
       Image.Handle := CreateIconIndirect(@IconInfo);
    +  if IconInfo.hbmColor<>0 then
    +    DeleteObject(IconInfo.hbmColor);
    +  if IconInfo.hbmMask<>0  then
    +    DeleteObject(IconInfo.hbmMask);
     
       RawImg.FreeData;
     end;
    
    imglist.inc-2.patch (823 bytes)

Relationships

related to 0028577 resolvedJuha Manninen Patches GDI handle/memory leak in image list 
related to 0029901 resolvedJuha Manninen Lazarus TImageList.GetIcon() memory leak 
has duplicate 0035888 closedJuha Manninen Lazarus 0035684: Memory leak on TImageList.GetIcon() 

Activities

serbod

2019-06-07 14:10

reporter   ~0116604

Sample project

GetIconMemleak.zip (69,846 bytes)

BrunoK

2019-06-10 19:41

reporter   ~0116662

Last edited: 2019-06-10 19:42

View 2 revisions

Possible patch (maybe similar problem as 0028577 that has been resolved).

Seems to work ok, No more GDI rising.

Issue 0029901 also leaks GDI resources on Windows, patch seems to clear that too.



imglist.inc.patch (446 bytes)
Index: lcl/include/imglist.inc
===================================================================
--- lcl/include/imglist.inc	(revision 60715)
+++ lcl/include/imglist.inc	(working copy)
@@ -580,6 +580,8 @@
     ListImg.Free;
   end;
   Image.Handle := CreateIconIndirect(@IconInfo);
+  if IconInfo.hbmColor<>0 then DeleteObject(IconInfo.hbmColor);
+  if IconInfo.hbmMask<>0  then DeleteObject(IconInfo.hbmMask);
 
   RawImg.FreeData;
 end;
imglist.inc.patch (446 bytes)

jamie philbrook

2019-06-11 22:54

reporter   ~0116679

I just ran the test app without the "imglist.inc.patch and there is no leak on my end.
However, I did fix the "LoadFromRawImage" method which was leaking and did cause a few gdi controls
to leak memory..
  And this is with laz 2.0.2 but a slightly fixed version, not the one on the down load..
  The Trunk should have this fix too.

BrunoK

2019-06-12 10:30

reporter   ~0116685

@jamie philbrook 2019-06-11 22:54
The leak reported here is not detectable with heaptrc. It is not related to issue 0035372.

Original serbod's report is about :
5. Run program and look at resource counters in SYSTEM TASK MANAGER.
GDI objects, in Windows, are allocated in Process memory but not thru the FPC heap manager.
See https://blogs.msdn.microsoft.com/dsui_team/2013/04/23/debugging-a-gdi-resource-leak.

I did know about 3 potential source of leaks these were :

In FPC(3.0.4)/Lazarus applications there are 2 types of user leaks :
1 - Allocated but non free'd TOBject.Create / GetMem, these are catched by heaptrc.
2 - Invisible usage leaks. Those are TComponent descendants that are created multiple times but actually use only the last created instance. An example is .\laztools\debugserver\frmmain.pp that does in TMainForm.ShowOptions TOptionsForm.Create+ShowModal. The TOptionsForm instance is not freed after showmodal so every time you call the ShowOptions you have one more instance allocated.
There will be no memory leak reported because when the TMainForm instance is freed all its components, including all additional TOptionsForm instances, will be freed.

3 - In FPC(3.0.4) I detected one compiler generated code leak (in the compiler itself) regarding const ansistring function parameters. I couldn't generate a simple demonstration for that, so I didn't report it.

Now a new possibility of leaks in Program memory space is added to the list :
4 - Forgetting to release GDI objects after use.

Michael Van Canneyt

2019-06-24 12:46

administrator   ~0116887

@BrunoK:
I fixed the type 2 memleak in debugserver.

BrunoK

2019-07-26 10:11

reporter   ~0117406

Improved patch : initialize IconInfo

Reason : IconInfo being on the stack it may contain undefined values.

imglist.inc-2.patch (823 bytes)
Index: lcl/include/imglist.inc
===================================================================
--- lcl/include/imglist.inc	(revision 60715)
+++ lcl/include/imglist.inc	(working copy)
@@ -565,8 +565,8 @@
   GetRawImage(Index, RawImg);
   RawImg.PerformEffect(AEffect, True);
 
+  FillChar(IconInfo, sizeof(TIconInfo), 0);
   IconInfo.fIcon := True;
-  IconInfo.hbmMask := 0;
   if not CreateCompatibleBitmaps(RawImg, IconInfo.hbmColor, IconInfo.hbmMask, True)
   then begin
     // bummer, the widgetset doesn't support our 32bit format, try device
@@ -580,6 +580,10 @@
     ListImg.Free;
   end;
   Image.Handle := CreateIconIndirect(@IconInfo);
+  if IconInfo.hbmColor<>0 then
+    DeleteObject(IconInfo.hbmColor);
+  if IconInfo.hbmMask<>0  then
+    DeleteObject(IconInfo.hbmMask);
 
   RawImg.FreeData;
 end;
imglist.inc-2.patch (823 bytes)

Juha Manninen

2019-07-26 11:09

developer   ~0117407

I applied the second patch. Thanks. It looks safe and I will add it to the merge list for 2.0.4.

BrunoK

2019-07-26 11:11

reporter   ~0117408

This patch is intended to solve a leak in
procedure TCustomImageListResolution.GetIcon(Index: Integer; Image: TIcon;
  AEffect: TGraphicsDrawEffect);

NOT THE SAME bug as resolved by r49777 in
procedure TCustomImageListResolution.ReadData(AStream: TStream);
  procedure CreateImagesFromRawImage(IntfImage: TLazIntfImage; NewCount: integer);

BrunoK

2019-07-26 11:14

reporter   ~0117410

In short NOT FIXED !

Juha Manninen

2019-07-26 11:17

developer   ~0117411

Last edited: 2019-07-26 11:18

View 2 revisions

BrunoK, r49777 is very old. Why do you mention it here? Should we adjust it somehow?
What is not fixed?

BrunoK

2019-07-26 11:59

reporter   ~0117415

@Juha Manninen

It is OK now, thanks.

I was following your related issues in the svn and you hadn't yet applied the changes. Look at the time stamps, we are in the same time zone, but for some reason our times are not fully synchronized.

Juha Manninen

2019-07-26 16:24

developer   ~0117419

So it works now. Cool, thanks!

serbod

2019-10-07 09:43

reporter   ~0118389

Tested under Windows, ОК.

Issue History

Date Modified Username Field Change
2019-06-07 14:05 serbod New Issue
2019-06-07 14:10 serbod File Added: GetIconMemleak.zip
2019-06-07 14:10 serbod Note Added: 0116604
2019-06-10 19:41 BrunoK File Added: imglist.inc.patch
2019-06-10 19:41 BrunoK Note Added: 0116662
2019-06-10 19:42 BrunoK Note Edited: 0116662 View Revisions
2019-06-11 22:54 jamie philbrook Note Added: 0116679
2019-06-12 10:30 BrunoK Note Added: 0116685
2019-06-24 12:46 Michael Van Canneyt Note Added: 0116887
2019-07-26 10:11 BrunoK File Added: imglist.inc-2.patch
2019-07-26 10:11 BrunoK Note Added: 0117406
2019-07-26 10:36 Juha Manninen Relationship added related to 0028577
2019-07-26 10:37 Juha Manninen Relationship added related to 0029901
2019-07-26 11:04 Juha Manninen Assigned To => Juha Manninen
2019-07-26 11:04 Juha Manninen Status new => assigned
2019-07-26 11:09 Juha Manninen Status assigned => resolved
2019-07-26 11:09 Juha Manninen Resolution open => fixed
2019-07-26 11:09 Juha Manninen Fixed in Revision => r61624
2019-07-26 11:09 Juha Manninen LazTarget => -
2019-07-26 11:09 Juha Manninen Widgetset Win32/Win64 => Win32/Win64
2019-07-26 11:09 Juha Manninen Note Added: 0117407
2019-07-26 11:11 BrunoK Note Added: 0117408
2019-07-26 11:14 BrunoK Note Added: 0117410
2019-07-26 11:17 Juha Manninen Note Added: 0117411
2019-07-26 11:18 Juha Manninen Note Edited: 0117411 View Revisions
2019-07-26 11:59 BrunoK Note Added: 0117415
2019-07-26 16:24 Juha Manninen Note Added: 0117419
2019-07-26 17:09 Juha Manninen Relationship added has duplicate 0035888
2019-10-07 09:43 serbod Status resolved => closed
2019-10-07 09:43 serbod Note Added: 0118389