Questions about MessageManagerLock

General JUCE discussion
1

I have some functions that are almost always called on the MessageThread, that eventually cause Component repainting.

In some rare cases (I’ve recently discovered), they can also be called from a HighResolutionTimer thread, where I’m hitting the assert:

    // if component methods are being called from threads other than the message
    // thread, you'll need to use a MessageManagerLock object to make sure it's thread-safe.
    JUCE_ASSERT_MESSAGE_MANAGER_IS_LOCKED

In this case, is it better to just put in a MessageManagerLock (always), like:

        if (numbox)
        {
            MessageManagerLock mml;
            
            setNumBoxRange(numbox, (double) minValue, (double) maxValue);
        }

Or should you test for being on the MessageThread first, something like:

        if (numbox)
        {
             if (! MessageManager::getInstance()->isThisTheMessageThread())
            {
                MessageManagerLock mml;
                
                setNumBoxRange(numbox, (double) minValue, (double) maxValue);
            }
            else
            {
                setNumBoxRange(numbox, (double) minValue, (double) maxValue);
            }

Put another way, does a MessageManagerLock do anything if you are already on the MessageThread?

And how safe, in general, is it to use this? I’ve never run into this before so I’m unsure the best way to handle it.

Does a MessageManagerLock interrupt other things that may be happening on the MessageThread?

2

No it sees the lock is already acquired and continues.

I’m not a huge fan of using the MML I would normally just trigger the work to happen at a slightly later time on the message thread. In this case you’re on a very high priority thread so any lock that contends with another lower priority thread (such as the message thread) is going to cause priority inversion!

We also know there are some issues with the MML that are non-trivial to fix and make sure we don’t break other cases so I strongly recommend avoiding if at all possible.

Inheriting from AsyncUpdater is probably a good alternative to try. If you’re not on the message thread just call triggerAsyncUpdate() then do the work in handleAsyncUpdate(). If the AsyncUpdater object is going to be destroyed on a thread other than the MessageThread make sure to call cancelPendingUpdate() in it’s destructor.

It acquires the lock meaning while you have that lock the message thread is paused waiting to obtain the lock back again before it can do any more work.

2 Likes
3

Thanks @anthony-nicholls, I will avoid using it and come up with a more robust solution. You’ve given me some ideas. :slight_smile:

4

There’s also MessageManager::callAsync, which I find very convenient.

One has to be careful, though, that whatever the callback references is still valid when the callback happens.

Also, the calls don’t coalesce into a single callback as with the AsyncUpdater.

1 Like
5

Thank you for reminding me about this. I’ve used it before, but forgot. :wink:

This is the solution I arrived at (for an override of Slider::setValue() that may get called on a thread other than the Message Thread):

void NumberBox::setValue (double newValue, NotificationType notification)
{
    // declare a lambda that will callAsync if not on the message thread

    auto doNumBoxValue = [&] (double _newValue, NotificationType _notification)
    {
        Slider::setValue (_newValue, _notification);
    };
    
    if (! MessageManager::getInstance()->isThisTheMessageThread())
    {
        auto safeThis = SafePointer<NumberBox> (this);
        MessageManager::callAsync ([safeThis, doNumBoxValue, newValue, notification]() mutable
			{
                  if (safeThis != nullptr)
                      doNumBoxValue(newValue, notification);
               });
    }
    else
        doNumBoxValue(newValue, notification);
}

Any comments, let me know. Thanks.

6

Hi! Perhaps you could simplify the code by not checking whether it’s the message thread at all? I imagine that, depending on what you are doing, it could make no practical difference if you just always call MessageManager::callAsync. I’d try it out and see if there’s a difference.
(And If it does make a difference I’d love to know!).

7

Well, the difference is that if it’s on the message thread, it executes immediately, and in the debugger the stack trace shows thecalling chain as it is supposed to. If it’s NOT on the message thread, it ends up executing an indeterminate period of time later (small, but there nonetheless) as a result of the MessageQueue::runLoopCallback, so you can’t tell where it came from.

Since in my use case, this happens very rarely, it seems better to test for it so that I can have it operate “normally” most of the time.

1 Like
8

I would definitely delete the & from the lambda capture, because this could lead to undefined behavior if you asynchronously call a value which has been captured by reference. Anything that goes in the lambda capture must be copied by value, if you’re calling it asynchronously.

If it were me, I would take a different approach to what you’ve done here. Instead of trying to give this one function two different behaviors for different threads, I would enforce that this function has to be called on the message thread, and then fix the upstream problem of why it is being called by another thread.

In other words, I would do:

void setValue()
{
    JUCE_ASSERT_MESSAGE_THREAD
    ....
}

Then when you hit an assertion from the other thread, look up the stack and identify where the right place is to make your code asynchronous. The asynchronous call you’ve added here is probably not the right place for it, since your function now has two different paths that work in different ways, and one of them doesn’t actually guarantee that the value is being set. For instance, if you do something like this:

numBox.setValue(5);
auto v = numBox.getValue();

… v is going to be a different value depending on which thread you’re using, and that is something that might come back to bite you later on.

Probably there is a more sensible place to make the asynchronous call higher up the chain. As others have pointed out, using AsyncUpdater might make more sense than callAsync, because if you call the latter one from a HighPrecisionTimer, you’re going to be flooding your message thread with many incremental changes, where just one would have been enough.

4 Likes
9

Thanks, you’re right - this was a hastily chosen solution, for a number of reasons you’ve covered. I’ll come up with something better.