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

Question

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.. :(

Answer 1

// 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

Answer 2

Finally I have found the real reason for the Heap Corruption that happened in the code.

The put_StreetName/put_CityName of the IAddress/CAddress is designed in a following way.

STDMETHODIMP CAddress::put_CityName(BSTR bstrCityName)
{
    AFX_MANAGE_STATE(AfxGetStaticModuleState())

    m_sCityName.Empty();
    TrimBSTR(bstrCityName);
    m_sCityName = ::SysAllocString(bstrCityName);

    return S_OK;
}

BSTR CAddress::TrimBSTR(BSTR bstrString)
{
    CString sTmpStr(bstrString);
    sTmpStr.TrimLeft();
    sTmpStr.TrimRight();
    SysReAllocString(&bstrString,sTmpStr);  // The Devilish Line
}

The Devilish Line of code is the real culprit that caused the Memory to go hell.

What caused the trouble?

In this line of code, the BSTR string passed as a parameter is from another application and the real memory is in another realm. So the system is trying to reallocate teh string. Either succeed or not, the same is tried to cleared off from the memory in original application/realm, thus causing a crash.

What still unsolved?

Why the same piece of code doesn't crashed a single time in Win XP and older systems? :(

Thanks for all who took their time to answer and solve my problem :)