Search code examples
winapimfcimagelist

Can changing ImageList used by TVS_CHECKBOXES cause resource leak?


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?


Solution

  • 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: