0

I use C++ Builder XE3. In windows service we have IdTCP server on ( Indy TCP Server ) in function tcp_serverExecute(TIdContext *AContext) - which as I understand spawn new thread.

I create TADOConnection and TADOQuery ( after I call CoInitialize ) problem is no matter what I do application always leak memory unless I use service object as parent for connection and query

  ::CoInitialize(NULL);
  TADOConnection * sql_conn = new TADOConnection(service_object);
  TADOQuery * pos_q = new TADOQuery(service_object);

try
{

}
__finally
{
  delete pos_q;
  delete sql_conn;
  ::CoUninitialize();
}

however if I do use service object as parent I eventually get an exception and application crashes. If I use NULL for parent ( owner ) works just fine just but process keep growing in memory. As far as I am aware and tested if I do similar code in TThread I don't get same issue.

Johan
  • 74,508
  • 24
  • 191
  • 319
Oleg
  • 9
  • 2
  • Why do you think there is a leak? An increase in process memory is not guaranteed to be an indication of a real leak. Remember that the RTL caches and reuses freed memory, it is not returned back to the OS. You might just be seeing memory fragmentation, not memory leaking. If you are not already, you should install [FastMM](http://fastmm.sourrceforge.net) or other memory manager that are designed to prevent fragmentation. – Remy Lebeau Nov 20 '13 at 02:43

2 Answers2

0

You should pass NULL as Owner and delete created objects yourself, Also calling CoInitialize and CoUninitialize inside thread is dangerous, put them in form constructor and destructor:

TADOConnection * sql_conn = new TADOConnection(NULL);
TADOQuery * pos_q = new TADOQuery(NULL);

try
{
}
__finally
{
    delete pos_q;
    delete sql_conn;
}
mh taqia
  • 3,506
  • 1
  • 24
  • 35
  • Yes, you should use a `NULL` Owner, however `CoInitialize/Ex()` and `CoUniitialize()` MUST be called in this situation because this code is running in a worker thread that `TIdTCPServer` creates, so calling them in the Form's constructor/destructor is not an option. ADO uses COM objects, and any worker thread that wants to access COM objects MUST call `CoIntialize/Ex()` to establish the thread's relationship with COM (apartment-threaded vs multi-threaded) before then creating and using the COM objects. – Remy Lebeau Nov 20 '13 at 02:40
0

COM should be initialized only once per thread, but the OnExecute event is triggered multiple times during a client's lifetime.

If you are not using thread pooling with TIdTCPServer (by attaching a TIdSchedulerOfThreadPool component to the TIdTCPServer::Scheduler property), then you can use the TIdTCPServer::OnConnect and TIdTCPServer::OnDisconnect events to initialize/finalize your ADO objects and then use them in the TIdTCPServer::OnExecute event as needed, eg:

class TMyContextData
{
public:
    TADOConnection *sql_conn;
    TADOQuery *pos_q;

    TMyContextData();
    ~TMyContextData();
};

TMyContextData::TMyContextData()
{
    sql_conn = new TADOConnection(NULL);
    pos_q = new TADOQuery(NULL);
}

TMyContextData::~TMyContextData()
{
    delete pos_q;
    delete sql_conn;
}

void __fastcall TMyForm::tcp_serverConnect(TIdContext *AContext)
{
    ::CoInitialize(NULL);
    AContext->Data = new TMyContextData;
}

void __fastcall TMyForm::tcp_serverDisconnect(TIdContext *AContext)
{
    delete static_cast<TMyContextData*>(AContext->Data);
    AContext->Data = NULL;
    ::CoUninitialize();
}

void __fastcall TMyForm::tcp_serverExecute(TIdContext *AContext)
{
    TMyContextData *pData = static_cast<TMyContextData*>(AContext->Data);
    // use pData->sql_conn and pData->pos_q as needed...
 }

Or, derive a new class from TIdServerContext instead:

class TMyContext : public TIdServerContext
{
public:
    TADOConnection *sql_conn;
    TADOQuery *pos_q;

    __fastcall TMyContext(TIdTCPConnection *AConnection, TIdYarn *AYarn, TIdContextThreadList *AList = NULL);
    __fastcall ~TMyContext();
};

__fastcall TMyContext::TMyContext(TIdTCPConnection *AConnection, TIdYarn *AYarn, TIdContextThreadList *AList)
    : TIdServerContext(AConnection, AYarn, AList)
{
    ::CoInitialize(NULL);
    sql_conn = new TADOConnection(NULL);
    pos_q = new TADOQuery(NULL);
}

__fastcall TMyContext::~TMyContext()
{
    delete pos_q;
    delete sql_conn;
    ::CoUninitialize();
}

__fastcall TMyForm::TMyForm(TComponent *Owner)
    : TForm(Owner)
{
    // do this before activating TIdTCPServer
    tcp_server->ContextClass = __classid(TMyContext);
}

void __fastcall TMyForm::tcp_serverExecute(TIdContext *AContext)
{
    TMyContext *pContext = static_cast<TMyContext*>(AContext);
    // use pContext->sql_conn and pContext->pos_q as needed...
}

However, if you are using thread pooling, then multiple clients can be serviced by the same physical thread, so you should move your COM initialization into the actual thread object that manages TIdContext objects (you should also move your ADO objects into the thread so you can reuse them for multiple clients), eg:

class TMyADOThread : public TIdThreadWithTask
{
protected:
    virtual void __fastcall AfterExecute();
    virtual void __fastcall BeforeExecute();

public:
    TADOConnection *sql_conn;
    TADOQuery *pos_q;

    __fastcall TMyADOThread(TIdTask *ATask = NULL, const String AName = "");
};

__fastcall TMyADOThread::TMyADOThread(TIdTask *ATask, const String AName)
    : TIdThreadWithTask(ATask, AName)
{
}

void __fastcall TMyADOThread::BeforeExecute()
{
    TIdThreadWithTask::BeforeExecute();
    ::CoInitialize(NULL);
    sql_conn = new TADOConnection(NULL);
    pos_q = new TADOQuery(NULL);
 }

void __fastcall TMyADOThread::AfterExecute()
{
    delete pos_q;
    delete sql_conn;
    ::CoUninitialize();
    TIdThreadWithTask::AfterExecute();
}

__fastcall TMyForm::TMyForm(TComponent *Owner)
    : TForm(Owner)
{
    // do this before activating TIdTCPServer
    IdSchedulerOfThreadPool1->ThreadClass = __classid(TMyADOThread);
}

void __fastcall TMyForm::tcp_serverExecute(TIdContext *AContext)
{
    TMyADOThread *pThread = static_cast<TMyADOThread*>(static_cast<TIdYarnOfThread*>(AContext->Yarn)->Thread);
    // use pThread->sql_conn and pThread->pos_q as needed...
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770