Freeing a BSTR using ::SysFreeString(). More Platform Dependant

bstrcomdcomvisual c++visual-c++-6

I am writing a COM Server which have a plenty of Interfaces and methods. And most of the methods have the BSTR as the parameters and as local parameters used for the return. A snippet looks like

Update 5:

The real code. This fetches from bunch of Data based on a specific condition the DB to populate an array of Object.

STDMETHODIMP CApplication::GetAllAddressByName(BSTR bstrParamName, VARIANT *vAdddresses)
{
    AFX_MANAGE_STATE(AfxGetStaticModuleState())

//check the Database server connection

COleSafeArray saAddress;
HRESULT hr;

// Prepare the SQL Strings dan Query the DB

long lRecCount = table.GetRecordCount();

 if (lRecCount > 0)
 {
    //create one dimension safe array for putting  details
    saAddress.CreateOneDim(VT_DISPATCH,lRecCount);

    IAddress *pIAddress = NULL; 
    //retrieve details 
    for(long iRet = table.MoveFirst(),iCount=0; !iRet; iRet = table.MoveNext(),iCount++)
    {
        CComObject<CAddress> *pAddress;
        hr = CComObject<CAddress>::CreateInstance(&pAddress);
        if (SUCCEEDED(hr))
        {   
            BSTR bstrStreet = ::SysAllocString(table.m_pRecordData->Street);
            pAddress->put_StreetName(bstrStreet);

            BSTR bstrCity = ::SysAllocString(table.m_pRecordData->City);
            pAddress->put_CityName(bstrCity);
        }
        hr = pAddress->QueryInterface(IID_IAddress, (void**)&pIAddress);
        if(SUCCEEDED(hr)) 
        {
            saAddress.PutElement(&iCount,pIAddress); 
        }
    }
    *vAdddresses=saAddress.Detach(); 
}
table.Close(); 
return S_OK;
}


STDMETHODIMP CAddress::put_CityName(BSTR bstrCityName)
{
    AFX_MANAGE_STATE(AfxGetStaticModuleState())
    // m_sCityName is of CComBSTR Type
    m_sCityName.Empty();//free the old string 
    m_sCityName = ::SysAllocString(bstrCityName);//create the memory for the new string
    return S_OK;
}

The problem lies in the Memory Freeing part. The code works very fine in any Win XP machines, but when comes to WIN2K8 R2 and WIN7 the code crashes and pointing to the ::SysFreeString() as the culprit. The MSDN is not adequate to the solution.

Can anyone please help in finding the right solution?

Thanks a lot in advance 🙂

Update 1:

I have tried using the CComBSTR as per the suggestion in the place of raw BSTR, initialized using direct CString's and excluded the SysFreeString(). But for my trouble, on getting out of scope the system is calling the SysFreeString() which again causes the crash 🙁

Update 2:
With the same CComBSTR i tried to allocate using the SysAllocString() , the problem remains same 🙁

Update 3:
I am tired of all the options and in peace I am having only question in mind

Is it necessary to free the BSTR through SysFreeString() which was
allocated using SysAllocString()/string.AllocSysString()?

Update 4:
I missed to provide the information about the crash. When I tried to debug the COM server crashed with a error saying

"Possible Heap Corruption"

. Please help me out of here.. 🙁

Best Answer

// Now All Things are packed in to the Object
obj.Name = bstrName;
obj.Name2 = bstrname2;

I don't quite understand what do you mean by saying that things are packed since you're just copying pointers to the strings, and at the moment when you call SysFreeString obj.Name and obj.Name2 will point to an invalid block of memory. Although this code is not safe, it looks like if the source of your problem is class CFoo. You should show us more details of your code

I suggest you to use a CComBSTR class which will take a responsibility for releasing the memory.

UPDATE

#include <atlbase.h>
using namespace ATL;
...
{
    CComBSTR bstrname(_T("Some Name")); 
    CComBSTR bstrname2(_T("Another Name"));
    // Here one may work with these variables if needed
    ...
    // Copy the local values to the Obj's member Variable 
    bstrname.Copy(&obj.Name); 
    bstrname2.Copy(&obj.Name2);
}

UPDATE2 First of all one should free bstrCity and bstrStreetName with SysFreeString or use CComBSTR instead within this block:

if (SUCCEEDED(hr))
{   
    BSTR bstrStreet = ::SysAllocString(table.m_pRecordData->Street);
    pAddress->put_StreetName(bstrStreet);

    BSTR bstrCity = ::SysAllocString(table.m_pRecordData->City);
    pAddress->put_CityName(bstrCity);

    // SysFreeString(bstrStreet)
    // SysFreeString(bstrCity)
}

Consider to amplify the loop's condition !iRet with iCount < lRecCount.

for(...; !iRet /* && (iCount < lRecCount) */; ...)

Also, here:

m_sCityName = ::SysAllocString(bstrCityName);

you allocate memory but never release it since internally CComBSTR& operator = (OLESTR ..) allocates a new storage itself. One should rewrite is as follows:

m_sCityName = bstrCityName;

Everything else, looks good for me

UPDATE3 Well, Heap corruption is often a consequence of writing some values outside of the allocated memory block. Say you allocate an array of length 5 and put some value to the 6th position

Related Topic