1
votes

I am working on some legacy code which uses MFC's UI Threads to implement a manager thread-worker thread mechanism. The code used to run under an MFC GUI application but now its in a separate dll and is run both from the GUI application and from a console application.

The manager thread, worker threads and main application communicate via Thread messages (the worker thread doesn't really need to send messages to the manager thread but this is the way it was implemented originally and worked, so there you go).

Now, when I run my code from the console app, messages sent from the main thread to the manager thread are handled and my handler is called. It is only when I try to send a message from the manager thread to the worker threads that I have problems. The call to PostThreadMessage succeeds but the handler is never invoked. This behavior was reproduced in both a plain-old console application as well as in a Win32 console application (that includes a pre-compiled header with all the MFC goodies).

I found this old Microsoft article: http://support.microsoft.com/kb/142415 but I have to admit I didn't really understand it. I tried as it suggests to override the PreTranslateMessage function and to explicitly handle my custom messages there but the function was never invoked after calling PostThreadMessage

I tried to reproduce the problem in the sample below and in my sample even the message to the manager thread is never handled, which confirms my suspicion that I'm doing something wrong.

EDIT: I added the missing InitInstance and ExitInstance to overload ManagerThread that was missing from my sample code, as MarsRover suggested and indeed ManagerThread messages are now pumped, but WorkerThread messages are not, which acurately reproduces the problem I'm having in my original code. Sample code:

//Common.h

//update the progress message
#define WM_START_RUN (WM_USER + 1)

//update the progress message
#define WM_JOB_DONE (WM_USER + 2)

//run thread has finished
#define WM_RUN    (WM_USER + 3)

// ManagerThread.h
class ManagerThread : public CWinThread
{
    DECLARE_DYNCREATE(ManagerThread)
protected:
    ManagerThread(){}           // protected constructor used by dynamic creation
    virtual ~ManagerThread();
    BOOL InitInstance();
    int ExitInstance();
    std::vector<WorkerThread*> m_WorkerThreads;
    int numOfJobs;
    DECLARE_MESSAGE_MAP()
    afx_msg void OnStartRun(WPARAM wParam, LPARAM lParam);
    afx_msg void OnJobDone(WPARAM wParam, LPARAM lParam);
    afx_msg void OnQuit(WPARAM wParam, LPARAM lParam);
};

//WorkerThread.h
class WorkerThread : public CWinThread
{
    DECLARE_DYNCREATE(WorkerThread)

protected:
    WorkerThread(){}        // protected constructor used by dynamic creation
    virtual ~WorkerThread(){}
    virtual BOOL InitInstance();
    virtual int ExitInstance();

public:
    void SetManager(CWinThread* pManager) {m_Manager = pManager;}
    void SetID(int _id) {id = _id;}
protected:
    int id;
    CWinThread* m_Manager;
    DECLARE_MESSAGE_MAP()
    afx_msg void OnRun(WPARAM wParam, LPARAM lParam);
    afx_msg void OnQuit(WPARAM wParam, LPARAM lParam);
};

// ManagerThread.cpp

IMPLEMENT_DYNCREATE(ManagerThread, CWinThread)

ManagerThread::~ManagerThread() {
    while(!m_WorkerThreads.empty()) {
        std::vector<WorkerThread*>::iterator it = m_WorkerThreads.begin();
        (*it)->PostThreadMessage(WM_QUIT, 0, 0);
        m_WorkerThreads.erase(it);
    }
}

BOOL CFilterManagerThread::InitInstance()
{
    return CWinThread::InitInstance();
}


int CFilterManagerThread::ExitInstance()
{
    return CWinThread::ExitInstance();
}

BEGIN_MESSAGE_MAP(ManagerThread, CWinThread)
    ON_THREAD_MESSAGE(WM_START_RUN, OnStartRun)
    ON_THREAD_MESSAGE(WM_JOB_DONE, OnJobDone)
    ON_THREAD_MESSAGE(WM_QUIT, OnQuit)
END_MESSAGE_MAP()

void ManagerThread::OnJobDone( WPARAM wParam, LPARAM lParam) {
    numOfJobs--;
    if (!numOfJobs) {
        OnQuit(0,0);
    }
}

void ManagerThread::OnStartRun(WPARAM wParam, LPARAM lParam) {
    numOfJobs = (int) wParam;
    for (int i = 0; i < numOfJobs; i++) {
        WorkerThread *newThread = (WorkerThread*)AfxBeginThread(RUNTIME_CLASS(WorkerThread), THREAD_PRIORITY_LOWEST, 0, CREATE_SUSPENDED);
        newThread->SetID(i);
        newThread->SetManager(this);
        m_WorkerThreads.push_back(newThread);
        newThread->ResumeThread();
        Sleep(1000); //sleep 1 second before sending message to allow the thread to strat running
        newThread->PostThreadMessage(WM_RUN, 0, 0);
    }
}

void ManagerThread::OnQuit(WPARAM wParam, LPARAM lParam) {
    AfxEndThread(0);
}

// WorkerThread.cpp

IMPLEMENT_DYNCREATE(WorkerThread, CWinThread)

BOOL WorkerThread::InitInstance() {
    // TODO:  perform and per-thread initialization here
    CoInitializeEx(NULL, COINIT_APARTMENTTHREADED);
    return TRUE;
}

int WorkerThread::ExitInstance() {
    // TODO:  perform any per-thread cleanup here

    //uninitialize the COM library
    CoUninitialize();
    return CWinThread::ExitInstance();
}

BEGIN_MESSAGE_MAP(WorkerThread, CWinThread)
    ON_THREAD_MESSAGE(WM_RUN, OnRun)
    ON_THREAD_MESSAGE(WM_QUIT, OnQuit)
END_MESSAGE_MAP()

void WorkerThread::OnRun(WPARAM wParam, LPARAM lParam) {
    cout << id <<endl;
    m_Manager->PostThreadMessage(WM_JOB_DONE, id, 0);
}
void WorkerThread::OnQuit(WPARAM wParam, LPARAM lParam) {
    AfxEndThread(0);
}

and in main:

        ManagerThread *manager = (ManagerThread*)AfxBeginThread(RUNTIME_CLASS(ManagerThread), THREAD_PRIORITY_NORMAL, 0, CREATE_SUSPENDED);
        manager->ResumeThread();

        Sleep(1000); //sleep 1 second before sending message to allow the thread to start running

        manager->PostThreadMessage(WM_START_RUN, 10, 0);
        while(true){}

This is a rough sample. Of course in my original codeI use better mechanisms than Sleep and while(true) to ensure synchronization and to avoid the program ending before the manager thread has ended. But it reproduces the problem I'm having so I didn't see the point in adding any more complexity.

1
Why didn't you override 'InitInstance' in your 'ManagerThread' class.MarsRover
@MarsRover: You were right, see EDIT. It made me work out the problem with WorkerThread as well, will post solution sooneladidan

1 Answers

2
votes

Figured out what the problem was. The problem was the call to CoInitializeEx in WorkerThread::initInstance. Apparently the call blocked the initialization of the thread for a LONG time, even more than my Sleep(1000) in the sample code. And so I was posting the messages before the message queue was created. So following the instructions from MSDN:

The thread to which the message is posted must have created a message queue, or else the call to PostThreadMessage fails. Use the following method to handle this situation.

Create an event object, then create the thread.

Use the WaitForSingleObject function to wait for the event to be set to the signaled state before calling PostThreadMessage.

In the thread to which the message will be posted, call PeekMessage as shown here to force the system to create the message queue.

PeekMessage(&msg, NULL, WM_USER, WM_USER, PM_NOREMOVE)

Set the event, to indicate that the thread is ready to receive posted messages.

and from this previous question, I created a member CEvent for each thread class and changed the InitInstance to:

BOOL CFilterWorkerThread::InitInstance()
{
    BOOL worked=CWinThread::InitInstance();
    CoInitializeEx(NULL, COINIT_APARTMENTTHREADED);
    MSG msg;
    PeekMessage(&msg, NULL, WM_USER, WM_USER, PM_NOREMOVE);
    m_ControllerThreadReady.SetEvent();
    return TRUE;
}

In order to force the message queue to be initialized before I set the even to true. Then in ManagerThread I call WaitForSingleObject before posting any messages to WorkerThread.