views:

61

answers:

2

I'd like to just know if there is a well-established standard way to ensure that one's process doesn't leak COM based resources (such as IMalloc'd objects)?

Take the following code as an example:

HRESULT STDMETHODCALLTYPE CShellBrowserDialog::OnStateChange(__RPC__in_opt IShellView *ppshv, ULONG uChange)
{
    TRACE("ICommDlgBrowser::OnStateChange\n");

    if (uChange == CDBOSC_SELCHANGE)
    {
     CComPtr<IDataObject> data;

     if (ppshv->GetItemObject(SVGIO_SELECTION, IID_IDataObject, (void**)&data) == S_OK )
     {
      UINT cfFormat = RegisterClipboardFormat(CFSTR_SHELLIDLIST);

      FORMATETC fmtetc = { cfFormat, 0, DVASPECT_CONTENT, -1, TYMED_HGLOBAL };
      STGMEDIUM stgmed;

      if (data->GetData(&fmtetc, &stgmed) == S_OK)
      {
       TCHAR path[MAX_PATH];

       // check if this single selection (or multiple)
       CIDA * cida = (CIDA*)stgmed.hGlobal;
       if (cida->cidl == 1)
       {
        const ITEMIDLIST * pidlDirectory = (const ITEMIDLIST *)(((LPBYTE)cida) + cida->aoffset[0]);
        const ITEMIDLIST * pidlFile = (const ITEMIDLIST *)(((LPBYTE)cida) + cida->aoffset[1]);
        ITEMIDLIST * pidl = Pidl_Concatenate(pidlDirectory, pidlFile);

        // we now have the absolute pidl of the currently selected filesystem object
        if (!SHGetPathFromIDList(pidl, path))
         strcpy_s(path, _T("<this object has no path>"));

        // free our absolute pidl
        Pidl_Free(pidl);
       }
       else if (cida->cidl > 1)
        strcpy_s(path, _T("{multiple selection}"));
       else
        strcpy_s(path, _T("-"));

       // trace the current selection
       TRACE(_T("  Current Selection = %s\n"), path);

       // release the data
       ReleaseStgMedium(&stgmed);
      }
     }
    }

    return E_NOTIMPL;
}

So in the above code, I have at least three allocations that occur in code that I call, with only one of them being properly cleaned up automatically. The first is the acquisition of IDataObject pointer, which increments that object's reference count. CComPtr<> takes care of that issue for me.

But then there is IDataObject::GetData, which allocates an HGLOBAL for me. And a utility function Pidl_Concatenate which creates a PIDL for me (code left out, but you can imagine it does the obvious thing, via IMalloc::Alloc()). I have another utility Pidl_Free which releases that memory for me, but must be manually called [which makes the code in question full of exception safety issues (its utterly unsafe as its currently written -- I hate MS's coding mechanics - just asking for memory to fail to be released properly].

I will enhance this block of code to have a PIDL class of some sort, and probably a CIDA class as well, to ensure that they're properly deallocated even in the face of exceptions. But I would still like to know if there is a good utility or idiom for writing COM applications in C++ that can ensure that all IMallocs and AddRef/Dispose are called for that application's lifetime!

A: 

You can not free the global handle returned by IDataObject::GetData, otherwise other programs can not paste from the clipboard after the data is cleaned up. Any pidl you get from shell needs to be freed using IMalloc::Free or ILFree (same effect once OLE32.DLL is loaded into the process). Exceptions are pointers to the middle of item list which can not be freed independently. If you are worried about exceptions, guard your code with try/catch/finally and put the free code in the finally block.

Sheng Jiang 蒋晟
Yes, its important to know when a PIDL is a pointer into some other IIDL, or if its actually allocated of itself.
Mordachai
+2  A: 

Implementing the IMallocSpy interface (see CoRegisterMallocSpy Function) may help get you some of the way.
Note that this is for debugging only, and be careful. There are cautionary tales on the web...

Dan Blanchard
Thanks. I was looking at that interface, and that article points out some important *gotchas*.
Mordachai