tags:

views:

579

answers:

3

I have an out-of-process COM server written in C++, which is called by some C# client code. A method on one of the server's interfaces returns a large BSTR to the client, and I suspect that this is causing a memory leak. The code works, but I am looking for help with marshalling-out BSTRs.

Simplifying a bit, the IDL for the server method is

HRESULT ProcessRequest([in] BSTR request, [out] BSTR* pResponse);

and the implementation looks like:

HRESULT MyClass::ProcessRequest(BSTR request, BSTR* pResponse)
{
    USES_CONVERSION;
    char* pszRequest = OLE2A(request);
    char* pszResponse = BuildResponse(pszRequest);
    delete pszRequest;
    *pResponse = A2BSTR(pszResponse);
    delete pszResponse;
    return S_OK;
}

A2BSTR internally allocates the BSTR using SysAllocStringLen().

In the C# client I simply do the following:

string request = "something";
string response = "";
myserver.ProcessRequest(request, out response);
DoSomething(response);

This works, in that request strings get sent to the COM server and correct response strings are returned to the C# client. But every round trip to the server leaks memory in the server process. The crt leak detection support is showing no significant leaks on the crt heap, so I'm suspecting the leak was allocated with IMalloc.

Am I doing anything wrong here? I have found vague comments that 'all out parameters must be allocated with CoTaskMemAlloc, otherwise the interop marshaller won't free them' but no details.

Andy

A: 

I guess you need to destroy request with ::SysFreeString(). This memory is allocated on server side.

Also, OLE2A may allocate memory due to conversion (take a look). You do not free it also.

bocco
Thanks for responding.Re destroying 'request', I thought [in] parameters were allocated and freed by the caller?
Andy Johnson
No, per COM rules the callee never frees [in] parameters.
anelson
OLE2A uses _alloca, which allocates memory dynamically on the stack. It can be dangerous, if the string is of unknown size, but no deallocation is necessary -- stack space is reclaimed when the function ends.
Kim Gräsman
+1  A: 

I don't see an obvious problem with your code. Suggest you modify the ProcessRequest method to rule out COM interop as the source of the leak:

HRESULT MyClass::ProcessRequest(BSTR request, BSTR* pResponse)
{
    *psResponse = ::SysAllocStringLen(L"[suitably long string here]");
    return S_OK;
}

I suspect that won't leak, in which case you've narrowed the leak to your code.

I'd also note the OLE2A allocates memory on the stack, so not only should you not delete pszRequest, but you shouldn't use OLE2A at all, due to the possibility of stack overflow. See this article for safer alternatives.

I'd also suggest you replace A2BSTR with ::SysAllocString(CA2W(pszResponse))

anelson
Thanks. Its looking like the leak is not in the COM memory handling, although I'm not sure where. Time to try boundschecker...
Andy Johnson
+1  A: 

anelson has covered this pretty well, but I wanted to add a couple of points;

  • CoTaskMemAlloc is not the only COM-friendly allocator -- BSTRs are recognized by the default marshaller, and will be freed/re-allocated using SysAllocString & friends.

  • Avoiding USES_CONVERSION (due to stack overflow risks -- see anelson's answer), your full code should be something like this [1]

(note that A2BSTR is safe to use, as it calls SysAllocString after conversion, and doesn't use dynamic stack allocation. Also, use array-delete (delete[]) as BuildResponse likely allocates an array of chars)

  • The BSTR allocator has a cache that can make it appear as though there is a memory leak. See http://support.microsoft.com/kb/139071 for some details, or Google for OANOCACHE. You could try disabling the cache and see if the 'leak' goes away.

[1]

HRESULT MyClass::ProcessRequest(BSTR request, BSTR* pResponse)
{
    char* pszResponse = BuildResponse(CW2A(request));
    *pResponse = A2BSTR(pszResponse);
    delete[] pszResponse;
    return S_OK;
}
Kim Gräsman
Thanks. I've made the changes you suggested. Looks like the leak is elsewere though.
Andy Johnson