MessageManager::callAsynch() calling deleted object (and crashing)

I have a couple of Components that derive from AudioProcessorValueTreeState::Listener, and respond to parameterChanged() in order to update related text views when a parameter value changes. But I have noticed that in some instances (Studio One 4, for instance), my editor is created, and then almost immediately destroyed, and then created again. But before it is destroyed that first time, my parameterChanged() function has been called, which calls this code:

MessageManager::callAsync([this]()
{
	xAxisValue->setText( xString, dontSendNotification );
}

I do this to make sure that the text is only changed on the message thread, never on the process thread. But after this code is called, the editor (and thus this component) gets destroyed, and a new one created and shown. The problem is that sometimes the setText() function then gets called (asynchronously), and everything crashes.

I created the component that contains this code in Projucer, so it’s being created and destroyed correctly. So… how can I fix this? The “this” pointer at the time that setText() call happens is still pointing to the (now destroyed) component, so checking for NULL isn’t a valid option. Is there some way to prevent any pending asynchronous calls from being made to my component?

Take a look at Component::SafePointer

I don’t understand how I would use that in the code above, which uses a lambda.

Common practice is to capture a WeakReference to this and check if this reference is still valid:

WeakReference<MyEditor> editor (this);
MessageManager::callAsync([editor]()
{
    if (editor)
        editor->setXAxisValueText (xString);
}

You will need either a public function, or use the editor WeakReference just to check, and still capture the this pointer as well (haven’t tried this, and TBH it looks ugly):

WeakReference<MyEditor> editor (this);
MessageManager::callAsync([this, editor]()
{
    if (editor)
        xAxisValue->setText( xString, dontSendNotification );
}

(SafePointer is similar to WeakReference, but is specific to Component)

EDIT: this doesn’t solve the problem, if the destruction happens from a thread other than the message thread

Um, ok… that last edit worries me. I’m not sure what relation the message thread has with the thread on which a plug-in’s editor is created and destroyed. This (using SafePointer) seems to be working in Studio One 4, but I haven’t tested Logic, ProTools, Cubase, or any others, so I’m unsure if this is the solution or not. I see MessageManager::callAsynch() used in the JUCE SDK liberally, without this weak reference stuff. What’s the correct way to update the text of a label safely from the parameterChanged() callback?

I think you are fine, since the construction and destruction of the editor happens on the message thread. And since your lambda is also happening on the message thread, you are safe here.
Just wanted to mention, that there is still a chance for threading issues, if at the moment you checked the WeakReference (SafePointer) to be valid, a different thread is deleting the object.

But like I said, most actions on Components have an assert to happen on the message thread.

1 Like

Hi,

I came across this thread while investigating some code that is crashing (on a system that I inherited). I think the problem is the same basic issue as mentioned above.

There is a thread that is processing incoming network data, creating a new PropertyData object (allocating with new), then enqueuing that object to be executed on the main thread using MessageManager::callAsync(). When the callback is executed on the main thread, it deletes the PropertyData object. All of this code is compiled and executed in the same application (not DLLs or other libraries involved).

I see the callback getting executed twice which on the second time, it crashes due to an already deleted object.

From reading the various forum posts on this topic, it’s not exactly clear if I should be wrapping the created object with a SafePointer or similar object or if my root problem is something else.

The variable changeCallback is owned by NetworkThread and is installed on the construction of the NetworkThread. It is std::function< void (PropertyData*)> changeCallback;

void NetworkThread::run()
{
…
PropertyData* receiveProperty = new PropertyData();

// perform callback on main thread
auto func = [this, receiveProperty]()
{
    changeCallback(receiveProperty);
};

MessageManager::callAsync(func);

…
}

Yes it should be wrapped in a SafePointer or weak_ptr.