Memory leaks while using CComBstr and system::string

Question

    class ATL_NO_VTABLE CMasterStore : 
    
        public CComObjectRootEx<CComSingleThreadModel>,
    
        public CComCoClass<CMasterStore, &CLSID_MasterStore>,
    
        public IDispatchImpl<IMasterStore, &IID_IMasterStore, &LIBID_PSLOGLib>
    {
    
    STDMETHODIMP CMasterStore::get_Description(BSTR *pVal)
    {
       *pVal = fbstrDescription.Copy();
        return S_OK;
    }
    
    STDMETHODIMP CMasterStore::put_Description(BSTR newVal)
    {
    
       //SetDirty();
    
       fbstrDescription = newVal;
    
        return S_OK;
    }
    };

/* masterStore used below is a c# class as define here

public ref class **MasterStore**
            {
            public:
               property short Code;
               property String^ Description;*/

IMasterStore* _CurrentMasterStore; //interface

This line of code is causing a memory leak:

    _CurrentMasterStore->put_Description(static_cast<BSTR>(Marshal::StringToBSTR(masterStore->Description).ToPointer()));

If I simply pass string as below, I don't see any memory leaks.

    _CurrentMasterRecord->put_Description(L"Test memory leaks");

I am unable to find why it is leaking and how to fix it.

Any help is appreciated.

Thank you very much in advance!!

You allocate a BSTR when you do StringToBSTR. Your code doesn't seem to ever deallocate it. — Simon Mourier, Jan 17 '22 at 16:11
if `masterStore->Description` is the same code we see, then it allocates once (`copy()`). Then you allocates another time using `StringToBSTR` (why? this should already be a `BSTR`), so you have a leak since the `CComBstr` will deallocate once automatically on class dtor, but not twice. — Simon Mourier, Jan 17 '22 at 17:30
CComBSTR desc = DotnetStringToBstr(masterStore->Description); _CurrentMasterStore->put_Description(desc); This is also causing memory leak. — LavanyaBhuma, Jan 18 '22 at 05:04

BertZ · Answer 1 · 2022-01-19T14:51:18.540

0

The StringToBSTR method allocates a BSTR on the system heap for the string and assigns puts the string in the newly allocated BSTR and returns the pointer to the string. Memory has been allocated on the caller's behalf and the caller is responsible for releasing the memory when it is no longer needed (presumably after the call to put_Description)

The second approach probably puts the string in the code as a constant literal so no memory is allocated. BSTRs are typically used when the string is going across a process boundary where as the string literal will not cross the process boundary intact. Also a BSTR is not the same a wide string (L'My string') - see https://social.msdn.microsoft.com/Forums/vstudio/en-US/75a504d1-03b7-4592-8dab-b6897585de6a/bstr-bstrt-sysallocstring-wchar-and-passing-parameters-to-wmi-methods?forum=vcgeneral.

I see you are going from managed string (String^) to a BSTR. https://learn.microsoft.com/en-us/cpp/dotnet/overview-of-marshaling-in-cpp?view=msvc-170 gives a table showing a recommended marshalling method for all kinds of conversions. For System::String^ to BSTR it indicates a marshal_context should be used. https://learn.microsoft.com/en-us/cpp/dotnet/marshal-context-class?redirectedfrom=MSDN&view=msvc-170#marshal-as describes the marshal context and provides an example. Applied to your scenario:

marshal_context^ context = gcnew marshal_context();
BSTR* bstrDescription = context->marshal_as<BSTR*>(masterStore->Description);
_CurrentMasterRecord->put_Description(bstrDescription);
delete context;

Note that the delete context should free up the BSTR so the put_Description should be making a copy of the BSTR and freeing up its previous value if any. Note that the destructor for MasterStore is also responsible for cleaning up the fbstrDescription. Check out the documentation on CComBSTR and consider using that for fbstrDescription - it encapsulates a lot of the memory management for you.

edited Jan 19 '22 at 14:51

answered Jan 17 '22 at 15:30

BertZ

13
4

I have tried this.. BSTR desc = static_cast(Marshal::StringToBSTR(masterStore->Description).ToPointer()); _CurrentMasterStore->put_Description(desc); ::SysFreeString(desc); this didn't help.. – LavanyaBhuma Jan 17 '22 at 15:39
I would use the CComBSTR and let the scope manage the manager. Referring to https://learn.microsoft.com/en-us/cpp/atl/programming-with-ccombstr-atl?redirectedfrom=MSDN&view=msvc-170. CComBSTR desc(L"Test memory leaks); // Constructor has an overload for widestring _CurrentMasterStore->put_Description(desc); // When desc falls out of scope its destructor is called that cleans up the memory. ` – BertZ Jan 17 '22 at 17:13
Typo in the last comment - let the scope manage the MEMORY – BertZ Jan 17 '22 at 17:26
Also, change the put_Description to make a copy of the incoming BSTR or it will be pointing at deallocated memory when desc falls out of scope which would be bad. – BertZ Jan 17 '22 at 17:30
CComBSTR desc = DotnetStringToBstr(masterStore->Description); _CurrentMasterStore->put_Description(desc); This is also causing memory leak. – LavanyaBhuma Jan 18 '22 at 04:58
I see that below line is causing the memory leak.. STDMETHODIMP CMasterStore::put_Description(BSTR newVal) { fbstrDescription = newVal; return S_OK; } I have tried attach also.. fbstrDescription.Attach(newVal); Still its leaking.. – LavanyaBhuma Jan 18 '22 at 10:42
Thank you all for the help. the root cause here was reference count mismatch for masterrecord. After createInstance, I have Addref() call and then QueryInterface() call. AddRef() is also called by QueryInterface() internally. So, reference count was 2. I was calling release() from the caller. But this was not deleting the object as there is still reference count. Fix: After object is handed over to the caller, I have called release(). – LavanyaBhuma Feb 09 '22 at 05:00

Memory leaks while using CComBstr and system::string

1 Answers1