locked
File Icon Memory Leak RRS feed

  • General discussion

  • Hi there.

    I'm developing an application launcher for Microsoft Windows, which indexes all files in specific user defined folders. The application launcher displays the icon of the file that the user is looking for, while inserting text.

    So I have an IconManager class that stores file icons in a cache so the application doesn't have to be always reading from the hard disk.

    1// File: IconManager.cs 
    2 
    3public void AddIcon(string path) 
    4
    5    try 
    6    { 
    7        IntPtr ptr = SearchFileIconPtr(path); 
    8        Icon temp_icon = Icon.FromHandle(ptr); 
    9        Icon icon = (Icon)temp_icon.Clone(); 
    10        DestroyIconPtr(ptr); 
    11        _cache.RegisterIcon(path, icon); 
    12    } 
    13    catch (Exception) 
    14    { 
    15        _cache.RegisterIcon(path, null); // problema!!!! 
    16    } 
    17
    18 
    19private IntPtr SearchFileIconPtr(string file_path) 
    20
    21    SHFILEINFO shfileinfo = new SHFILEINFO(); 
    22    Win32.SHGetFileInfo(file_path, Win32.SHELL32_FILE_ATTRIBUTE_NORMAL, ref shfileinfo, (uint)Marshal.SizeOf(shfileinfo), 
    23                                        Win32.SHGFI_ICON); // Win32.SHGFI_ICON | Win32.SHGFI_LARGEICON 
    24    return shfileinfo.hIcon; 
    25
    26 
    27private bool DestroyIconPtr(IntPtr ptr) 
    28
    29    return Win32.DestroyIcon(ptr); 
    30
    31 
    32// File: Win32.cs 
    33 
    34[StructLayout(LayoutKind.Sequential)] 
    35public struct SHFILEINFO 
    36
    37    public IntPtr hIcon; 
    38    public IntPtr iIcon; 
    39    public uint dwAtrributes; 
    40    [MarshalAs(UnmanagedType.ByValTStr, SizeConst = 260)] 
    41    public string szDisplayName; 
    42    [MarshalAs(UnmanagedType.ByValTStr, SizeConst = 80)] 
    43    public string szTypeName; 
    44}; 
    45 
    46public const uint SHGFI_ICON = 0x100
    47public const uint SHGFI_LARGEICON = 0x0
    48public const uint SHGFI_SMALLICON = 0x1
    49public const uint SHELL32_FILE_ATTRIBUTE_NORMAL = 0
    50public const uint SHELL32_FILE_ATTRIBUTE_DIRECTORY = 0x10
    51 
    52[DllImport("shell32.dll", CharSetCharSet = CharSet.Auto)] 
    53public static extern IntPtr SHGetFileInfo( 
    54    string pszPath, 
    55    uint dwFileAttributes, 
    56    ref SHFILEINFO psfi, 
    57    uint cbSizeFileInfo, 
    58    uint uFlags); 
    59 
    60[DllImport("user32.dll", EntryPoint = "DestroyIcon"
    61    CharSetCharSet = CharSet.Auto, SetLastError=true)] 
    62[return: MarshalAs(UnmanagedType.Bool)] 
    63public static extern bool DestroyIcon(IntPtr handle); 


    So, as you can see, I use the shell32.dll method 'SHGetFileInfo' to get the icon pointer and the user32.dll method 'DestroyIcon' to destroy the same pointer, so it just doesn't stays there holding memory.
    The AddIcon method is called every time the user rebuilds the index, for each file. So, it's called about 500 times (average), regarding that a regular user (my case, for example) is indexing about 500 files.

    However I noticed that the following block of code, from AddIcon, was causing a memory leak:

    1IntPtr ptr = SearchFileIconPtr(path); 
    2Icon temp_icon = Icon.FromHandle(ptr); 
    3Icon icon = (Icon)temp_icon.Clone(); 
    4DestroyIconPtr(ptr); 

    Then, I just started to suspect that either SHGetFileInfo or DestroyIcon might be the cause for this leak. So I overwritten the previous block of code with this one:

    1Icon icon = (Icon)Properties.Resources.trash.Clone(); 

    And the leak was gone.

    So, just to make sure, I replaced, again, the previous block of code with the following one:

    1IntPtr ptr = SearchFileIconPtr(path); 
    2DestroyIconPtr(ptr); 
    3Icon icon = (Icon)Properties.Resources.trash.Clone(); 

    ... and the Leak was back again. So, either the 'SHGetFileInfo' or the 'DestroyIcon' are not working well, or I have a much major problem that I'm not aware of. Can anyone give me a help, trying to find the memory leak here?

    Thanks in advance.

    PS: Properties.Resources.trash is a test icon that I imported to the resources.
    • Changed type Bruce.Zhou Tuesday, January 20, 2009 5:10 AM Wait for coming back
    Tuesday, January 13, 2009 10:41 PM

All replies

  • Hmya, having P/Invoke code leak handles is a pretty common problem, especially when you don't check API function return values so they can fail without noticing.  That's why Microsoft wrote Icon.ExtractAssociatedIcon().
    Hans Passant.
    Wednesday, January 14, 2009 1:50 AM
  • Thanks. I really wasn't aware of that useful method.

    Still, the leak is there.

    This code doesn't leaks any memory:

    1 public void AddIconByPtr(string path) 
    2
    3     try 
    4     { 
    5         Icon icon = (Icon)Properties.Resources.trash.Clone(); 
    6         _cache.RegisterIcon(path, icon); 
    7     } 
    8     catch (Exception) 
    9     { 
    10         _cache.RegisterIcon(path, null); // problema!!!! 
    11     } 
    12


    This code leaks about 350kB:

    1 public void AddIconByPtr(string path) 
    2
    3     try 
    4     { 
    5         Icon leak = Icon.ExtractAssociatedIcon(path); 
    6         leak.Dispose(); 
    7          
    8         Icon icon = (Icon)Properties.Resources.trash.Clone(); 
    9         _cache.RegisterIcon(path, icon); 
    10     } 
    11     catch (Exception) 
    12     { 
    13         _cache.RegisterIcon(path, null); // problema!!!! 
    14     } 
    15


    Notice that both do the same and that I don't use the icon created by Icon.ExtractAssociatedIcon().

    I know that 350kB at every index rebuild may not seem very harmful. But if you leave your computer running the whole day, with the index being rebuilt every 15 minutes, in the end it will have a huge amount of stuck memory.

    How should I solve this?

    Thanks for any help.

    PS: I tried to call GC.Collect() inside AddIconByPtr, but it didn’t help much.
    Wednesday, January 14, 2009 3:34 AM
  • What do you call a leak?  A real leak is your process' Handle count going up without bound.  350 KB is not a leak, that is memory getting used to store icons.  That memory doesn't get released just because you dispose an icon.  The Windows memory manager and the garbage collector are far too sophisticated to guess their behavior from looking at TaskMgr.exe.  Minimize your form to make you feel better.  Leave your computer running for a whole day, come back when it crashes with OOM.
    Hans Passant.
    Wednesday, January 14, 2009 4:17 AM
  • Hi there. I left my application running during sleep time (about 10 hours). When it first started, it was consuming about 33mb. When I woke up, it was consuming about 107mb. All it was doing was rebuilding the index every 15 minutes, which calls that AddIconByPtr where I spotted the leak. So, I really think it is a leak.

    Thanks for any help.

    Wednesday, January 14, 2009 3:20 PM
  • You actually left it at 15 minutes?  Rebuild the index in an endless loop so you do it every millisecond.  If you get OOM, post a small version of your project the reproduces the leak on a file sharing service so we can take a look at it.

    Hans Passant.
    Wednesday, January 14, 2009 7:46 PM
  • nobugz said:

    You actually left it at 15 minutes?  Rebuild the index in an endless loop so you do it every millisecond.  If you get OOM, post a small version of your project the reproduces the leak on a file sharing service so we can take a look at it.


    Hans Passant.


    Yes I did.

    I just ran my app for 50 minutes, rebuilding the index on an endless loop. It started on 33mb and now is stuck on 480.620K. It doesn't go further then 480mb but, still, it is a lot of leaked memory. I will build a smaller version of my project and post it on a sharing service so you can take a look. I should post it till the weekend.

    Thanks.

    Wednesday, January 14, 2009 9:10 PM
  • Hi RevengerPT,

    I got to know you are going to share a smaller version of your project from your last post. Has the project got prepared?

    Best regards,
    Bruce Zhou

    Please mark the replies as answers if they help and unmark if they don't.
    Monday, January 19, 2009 8:03 AM
  • Hi Bruce.Zhou,

    My project is about 1/3 prepared, but I got a PM exam next Monday and I really have to focus on studying for it. So, I should get it ready in the end of the next week. I'm sorry for the delay.

    Regards.
    Monday, January 19, 2009 10:31 PM
  • Hi RevengerPT,

    We are temporarily changing the type of this case to "General Discussion". When the eaxm is finished and you come back, please feel free to change the type to "Question" again. We will also change the type to "Question" if you come back.

    Best regards,
    Bruce Zhou

    Please mark the replies as answers if they help and unmark if they don't.
    Tuesday, January 20, 2009 5:09 AM
  • I noticed what I believe is a similar issue with SHGetFileInfo() where I request the system shell icon index via a pidl. I came across a file that when SHGetFileInfo() is used to get the shell icon my application memory jumps to nearly 500mb. I tested a few other applications that implement shell file type browsers and they too produce the same results.

    I can provide someone the file that triggers the issue, I have confirmed the problem on Windows 7 with multiple computers.

    Wednesday, June 8, 2011 11:28 PM