ActionBroadcaster::sendActionMessage silently fails

If the message queue on windows is full, then ActionBroadcaster::sendActionMessage silently fails, and then message is lost. Could we at minimum get an assert, and a return value from ActionBroadcaster::sendActionMessage or best case somehow delay and retry later.

The queue limit is 10,000 which seems like a lot, but one message gets generated per listener, so if you have a bunch of listeners, it gets pretty easy to flood the queue. Wouldn’t it be better to generate one ActionMessage and it iterates the list of actionListeners and calls the callback? The ActionMessage already has to check if the pointer is still in the list.

What about something like this:

bool ActionBroadcaster::sendActionMessage (const String& message) const
    const ScopedLock sl (actionListenerLock);

    bool allOk = true;

    for (int i = actionListeners.size(); --i >= 0;)
        bool ok = (new ActionMessage (this, message, actionListeners.getUnchecked(i)))->post();
        jassert (ok);
        if (! ok)
            allOk = false;

    return allOk;
1 Like

Ok, I just hit this again in a different program. We really need asserts on this.

If you use triggerAsyncUpdate too much, it will start to silently fail and then any future ActionBroadcaster::sendActionMessage fail as well.

I’m cleaning up a lot of unneeded triggerAsyncUpdate in my app. Can I get the following change to viewport:

diff --git a/modules/juce_gui_basics/layout/juce_Viewport.cpp b/modules/juce_gui_basics/layout/juce_Viewport.cpp
index d10006cb8..70f9fb8be 100644
--- a/modules/juce_gui_basics/layout/juce_Viewport.cpp
+++ b/modules/juce_gui_basics/layout/juce_Viewport.cpp
@@ -405,8 +405,8 @@ void Viewport::updateVisibleArea()
     auto& vbar = getVerticalScrollBar();

     hbar.setBounds (contentArea.getX(), hScrollbarBottom ? contentArea.getHeight() : 0, contentArea.getWidth(), scrollbarWidth);
-    hbar.setRangeLimits (0.0, contentBounds.getWidth());
-    hbar.setCurrentRange (visibleOrigin.x, contentArea.getWidth());
+    hbar.setRangeLimits (0.0, contentBounds.getWidth(), dontSendNotification);
+    hbar.setCurrentRange (visibleOrigin.x, contentArea.getWidth(), dontSendNotification);
     hbar.setSingleStepSize (singleStepX);

@@ -414,8 +414,8 @@ void Viewport::updateVisibleArea()
         visibleOrigin.setX (0);

     vbar.setBounds (vScrollbarRight ? contentArea.getWidth() : 0, contentArea.getY(), scrollbarWidth, contentArea.getHeight());
-    vbar.setRangeLimits (0.0, contentBounds.getHeight());
-    vbar.setCurrentRange (visibleOrigin.y, contentArea.getHeight());
+    vbar.setRangeLimits (0.0, contentBounds.getHeight(), dontSendNotification);
+    vbar.setCurrentRange (visibleOrigin.y, contentArea.getHeight(), dontSendNotification);
     vbar.setSingleStepSize (singleStepY);

I don’t see the point of sending the async updates and then immediately cancelling them. This saves me about 1000 messages posted MessageManager.

Ok, here is a very simple but contrived example on how to break the message manager.

Create 2500 sliders. You don’t even need to add them as child components. Now the message manager is not responding and async updates, actions messages and timers are all getting lost. The undelivered message are leaking memory. Change it to 2400 sliders, everything works fine.

Reproduced on Windows 10.

The design on MacOS seems much better. Every time a message is posted, it’s added to a ReferenceCountedArray<MessageManager::MessageBase, CriticalSection> messages; and then the main queue is woken up. When the queue wakes up, it dispatches the messages in the queue. Seems like Windows should adopt a similar design.

Thanks for posting the example. I’ve added an overflow buffer for messages that are posted after the message queue limit is reached in commit ce9bb8b. This should prevent the messages from just being silently dropped but the ordering might get skewed a bit if multiple threads are thrashing the queue. AFAIK it’s not possible to replicate the macOS message queue behaviour on Windows as there’s no way to signal that the queue should “wake up”, so this is the best we can do really.

1 Like

It’s working now, thanks.