Unsafe uses of callAsync

void SomeComponent::notifyStatusChanged() // called from any thread
{
	MessageManager::getInstance()->callAsync([]()
	{
		listeners.call(&Listener::statusChanged);
	});
}

I’m thinking this is an unsafe race condition?

There might be something in the message manager queue which deletes the SomeComponent before the listeners.call is executed?

A problem which is avoided using AsyncUpdater etc.

void SomeComponent::notifyStatusChanged() // called from any thread
{
    Component::SafePointer<SomeComponent> sp (this);
    MessageManager::getInstance()->callAsync ([sp]()
    {
        if (sp != nullptr)
	    listeners.call(&Listener::statusChanged);
    });
}
1 Like

But generally you have more of a race condition if notifyStatusChanged is called from any thread. You’ll need to guard access to the creation of the SafePointer as they are not thread safe.

In general, yes use an async updater to be on the safe side.

1 Like

Cheers Dave, though it looked iffy, but didn’t get enough sleep last night.

class MultiAsyncUpdater
	:
AsyncUpdater
{
public:
	MultiAsyncUpdater()
	{}

	void callOnMessageThread(std::function<void()> callback)
	{
		ScopedLock l(lock);
		queue.push_back(callback);
		triggerAsyncUpdate();
	}

private:
	void handleAsyncUpdate() override
	{
		ScopedLock l(lock);

		for (auto & func : queue)
			func();

		queue.clear();
	}

	CriticalSection lock;
	std::vector<std::function<void()>> queue;
};

I’m thinking this is the general purpose solution… obviously lots of other considerations if performance is critical and it’s gong to be called really frequently…but i think it’ll be good.

1 Like

Yes, that should work as long as you don’t delete the MultiAsyncUpdater concurrently with calling callOnMessageThread.
Also, for a small speed increase you probably want to move callback into the vector:
queue.push_back (std::move (callback));

Good point. Though you’ve forced me to open up a whole can of worms here now instead of getting the job finished…

I’m going to talk about this next week…
In general though I’ve seen a lot of bugs arise from passing std::function by reference. If you’re going to make a copy anyway you might as well make the copy when passing the argument then move the parameter. Then, it’s up to the caller to move the argument if they don’t need it anymore. I’ve not looked in to it fully but I’m also hoping that one day compilers will be able to spot xvalues and automatically move them into arguments if they are values not references.

1 Like

Bloody hell … that MultiAsyncUpdater class just crashed! :slight_smile: Thought that was looking safe… maybe a reentrancy problem where a callback then called the callOnMessageThread function…

Hi Dave,
this is an old topic, but I stumbled into a similar situation. And I want to use the approach with the “SafePointer” inside a lambda. Thanks for suggesting.
You wrote, that one has to safeguard the creation of the SafePointer. Can you elaborate on that? When/why is that necessary? In case the function is not re-entrant?

Um, bit difficult to remember the context from 3 years ago…
I think in general though, you can only create SafePointers on the message thread as that’s the only time you know a Component won’t be being deleted from underneath you…

Thanks for explaining… I see…
When I call MessageManager::getInstance()->callAsync([](), I am of course not on the message thread. So, I must create the SafePointer at some earlier point in time, I assume, when on the message thread?

Wondering if that’s really safe? Even if I create the SafePointer at an earlier time, the class could just be about to get destructed, while pushing the Async lambda on the message thread. Then I might copy an already destructed SafePointer to the lambda. Hmmm…

You could hold a message manager lock when you create the safe pointer.

Personally I think callAsync should have some safety warnings attached in capital letters in the documentation. Pretty much 90% of the times I’ve seen it used it’s been terrifying :slight_smile:

Also, is callAsync safe here? https://github.com/WeAreROLI/JUCE/blob/master/extras/Projucer/Source/Application/Windows/jucer_PIPCreatorWindowComponent.h

You make me a little worried here. So would you discourage using AsyncCallbacks in plugins? Because you never know, when the user decides to kill the UI?

I have to admit, that I am using MessageManager::getInstance()->callAsync quite a lot :slight_smile:

Well, as long as you can guarantee that the pointers and objects you use in your callAsync lambda are still valid when it’s executed everything will be fine :slight_smile:

Right :wink:
How do I do that? How can I safely know, if the UI has been killed or not in the meantime, when executing the async UI callback?

AsyncUpdater is much easier to get right without thinking!

You would normally create the SafePointer on the message thread, then pass that to your thread to be run, then forward it on to your async lambda where you can check it for validity before using it.

And Jim says though, maybe what you’re looking for is really AsyncUpdater?

1 Like

I am still a little new to this :slight_smile:
How should I use AsyncUpdater to make it safer than MessageManager::getInstance()->callAsync ?
Both result in async execution on the message thread. And as far as I can see, both rely internally on MessageManager::MessageBase.
Doesn’t that make them equivalent?

Well it really depends on what you want to do. They’re different tools for different jobs.
AsyncUpdater coalesces updates and is a simple signal notification, there’s no payload. If all you want to do is find out that another thread has finished its job, then this is the simplest tool.

MessageManager::callAsync is for dispatching specific messages (via std::function) to be called back from the message thread. If you call it multiple times, multiple messages will be dispatched and therefore delivered. These are useful for one-hit notifications.

However, as they’re asynchronous and you need to make sure anything the callback references is still valid when the callback happens. This often means capturing SafePointers (which internally are weak-pointers).
If you’re calling MessageManager::callAsync from a background thread, you also need to pay attention to data races as you would do in any other multi-threaded code. This usually means making sure objects can’t get deleted out from underneath you.