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;
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);
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;
if (pPropSet)
{
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;
}
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
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.