Crash when thread is deleted

Question

I am developing an application with MFC. The UI thread launch a worker thread and stop it when the app is closing. The issue is that the app is crashing each time it tries to delete the thread.

here is the code :

First the thread class and its implementation :

class FileThread : public CWinThread
{
public:
    static FileThread* CreateWorkerThread(LPVOID params, UINT priority, UINT flags);

    void InitThread();
    void StopThread();
    inline HANDLE GetStopHandle() const { return m_stopThread; }
    inline HANDLE GetWaitHandle() const { return m_waitThread; }

private:
    HANDLE m_stopThread;
    HANDLE m_waitThread;

    FileThread(): m_stopThread(NULL), m_waitThread(NULL) { }

    static UINT MyThreadProc(LPVOID pParam);
};

FileThread* FileThread::CreateWorkerThread(LPVOID params, UINT priority, UINT flags)
{
    return (FileThread*) AfxBeginThread(FileThread::MyThreadProc, params, priority, 0,  flags);
}

void FileThread::InitThread()
{
    m_stopThread = CreateEvent(0, TRUE, FALSE, 0);
    m_waitThread = CreateEvent(0, TRUE, FALSE, 0);
}

void FileThread::StopThread()
{
    ::SetEvent(m_stopThread);
    ::WaitForSingleObject(m_waitThread, INFINITE);
    ::CloseHandle(m_stopThread);
    ::CloseHandle(m_waitThread);
}


UINT FileThread::MyThreadProc(LPVOID pParam)
{
    ThreadData* pLink = (ThreadData*)pParam;
    BOOL continueProcess = TRUE;
    int returnCode = EXITCODE_SUCCESS;

    while (continueProcess)
    {
        if(::WaitForSingleObject(pLink->pMe->GetStopHandle(), 0) == WAIT_OBJECT_0)
        {
            ::SetEvent(pLink->pMe->GetWaitHandle());
            continueProcess = FALSE;
        }

        // the thread is looking for some files...
    }

    delete pLink; // it was allocated from the UI thread

    return returnCode;
}

Then, where I start the thread:

ThreadData * td = new ThreadData();

m_myFileThread = FileThread::CreateWorkerThread((LPVOID)td, THREAD_PRIORITY_LOWEST, CREATE_SUSPENDED);

td->pMe = m_myFileThread;
m_myFileThread->m_bAutoDelete = FALSE;

m_myFileThread->InitThread();
m_myFileThread->ResumeThread();

Finally, the stop (and the crash):

DWORD exitCode;
if (m_myFileThread != NULL && GetExitCodeThread(m_myFileThread->m_hThread, &exitCode) && (exitCode == STILL_ACTIVE))
    {
        m_myFileThread->StopThread();
        if(::WaitForSingleObject(m_myFileThread->m_hThread, 5000) == WAIT_TIMEOUT)
        {
            TerminateThread(m_myFileThread->m_hThread, EXITCODE_ABORT);
        }
    }

if (m_myFileThread != NULL)
{ 
    delete m_myFileThread; // => CRASH
}

It seems I try to delete something already deleted and end up with a heap corruption. I have try to set the m_bAutoDelete to TRUE and not delete the thread myself by I got the same crash (while the program was trying to call AfxEndThread).

The thread terminate its thread proc and return the exit code.

Answer 1

It looks to me like there is a problem here:

FileThread* FileThread::CreateWorkerThread(LPVOID params, UINT priority, 
                                           UINT flags)
{
    return (FileThread*) AfxBeginThread(FileThread::MyThreadProc, params, 
                                        priority, 0,  flags);
}

AfxBeginThread returns a CWinthread*, so just casting this to a derived class of your own does not make it an instance of that derived class. I'm surprised it works at all.

Rather than deriving FileThread from CWinThread, it might be better to hold a CWinthread* member variable inside your wrapper class and expose the thread handle via an accessor if necessary.