MessageManager::broadcastMessage lock condition


#1

A FYI for anyone using broadcastMessage to send inter-JUCE-application messages: be careful when broadcasting messages back and forth between JUCE apps (windows only). If you are receiving a broadcast message in your app, do NOT send a broadcast message back from within that receive routine or from any other thread until that receive routine returns. JUCE uses SendMessageTimeout with SMTO_BLOCK and you will get stuck in that infamous Windows SendMessage lock condition if you do this. With two apps, that means up to 16 seconds (8000ms timeout) of locking.

For those of you (like me) who must send broadcast messages back and forth (one of mine runs in a timer callback and it would be a really bad thing to spin lock within it) but want to avoid the lock condition, here is a quick and dirty fix:

In juce_MessageManager.h, replace:[code]
//==============================================================================
/** Sends a message to all other JUCE applications that are running.

    @param messageText      the string that will be passed to the actionListenerCallback()
                            method of the broadcast listeners in the other app.
    @see registerBroadcastListener, ActionListener
*/
static void broadcastMessage (const String& messageText);[/code]

with:[code]
//==============================================================================
/** Sends a message to all other JUCE applications that are running.

    @param messageText      the string that will be passed to the actionListenerCallback()
                            method of the broadcast listeners in the other app.
    @param wait             true = waits until message has been processed by the receiving app
                            false = delivers message and immediately returns
    @see registerBroadcastListener, ActionListener
*/
static void broadcastMessage (const String& messageText, bool wait=true);[/code]

and in juce_win32_Messaging.cpp, replace:[code]
//==============================================================================
static BOOL CALLBACK BroadcastEnumWindowProc (HWND hwnd, LPARAM lParam)
{
TCHAR windowName [64]; // no need to read longer strings than this
GetWindowText (hwnd, windowName, 64);

if (hwnd != juce_messageWindowHandle
     && String (windowName) == String (messageWindowName))
{
    DWORD result;
    SendMessageTimeout (hwnd, broadcastId, 0, lParam,
                        SMTO_BLOCK | SMTO_ABORTIFHUNG,
                        8000,
                        &result);
}

return TRUE;

}

void MessageManager::broadcastMessage (const String& value)
{
ATOM atom = GlobalAddAtom (value);

if (atom != 0)
{
    EnumWindows (&BroadcastEnumWindowProc, (long) atom);
    GlobalDeleteAtom (atom);
}

}

//==============================================================================[/code]
with:[code]
//==============================================================================

static HWND* handleArray = NULL;
static int* handleIndex = NULL;

static BOOL CALLBACK BroadcastEnumWindowProc (HWND hwnd, LPARAM lParam)
{
TCHAR windowName [64]; // no need to read longer strings than this
GetWindowText (hwnd, windowName, 64);

if (hwnd != juce_messageWindowHandle
    && String (windowName) == String (messageWindowName))
{
    handleArray[(*handleIndex)++] = hwnd;
    if(*handleIndex == HANDLEMAX)
        return false;
}
return TRUE;

}

void MessageManager::broadcastMessage (const String& value, bool wait)
{
// Global atom strings cannot be longer than 255 characters
// Find some way of breaking up your strings into smaller pieces
jassert (value.length() < 255);

ATOM atom = GlobalAddAtom (value);

if (atom != 0)
{
    // Initialize our array (trying to make it thread safe here)
    int totalHandles = 0;
    handleIndex = &totalHandles;
    HWND windows[HANDLEMAX];
    handleArray = windows;

    // Get handles to all our top level windows
    EnumWindows (&BroadcastEnumWindowProc, (long) atom);

    // Now broadcast our message
    for(int i = 0; i < totalHandles; i++)
    {
        DWORD result;
        SendMessageTimeout (windows[i], broadcastId, 0, (long)atom,
            (wait?SMTO_BLOCK:SMTO_NORMAL) | SMTO_ABORTIFHUNG,
            8000,
            &result);
    }
    GlobalDeleteAtom (atom);
}

}

//==============================================================================
[/code]

You can then set “wait” to false if your broadcast message is not a critical “must be received” event and you can tolerate some dropped messages.

A much better fix would be to receive the message, copy the data into a global, post a flag message, then return. The flag message would then deliver the broadcast data to the receiver. Unfortunately, I don’t have the time to do this myself, so the above fix will have to do for now.

As an aside, Jules, what’s up with using global atoms?? That was last fashionable around, oh, 1993 or so. A much better choice would be WM_COPYDATA – no 255 byte size limitation, no case insensitivty, plus we could pass around any sort of data, not just strings.

My $.02

  • kbj

#2

Good idea - I could add some code to re-post it as a message, like you suggest.

I think I probably wrote it around 1993…[/code]


#3

Ok, that did need a bit of a tidy-up. Try this non-blocking version:

static LRESULT CALLBACK juce_MessageWndProc (HWND h,
                                             UINT message,
                                             WPARAM wParam,
                                             LPARAM lParam) throw()
{
    JUCE_TRY
    {
        if (h == juce_messageWindowHandle)
        {
            if (message == specialCallbackId)
            {
                MessageCallbackFunction* const func = (MessageCallbackFunction*) wParam;
                return (LRESULT) (*func) ((void*) lParam);
            }
            else if (message == specialId)
            {
                // these are trapped early in the dispatch call, but must also be checked
                // here in case there are windows modal dialog boxes doing their own
                // dispatch loop and not calling our version

                --numMessagesInQueue;
                MessageManager::getInstance()->deliverMessage ((void*) lParam);
                return 0;
            }
            else if (message == broadcastId)
            {
                String* const messageString = (String*) lParam;
                MessageManager::getInstance()->deliverBroadcastMessage (*messageString);
                delete messageString;
                return 0;
            }
            else if (message == WM_COPYDATA && ((const COPYDATASTRUCT*) lParam)->dwData == broadcastId)
            {
                const String messageString ((const juce_wchar*) ((const COPYDATASTRUCT*) lParam)->lpData, 
                                            ((const COPYDATASTRUCT*) lParam)->cbData / sizeof (juce_wchar));

                PostMessage (juce_messageWindowHandle, broadcastId, 0, (LPARAM) new String (messageString));
                return 0;
            }
        }

        return DefWindowProc (h, message, wParam, lParam);
    }
    JUCE_CATCH_EXCEPTION

    return 0;
}

and

//==============================================================================
static BOOL CALLBACK BroadcastEnumWindowProc (HWND hwnd, LPARAM lParam)
{
    if (hwnd != juce_messageWindowHandle)
    {
        TCHAR windowName [64]; // no need to read longer strings than this
        GetWindowText (hwnd, windowName, 64);
        windowName [63] = 0;

        if (String (windowName) == String (messageWindowName))
        {
            DWORD result;
            SendMessageTimeout (hwnd, WM_COPYDATA, (WPARAM) (HWND) juce_messageWindowHandle, lParam,
                                SMTO_BLOCK | SMTO_ABORTIFHUNG,
                                8000,
                                &result);
        }
    }

    return TRUE;
}

void MessageManager::broadcastMessage (const String& value)
{
    const String localCopy (value);

    COPYDATASTRUCT data;
    data.dwData = broadcastId;
    data.cbData = (localCopy.length() + 1) * sizeof (juce_wchar);
    data.lpData = (void*) (const juce_wchar*) localCopy;

    EnumWindows (&BroadcastEnumWindowProc, (LPARAM) &data);
}

#4

I’ve always used broadcast messages like that just to send a quick data burst. For large amounts of data that needs to be transfered in windows, if I’m lazy I will just open up a socket, else I will allocate a section of global memory and wrap it up in syncronization primitives and have them use it to pass data.


#5

Hey, you’re fast. I suppose it helps that you know the ins and out of the entire library. One thing though is that it’s really bad form to be doing anything other than collecting data inside any EnumXXXXProc routine. Any number of subtle bugs could pop up, even if everything you’re doing is Windows “legal.”

[quote=“OvermindDL1”]For large amounts of data that needs to be transfered in windows[/quote]For transferring large data chunks across process boundaries, memory mapped files are your friend.

  • kbj

#6

[quote=“killerbobjr”][quote=“OvermindDL1”]For large amounts of data that needs to be transfered in windows[/quote]For transferring large data chunks across process boundaries, memory mapped files are your friend.

  • kbj[/quote]

Quite so. Actually, boost just added a library for shared memory and it should be in the next released revision (probobly in cvs now), can’t wait to see it, perhaps I won’t be so lazy to use shared memory if there is a simple interface. :stuck_out_tongue:


#7

That’s interesting - what sort of things are you suggesting could go wrong?


#8

In your code, probably nothing since it does very little. But I’ve run into stuff where things like trying to do TerminateProcess didn’t work quite right unless I had exited the EnumWindowsProc and then iterated through the saved window list. Admittedly, this may have been on the Win9x platform and not NT/2K/XP (it’s been a while and I vaguely remember it had worked on one platform but not the other), so it may be okay for XP. In general though, I like to do things as safely as possible in any sort of callback/interrupt routine.

  • kbj

#9

Ok, I’ll tweak it. Better safe than sorry.


#10