1

Win32:

With a Tree Control created and the style changed to TVS_CHECKBOXES and then the ImageList for the TVSIL_STATE changed to a custom ImageList, do you need to delete the returned prior ImageList or is it a shared resource and should not be.

MFC:

Since there is an object hierarchy, in this case you don't know if the CImageList is replacing a one provided by the system or from one of the parent classes. In that case what is the proper handling? For ImageLists, can you CImageList::Attach(), CTreeCtrl::SetImageList(), CImageList::Detach() then CTreeCtrl::OnDestroy() go ahead and CImageList *pil=CTreeCtrl::SetImageList(NULL, TVSIL_STATE) and then pil->DeleteImageList(), but then what about the object, are we supposed to delete pil instead? Or are we always required to setup a member variable that is the image list and just CTreeCtrl::SetImageList() to change it then OnDestroy() put back the old one or just set it to NULL?

Jonathan Hall
  • 75,165
  • 16
  • 143
  • 189
user3161924
  • 1,849
  • 18
  • 33
  • 3
    [Beware of the leaked image list when using the TVS_CHECKBOXES style](https://blogs.msdn.microsoft.com/oldnewthing/20171128-00/?p=97475) – Jonathan Potter Dec 18 '18 at 06:36
  • So in the Win32 case, that means YES. What about the MFC case which uses CImageLists ? – user3161924 Dec 18 '18 at 07:16
  • MFC is a pretty thin wrapper so I'd guess it's the same but don't know for sure. – Jonathan Potter Dec 18 '18 at 08:27
  • Check the Task Manager for GDI handles. – Michael Chourdakis Dec 18 '18 at 12:50
  • `CTreeCtrl::SetImageList()` returns a **temporary** object (via `CImageList::FromHandle()`) that does not own the handle it wraps. You have to `DeleteImageList()` to avoid the resource leak (see first comment), but never `delete` on the pointer returned by `SetImageList`. MFC automatically cleans up temporary objects during idle processing. [More about that](https://learn.microsoft.com/en-us/cpp/mfc/tn003-mapping-of-windows-handles-to-objects?view=vs-2017). – zett42 Dec 18 '18 at 13:36
  • @zett42 - Thanks, so if a base class is using a member variable, that handle may have already been deleted when the base class comes time to clean itself up? Does MFC manage references to the handle in some way similar to COM or should base classes ensure they don't use member variables when they attach things like imagelists ? – user3161924 Dec 18 '18 at 18:00
  • Please drop a comment at my answer, if it still leaves things unclear. – zett42 Dec 18 '18 at 21:26

1 Answers1

3

Can changing ImageList used by TVS_CHECKBOXES cause resource leak?

Yes. From Tree-View Control Window Styles:

Destroying the tree-view control does not destroy the check box state image list. You must destroy it explicitly. Get the handle to the state image list by sending the tree-view control a TVM_GETIMAGELIST message. Then destroy the image list with ImageList_Destroy.

Bonus "Old New Thing" link: Beware of the leaked image list when using the TVS_CHECKBOXES style

Win32 Solution

We can take advantage of the fact that TVM_SETIMAGELIST (wrapped by TreeView_SetImageList()) returns the handle to the previous image list.

HIMAGELIST hNewImageList = ImageList_Create(/* insert arguments */);
HIMAGELIST hOldImageList = TreeView_SetImageList( hwndTreeView, hNewImageList, TVSIL_STATE );
if( hOldImageList )  // a good habit, to check if handle is not NULL
{
    ImageList_Destroy( hOldImageList );
    hOldImageList = NULL;
}

After the tree control has been destroyed, you also have to ImageList_Destroy the new image list.

MFC Solution

MFC is not that much different, i. e. it doesn't automatically clean up the image list created by the TVS_CHECKBOXES style.

Create a member variable CImageList m_newImageList; in the declaration of the class that hosts the tree control (e. g. a CDialog derived class). This makes sure that the new image lists lifetime does not end before that of the tree control window and automatically destroys the image list through its destructor.

m_newImageList.Create(/* insert arguments */);
CImageList* pOldImageList = m_treeCtrl.SetImageList( &m_newImageList, TVSIL_STATE );
if( pOldImageList )
{
    pOldImageList->DeleteImageList();
    pOldImageList = nullptr;
}

CTreeCtrl::SetImageList() returns a pointer to a temporary object (via CImageList::FromHandle()) that does not own the handle it wraps. You have to DeleteImageList() to avoid the resource leak, but never delete on the pointer returned by SetImageList. MFC automatically cleans up temporary objects during idle processing (in CWinApp::OnIdle()).

Further reading:

zett42
  • 25,437
  • 3
  • 35
  • 72
  • good info, just one question remains when using a MFC and member variable. say something extends that base class and puts in its own image list using process above. the member variable handle is now invalid in the base? or does it somehow sync that? – user3161924 Dec 19 '18 at 16:20
  • @user3161924 This scenario works without further ado. The base class's member variable is kept in sync through the MFC handle map. When you call `CImageList::Create()`, MFC stores the `HIMAGELIST` and a pointer to `CImageList` in the "permanent" handle map. When you call `SetImageList()`, you will effectively get a pointer to the `CImageList` member variable of the base class. When you call `pOldImageList->DeleteImageList()` as in my code sample, you will delete the image list through the base class's member. – zett42 Dec 22 '18 at 15:02