Memory Leaks in ATL OLEDB Consumer Objects on CCommand

Introduction

The article reveals a very critical bug in the CCommand ATL OLEDB object. The article explains why it's happening and provides a solution for it.

Background

Microsoft Knowledge Base http://support.microsoft.com/kb/271926 describes a well-known memory leak problem and the way to fix it. However, there is one major memory leak problem not seen reported anywhere. Most likely, nobody uses ATL OLEDB this way. However, it is a typical use scenario in my project as performance is critical.

Who would run the same command for a million times as in this example? I will. The idea is to prepare the command with parameters and run it with different parameters. We use it to write trading orders during the day, real time to a SQL Server. This way, we will have the best performance as you don't need to generate the SQL string again and again.

CDataSource ds;
//Create and open the data source
CDBPropSet dbinit(DBPROPSET_DBINIT);
dbinit.AddProperty(DBPROP_INIT_DATASOURCE, "192.168.60.18");
dbinit.AddProperty(DBPROP_AUTH_USERID, "user");
dbinit.AddProperty(DBPROP_AUTH_PASSWORD, "");
dbinit.AddProperty(DBPROP_INIT_CATALOG, "master");
dbinit.AddProperty(DBPROP_AUTH_PERSIST_SENSITIVE_AUTHINFO, false);
dbinit.AddProperty(DBPROP_INIT_LCID, 1033L);
dbinit.AddProperty(DBPROP_INIT_PROMPT, static_cast<short>(4));

HRESULT hr = ds.Open(_T("SQLOLEDB.1"), &dbinit);
//Create Session.
CSession session;
session.Open(ds);
CCommand<CDynamicAccessor, CRowset, CMultipleResults> command;
hr = command.Create(session, "exec sp_tables");
for(int i=0; i<1000000; i++ )
{
    hr = command.Open(NULL,NULL,false,0);
    command.Close();
}

If you copy the code in your project and run it (please use the correct user and password for your DB environment), you will find in the Task Manager that the memory usage increases in 100Ks a second.

The Bug

CCommand::Open() calls CCommand::ExecuteAndBind and then CCommand::Execute(). This is the implementation of CCommand::Execute():

HRESULT Execute(IUnknown** ppInterface, DBPARAMS* pParams, DBPROPSET *pPropSet, 
DBROWCOUNT* pRowsAffected, ULONG ulPropSets = 0) throw()
{
    HRESULT hr;
    // Specify the properties if we have some
    if (pPropSet)
    {
        // For backward compatibility, if the default parameter is not
        // specified, then set it to one if a property set exists
        if (ulPropSets == 0)
            ulPropSets = 1;

        CComPtr<ICommandProperties> spCommandProperties;
        hr = m_spCommand->QueryInterface(&spCommandProperties);
        if (FAILED(hr))
            return hr;

        hr = spCommandProperties->SetProperties(ulPropSets, pPropSet);
        if (FAILED(hr))
            return hr;
    }

    // If the user want the rows affected then return it back, otherwise
    // just point to our local variable here.

    DBROWCOUNT nAffected, *pAffected;
    if (pRowsAffected)
        pAffected = pRowsAffected;
    else
        pAffected = &nAffected;

    if (UseMultipleResults())
    {
        hr = m_spCommand->Execute(NULL, __uuidof(IMultipleResults), pParams,
            pAffected, (IUnknown**)GetMultiplePtrAddress());

        if (SUCCEEDED(hr))
            hr = GetNextResult(pAffected, false);
        else
            // If we can't get IMultipleResults then just try to get IRowset
            hr = m_spCommand->Execute(NULL, GetIID(), pParams, pAffected,
                ppInterface);
    }
    else
    {
        hr = m_spCommand->Execute(NULL, GetIID(), pParams, pAffected,
            ppInterface);
    }

    if (SUCCEEDED(hr))
        SetupOptionalRowsetInterfaces();
    return hr;
}

The line in bold is the source of the problem.

hr = m_spCommand->Execute(NULL, __uuidof(IMultipleResults), pParams,
         pAffected, (IUnknown**)GetMultiplePtrAddress());

This code doesn't check if GetMultiplePrtAddress, in this case ComPtr<IMultipleResults> m_spMultipleResults, is previously set or not. If it's set previously, this interface pointer should be released first before being assigning with the new interface. Although the interface is protected in a smart pointer ComPtr, all previous interface pointers are lost except the last interface pointer. So, in the end, the last interface is correctly released.

Solution

Release this interface explicitly every time after the command closes.

for(int i=0; i<1000000; i++ )
{
    hr = command.Open(NULL,NULL,false,0);
    command.Close();
    if( command.GetMultiplePtr() != NULL )
    {
    command.GetMultiplePtr()->Release();
    *command.GetMultiplePtrAddress() = NULL;
    }
}

To make it nice, you can override CCommand::Close() and put the fix in as an extra.