Search code examples
c++memory-leakscombstr

Memory leak in BSTR to wstring


Consider following code, Is there a memory leak in this?

HRESULT GetPath(wstring &outDomainPath)
{
    CComBSTR bstrDomainPath;

    AnotherGetPath(&bstrDomainPath);
    outDomainPath = bstrDomainPath.Detach();
}

What is the difference in this?( Still memory leak??)

HRESULT GetPath(wstring &outDomainPath)
{
    CComBSTR bstrDomainPath;

    AnotherGetPath(&bstrDomainPath);
    outDomainPath = bstrDomainPath;//removed detach
}

Solution

  • CComBSTR is a RAII wrapper around raw BSTR strings.

    In the first code snippet you posted:

    outDomainPath = bstrDomainPath.Detach();
    

    you invoke CComBSTR::Detach, which releases the ownership of the wrapped BSTR string, and transfers this ownership to "someone else".
    In your case, "someone else" is a std::wstring object (referenced by outDomainPath), which is not a RAII wrapper around BSTRs.

    Now, consider that, from C++ type system perspective, a BSTR is basically a wchar_t*. Moreover, std::wstring implements constructor and assignment operator overloads that take const wchar_t* as input, assuming these raw wchar_t pointers are C-style NUL-terminated strings. So, you can create std::wstring objects from raw C-style strings.

    The problem is that, when you invoke CComBSTR::Detach, the raw BSTR ownership is transferred to the caller, which is responsible to properly release the BSTR string.

    But, in your case, you are creating a wstring object from the wchar_t* (BSTR) returned by CComBSTR::Detach, "orphaning" the returned BSTR string.

    In fact, who will be responsible for properly releasing the BSTR, invoking SysFreeString on it? The std::wstring destructor won't do that.

    So, in this case you leak the BSTR returned by CComBSTR::Detach.


    On the other hand, in the second code snippet:

    outDomainPath = bstrDomainPath;//removed detach
    

    what happens is that the compiler implicitly invokes CComBSTR::operator BSTR, which basically gives the caller access to the BSTR wrapped inside CComBSTR. Note that in this case there is no ownership transfer: you are just observing the BSTR, that is still owned by the CComBSTR RAII wrapper.

    std::wstring has constructor and op= overloads to create wstring objects from raw const wchar_t* NUL-terminated C-style strings. BSTR is basically a wchar_t* from C++ type system perspective, so if your BSTR contains a NUL-terminated string (without embedded NULs inside), that line of code will duplicate the BSTR string, copying it to a std::wstring object.

    Then, at the end of your GetPath function, the local CComBSTR bstrDomainPath object will go out of scope, the CComBSTR destructor will be automatically invoked by the C++ compiler, and SysFreeString will be called to properly release the wrapped BSTR.

    In this case, you have no BSTR leak.

    Please note that this code works if you don't have embedded NULs in your BSTR. In case of embedded NULs, only the first "substring" will be copied, as the wstring constructor that takes a const wchar_t* as input will stop at the first NUL.

    In addition, I encourage you to read this interesting blog post:

    Eric Lippert's Guide to BSTR Semantics