Worker thread termination in MFC

Question

What is the correct way to terminate a worker thread if it is taking too long to complete? I've read several articles claming that TerminateThread should be used with extreme caution, but I can't find any viable alternative.

Psudo code:

void CMyDialog::RunThread()
{
    CWinThread* pThread;            // pointer to thread
    DWORD dwWaitResult;             // result of waiting for thread


    // start thread
    pThread = AfxBeginThread(beginThread, this, 
                           THREAD_PRIORITY_NORMAL, 
                           0,     
                           CREATE_SUSPENDED);
    pThread->m_bAutoDelete = FALSE;
    pThread->ResumeThread();

    // wait for thread to return
    dwWaitResult = ::WaitForSingleObject(pThread->m_hThread, (30 * 1000)); 

    switch (dwWaitResult) 
    {
        case WAIT_OBJECT_0: 

            delete pThread;

            // success, continue

            break;

        case WAIT_TIMEOUT: 

            // thread taking too long, terminate it
            TerminateThread(pThread->m_hThread, 0);
            delete pThread;

            break;

    } // end switch on wait result


} 


UINT CMyDialog::beginThread(LPVOID pParam)
{
    // convert parameter back to dialog object and call method
    CMyDialog* dlg = (CMyDialog*) pParam;
    dlg->readDuration();

    return 0;
} // end beginThread


void CMyDialog::readDuration()
{
    // call a dll function that may take longer than we are prepared to wait, 
    // or even hang

} // end readDuration

Is this acceptable? All comments and suggestions gratefully recieved.

I am using MFC/C++ in Visual Studio 2008. Developing on Vista, targeting XP, Vista and 7.

Answer 1

Is unsafe to terminate a thread, as Sanja already mentioned. The typical solution in such cases is to spawn a child process that only role is to host the DLL and call the method(s). You main process will communicate with the child process via some LPC mechanism to pass in the arguments for the DLL method invocation and get back the result. On timeout is perfectly safe to kill the child process, the kernel will reclaim all resources and there will be no in-memory or system object leaks (there could be persisted on-disk leaks, like files left over, though). It is significantly more complicated that just simply calling the DLL (you'll need to come up with the inter-process communication solution) but is the only reliable way.

Answer 2

Its a bad idea to use TerminateThread its not safe and can cause some leaks. You can use events to tell your thread end. Some useful links

1. http://www.codeproject.com/KB/threads/Synchronization.aspx

2. http://msdn.microsoft.com/en-us/library/ms686915(v=vs.85).aspx

Good answer about terminatethread here