views:

45

answers:

3

I have an Observer class and a Subscriber class.
For testing purposes, the observer creates a thread that generates fake messages and calls CServerCommandObserver::NotifySubscribers(), which looks like this:

void CServerCommandObserver::NotifySubscribers(const Command cmd, void const * const pData)
{
    // Executed in worker thread //

    for (Subscribers::const_iterator it = m_subscribers.begin(); it != m_subscribers.end(); ++it)
    {
        const CServerCommandSubscriber * pSubscriber = *it;

        const HWND hWnd = pSubscriber->GetWindowHandle();
        if (!IsWindow(hWnd)) { ASSERT(FALSE); continue; }

        SendMessage(hWnd, WM_SERVERCOMMAND, cmd, reinterpret_cast<LPARAM>(pData));
    }
}

The subscriber is a CDialog derived class, that also inherits from CServerCommandSubscriber.

In the derived class, I added a message map entry, that routes server commands to the subscriber class handler.

// Derived dialog class .cpp
ON_REGISTERED_MESSAGE(CServerCommandObserver::WM_SERVERCOMMAND, HandleServerCommand)

// Subscriber base class .cpp
void CServerCommandSubscriber::HandleServerCommand(const WPARAM wParam, const LPARAM lParam)
{
    const Command cmd = static_cast<Command>(wParam);

    switch (cmd)
    {
    case something:
        OnSomething(SomethingData(lParam)); // Virtual method call
        break;
    case // ...
    };
}

The problem is, that I see strange crashes in the HandleServerCommand() method:

It looks something like this:

Debug Error!

Program: c:\myprogram.exe
Module:
File: i386\chkesp.c
Line: 42

The value of ESP was not properly saved across a function call. This is usually the result of calling a function declared with one calling convention with a function pointer declared with a different calling convention.

I checked the function pointer that AfxBeginThread() wants to have:

typedef UINT (AFX_CDECL *AFX_THREADPROC)(LPVOID); // AFXWIN.H

static UINT AFX_CDECL MessageGeneratorThread(LPVOID pParam); // My thread function

To me, this looks compatible, isn't it?

I don't know, what else I have to look for. Any ideas?

I made another strange observation, that might be related: In the NotifySubscribersmethod, I call IsWindow() to check if the window to which the handle points, exists. Apparently it does. But calling CWnd::FromHandlePermanent() returns a NULL pointer.

A: 

To me, this looks compatible, isn't it?

Syntactically it looks that way.

I don't know, what else I have to look for. Any ideas?

Yes: I've had the same problem when compiling a plugin library with debug settings and used in a Release-compiled application.

Basically, the problem looks like a stack corruption.

Since you're running NotifySubscribers in a separate thread, consider using PostMessage (or PostThreadMessage) instead of SendMessage.

This may not be the actual cause of the crash, but the change should be made anyway (as you're switching threading contexts by using SendMessage with no guarding of the data whatsoever.

utnapistim
I also tried `PostMessage()` but it had the same effect. The reason for using `SendMessage()` was that the sender is blocked until the receiver has done it's work. I assumed this would save me the effort of having to store the data, that lParam points to, in some special place and having to figure out when the memory can be freed.
mxp
+3  A: 

From afxmsg_.h:

// for Registered Windows messages
#define ON_REGISTERED_MESSAGE(nMessageVariable, memberFxn) \
    { 0xC000, 0, 0, 0, (UINT_PTR)(UINT*)(&nMessageVariable), \
        /*implied 'AfxSig_lwl'*/ \
        (AFX_PMSG)(AFX_PMSGW) \
        (static_cast< LRESULT (AFX_MSG_CALL CWnd::*)(WPARAM, LPARAM) > \
        (memberFxn)) },

So the signature is LRESULT ClassName::FunctionName(WPARAM, LPARAM), while yours is void ClassName::FunctionName(const WPARAM, const LPARAM). This should not compile, at least under VS2008 it doesn't.

What is your HandleServerCommand declaration in the CServerCommandSubscriber class (in the header file)?

AOI Karasu
You are right, it was wrong (and shouldn't have compiled; but I'm bound to an ancient compiler). Correcting it to `LRESULT HandleServerCommand(WPARAM wParam, LPARAM lParam); ` did not fix the problem, however.
mxp
+1  A: 

I eventually decided to do it without window messages and am now posting my workaround here. Maybe it will help someone else.

Instead of letting the observer post window messages to its subscribers, I let the observer put data into synchronized subscriber buffers. The dialog class subscriber uses a timer to periodically check its buffers and call the apropriate handlers if those aren't empty.
There are some disadvantages:

  • It's more coding effort because for each data type, a buffer member needs to be added to the subscriber.
  • It's also more space consuming, as the data exists for each subscriber and not just once during the SendMessage() call.
  • One also has to do the synchronization manually instead of relying on the observer thread being suspended while the messages are handled.

A - IMO - huge advantage is that it has better type-safety. One doesn't have to cast some lParam values into pointers depending on wParam's value. Because of this, I think this workaround is very acceptable if not even superior to my original approach.

mxp