Memory Leak using _bstr_t, Can't fix

Question

There is a memory leak in this program I am working on and it's been around for quite some time as far as the commits are concerned. According to these two explanations explanation 1 and explanation 2 whenever using the = assignment operator with _bstr_t causes a memory leak.

Context - there is a database object that is commonly used to do quick sql queries of the database. Every method eventually uses the following method

NvStatus DbUtils::ReadFromDatabase(IUnknown * poNvData,
                                   const std::wstring & oConnectString,
                                   const std::wstring & oSQLStatement)
{
    //some checks
    _bstr_t tbtSQLStr = oSQLStatement.c_str();//memory leak
    _bstr_t tbtConnStr = oConnectString.c_str();//memory leak

    //pass the _bstr_t to another method and get data from DB
    return status;
}

According to articles this method will leak data every time it's called to query the database because of _bstr_t and how they are created. My question is what can I do to prevent the program from blowing up and force garbage collection on the _bstr_t objects?

Microsoft states that it's my responsibility to clean up the memory after using it so how can I do this without corrupting the data being passed to me? I tried doing a deep copy of the string but that failed... Any suggestions would be much appreciated!

After further investigation the two hot spots for my memory leak are the one I posted first and this one, hope this helps

static bool GetValueFromVariant(VARIANT & tvInputValue,
    std::wstring & roOutputValue)
{
    _bstr_t tTemp = tvInputValue.bstrVal;
    if(tTemp.length()>0)
    {
        roOutputValue = (wchar_t*) tTemp;
    }
    return true;
}

Comments suggest that these _bstr_t should clean themselves up automatically... however when debugging the heap size for my windows service, the heap size increases constantly and the debugger continues to point to functions that all use these _bstr_t objects. Clearly these _bstr_t aren't being cleaned up.

More context, the majority of this memory leak stems from creating a COM object over and over again, however I am releasing the object when I am finished with it and i check the reference count returned by Release() function call, it returns 0. Therefore I know that i don't have a build up of COM objects...

Can there be an issue when pointing a wstring to the address of a _bstr_t?

Answer 1

The first lines of your example don't leak memory, since you're assigning a C-style string to a _bstr_t wrapper. Things would look different, if you'd assign a previously allocated BSTR to a _bstr_t. This is the problem, which is described in your second explanation.

Consider the following case:

void foo()
{    
    BSTR s1 = SysAllocString(L"String1");
    _bstr_t s2 = s1;
}

Here, a native BSTR is allocated using SysAllocString and placed into s1. The next line constructs a new BSTR from s1, which is placed in s2. When s2 falls out of scope, its destructor calls SysFreeString, thereby deallocating the copy. The original s1 variable, however, remains intact and will leak.

To remedy this, you'll need to let s2 take ownership of s1:

void foo()
{ 
    BSTR s1 = SysAllocString(L"String1");
    _bstr_t s2(s1, false);
} 

or

void foo()
{
    BSTR s1 = SysAllocString(L"String1");         
    _bstr_t s2;
    s2.Attach(s1);
}

As indicated in my comments, _bstr_t's destructor will call SysFreeString, thereby taking care of deallocating resources.

You may not witness the memory deallocation right away, since BSTRs are usually cached. This behavior can be disabled for debugging purposes by setting an environment variable.

Answer 2

In the first case, simply don't call SysFreeString. There is no memory leak.

The constructor used in:

_bstr_t tbtSQLStr = oSQLStatement.c_str();

creates a copy of the source string, and this copy is freed by the destructor call when tbtSQLStr goes out of scope. The whole point of using a class type wrapper is so that you do not manually need to call SysFreeString.

Note that, depending on the exact nature of the code in the // .... block, you may have not even needed to create this copy of the string.

---

In the second case (which is really a separate question to the first case, you should have posted 2 different questions), there is also no memory leak.

tTemp allocates a copy; roOutputValue copies that copy, and then tTemp's destructor frees the first copy.

However you wasted time by creating and destroying tTemp, you could have just written:

if ( SysStringLen(tvInputValue.bstrVal) > 0 )
    roOutputValue = tvInputValue.bstrVal;

where I'm assuming the "real code" actually checks the variant holds a BSTR at this point.