ATL C++ memory leak with safearray of ccomobjects

Question

I find myself in need of help. Now, I'm not all that unfamiliar with C++, but combining it with ATL provides a whole new level of confusion. Anyways, my problem: I (finally) managed to return an array of objects in my COM method to C# caller. But upon 'testing' (running said function a number of times repeatedly) I recognized a small memory leak.

IDL excerpt:

...
interface IDISControl : IDispatch{
    ...
    [id(12)] HRESULT GetNets([out,retval] VARIANT* nets);
};

Header excerpt:

...
STDMETHOD(GetNets)(VARIANT* nets);
...

Code:

STDMETHODIMP CDISControl::GetNets(VARIANT* nets)
{
    SNet *netz;
    int32_t num;
    int result, i;
    result = DIS_GetNetNum(securityHandle, &num);
    netz = new SNet[num];
    result = DIS_GetNet(securityHandle, netz, num); //getting some data

    CComSafeArray<IDispatch*> netArray;
    CComObject<CDISNet> *net;
    CComVariant *var;

    netArray.Create(num, 0);

    for (i = 0;i<num;i++){
        CComObject<CDISNet>::CreateInstance(&net);
        if (net == NULL)
            return S_FALSE; 
        net->AddRef();

        net->Convert(netz[i]);

        netArray[i] = net;
        net->Release(); 
        net = NULL;
    }

    CComVariant val(netArray.Detach());
    val.Detach(nets);

    delete [] netz;
    netArray.Destroy();
    return S_OK;
}

I instantiate CDISNet objects and put some data in them (Convert()). I put them in my safearray and release. As I understand it, the responsibility for destroying them is transferred to safearray. Afterwards, I box the array in a VARIANT so I can fill my [out, retval] parameter. Since it's an out parameter, the responsibility for destruction should be transferred to caller (in my case C#, i.e. its GarbageCollector). I dispose of my dynamic array 'netz' and I destroy safearray wrapper.

So what am I missing? What is left allocated? (This project is really making me appreciate all the comforts of .net).

Help. Please.

EDIT: Further debugging revealed to me that the problem is certainely in my CComObject objects. They aren't being deallocated. If I delete net; in each iteration the array also looses data. I'm unsure as how to rectify that...

EDIT2: Ok, I poked around this code for a bit, and the leak seems to go away when I comment out variant boxing. The problem is that I borrowed this piece of code from Visual Studio sample on safearrays. So, does anyone have any idea what's up with:

CComVariant val(netArray.Detach());
val.Detach(nets);

...and what to do about it?

Maybe the [debug heap](http://msdn.microsoft.com/en-us/library/974tc9t1\(v=vs.80\).aspx) can help. Another thing I'd pay special attention to are your `detach` calls, are your sure that's doing what you think it's doing? — user786653, Oct 14 '11 at 18:30
Well... CComXXX are wrapper classes, Detach should just give the data. CComSafeArrays' Detach gives safearray, CComVariants' Detach gives variant. I'll check out debug heap. — Stonehead, Oct 14 '11 at 19:18
Does `ComObject::CreateInstance` ever fail, because I think you might have a memory leak in that case (if it fails for `i > 0`). — user786653, Oct 15 '11 at 09:58
No, it all works as it should. I debugged the heap and found out that the number of blocks in each iteration increases by four (which is the value of 'num'). I don't know. Sources I could find are very sketchy about these things... — Stonehead, Oct 15 '11 at 11:04

Kim Gräsman · Answer 1 · 2011-12-11T16:59:34.053

Most, if not all, of ATL's wrappers follow COM conventions -- they copy/addref incoming data, as their destructor will destroy/release.

So when you pass your detached SAFEARRAY to CComVariant's constructor, it will make a copy of the SAFEARRAY, which means nobody releases the result from CComSafeArray::Detach.

In cases like this, I always found it easier to forego the wrapper for the return value entirely;

nets->vt = VT_ARRAY | VT_DISPATCH;
nets->parray = netArray.Detach();

The alternative would be to pass your CComSafeArray directly to CComVariant's constructor, without calling Detach, but that would cost you an extra copy. I'd prefer the raw access presented above, as it is most straightforward and cheapest.

As to your first edit, what you're doing with AddRef/Release is fine, if somewhat unnecessary. CComObject::CreateInstance returns an object with reference count 0, so the AddRef will bring it to 1, and then assigning it to the CComSafeArray will bump it to 2, and the following Release back down to 1.

Unless the Convert method does anything with the object's reference count (e.g. QueryInterface itself or pass itself to another COM method), you could skip the AddRef/Release pair, and let Convert execute with refcount == 0. Then adding it to the array would increase it, and it would stay alive until released.

ATL C++ memory leak with safearray of ccomobjects

1 Answers1