Memory Leak Related to _bstr_t and _variant_t

I was helping investigate a memory leak issue the other day. We used LeakDiag to generate some log files and then used LDGrapher to analyze the log files. There were several call stacks shown in LDGrapher. We examined every call stack, starting from the ones that leaked the most.

One call stack points to a method that does something like this, where m_companyName is an instance variable of type _bstr_t :

_bstr_t MyClass::GetCompanyName()
{
   return m_companyName.copy();
}

This method looks innocent at first glance: the _bstr_t::copy() method creates and returns a copy of the underlying BSTR, which is passed into the copy constructor of an _bstr_t object, which is returned to the caller. It seems the ownship of the new copy of BSTR should be taken by the returned _bstr_t object. But why did LeakDiag detect memory leak in this call stack?

We traced into the constructor of _bstr_t and found that it was the _bstr_t::_bstr_t(const wchar_t* s) constructor that got called, which looks as follows.

inline _bstr_t::_bstr_t(const wchar_t* s)
 : m_Data(new Data_t(s))
{
   if (m_Data == NULL)  {
     _com_issue_error(E_OUTOFMEMORY);
   }
}

The constructor of _bstr_t creates a Data_t object by calling the Data_t::Data_t(const wchar_t* s) which allocates a string again!

inline _bstr_t::Data_t::Data_t(const wchar_t* s)
 : m_str(NULL), m_RefCount(1)
{
  m_wstr = ::SysAllocString(s);


   if (m_wstr == NULL && s != NULL) {
     _com_issue_error(E_OUTOFMEMORY);
   }
}

So we ended up with two copies of the original string. The one created by _bstr_t::copy() in MyClass::GetCompanyName() became dangling, causing memory leaks.

To solve this, we could simply return the m_companyName variable in MyClass::GetCompanyName(), or if we really want to create a copy of the string, we should do something like this:

_bstr_t MyClass::GetCompanyName()
{
   _bstr_t t(m_companyName.copy(), false);
   return t;
}

Another leak is caused by something like the following:

VARIANT GetVariant()
{
   VARIANT v;
   VariantInit(&v);
   v.vt = VT_BSTR;
   v.bstrVal = SysAllocString(L"This is a long string.");
   return v;
}
_variant_t t(GetVariant()); // This line causes a leak
std::wcout << (_bstr_t)t << std::endl;

Again, the constructor that takes a VARIANT as a parameter creates a copy of the parameter and keeps the copy instead of the one passed.

inline _variant_t::_variant_t(const VARIANT& varSrc)
{
   ::VariantInit(this);
   _com_util::CheckError(::VariantCopy(this, const_cast<VARIANT*>(&varSrc)));
}

To solve it, we can change the code that calls GetVariant() to

_variant_t t;
t.Attach(GetVariant());

Conclusion

Unlike auto_ptr, the copy constructors and assignment operators of _bstr_t and _variant_t don’t take the ownership of the memory passed in. Instead, they create a copy of it. When we use a smart pointer class, we should pay close attention to how the ownership of a resource is taken and tranfered.

Related Articles

Get updates

Spam-free subscription, we guarantee. This is just a friendly ping when new content is out.

Go back

Your message has been sent

Warning
Warning
Warning.

5 responses to “Memory Leak Related to _bstr_t and _variant_t”

  1. Why not just return m_companyName?

    _bstr_t MyClass::GetCompanyName()
    {
    return m_companyName;
    }

    1. Hi Mun,

      You are right. We can directory return m_companyName instead of creating a copy of it.

      Thanks,
      XueSong

  2. This is also correct:

    _bstr_t sendClientBSTR()
    {
    CComBSTR data;
    getData(&data);
    return _bstr_t(data.Detach(), false);
    }

    1. The problem I am having is that getData is compiled with /clr and is returning a BSTR from managed code. It appears that the returned BSTR is not being cleaned up in unmanaged code

    2. I believe so. 🙂

      Thanks,
      Xuesong

Leave a reply to Mun Cancel reply