Assertion failure in MessageManager after 7.0.5 -> 7.0.6

General JUCE discussion
1

I have a simple utility for destroying a bunch of Components in a worker thread:

/**
	Disposes components in a background thread. The dispose method takes a juce::OwnedArray of components,
	clears the array and returns. The contained components are disposed later. This is useful when you need to
	get rid of a lot of components, but don't want to keep the message thread waiting and UI irresponsive.
*/
template <typename T> class ComponentDisposer : public juce::Thread {
	public:
	
		ComponentDisposer () : Thread ("Component Disposer") {}
		
		~ComponentDisposer () {
			signalThreadShouldExit ();
			lock.abort ();
			notify ();
			waitForThreadToExit (-1);
		}
	
		void dispose (juce::OwnedArray<T>& components) {
			const juce::MessageManager::Lock::ScopedLockType l (lock);
			while (!components.isEmpty ()) {
				this->components.add (components.removeAndReturn (components.size () - 1));
				notify ();
			}
		}
		
		void run () override {
			while (!threadShouldExit ()) {
				bool isDirty = false;
				{
					const juce::MessageManager::Lock::ScopedTryLockType l (lock);
					if (l.isLocked () && !components.isEmpty ()) {
						components.remove (components.size () - 1);
						isDirty = !components.isEmpty ();
					}
				}
				if (!isDirty) {
					wait (-1);
				}
			}
		}
		
	private:
		juce::OwnedArray<T> components;
		juce::MessageManager::Lock lock;

		JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (ComponentDisposer);
};

After updating from JUCE 7.0.5 to 7.0.6, every time i close the app, i hit the below assertion in juce_ReferenceCountedObjectPtr line 394

    // the -> operator is called on the referenced object
    ReferencedType* operator->() const noexcept
    {
        jassert (referencedObject != nullptr); // null pointer method call!
        return referencedObject;
    }

This occurs while the destructor is in waitForThreadToExit (-1);

Here’s the stack trace

Screenshot 2023-08-21 at 22.50.10
Screenshot 2023-08-21 at 22.50.101794×572 164 KB

It looks as if the MessageManager itself is in invalid state at this point - trying to access an internal object through a null pointer.

How to fix this? Something has changed in JUCE and i would desperately need to know how to make this work again.

Here’s the entire MessageManager method for quick reference:

bool MessageManager::Lock::tryAcquire (bool lockIsMandatory) const noexcept
{
    auto* mm = MessageManager::instance;

    if (mm == nullptr)
    {
        jassertfalse;
        return false;
    }

    if (! lockIsMandatory && [&]
                             {
                                 const std::scoped_lock lock { mutex };
                                 return std::exchange (abortWait, false);
                             }())
    {
        return false;
    }

    if (mm->currentThreadHasLockedMessageManager())
        return true;

    try
    {
        blockingMessage = *new BlockingMessage (this);
    }
    catch (...)
    {
        jassert (! lockIsMandatory);
        return false;
    }

    if (! blockingMessage->post())
    {
        // post of message failed while trying to get the lock
        jassert (! lockIsMandatory);
        blockingMessage = nullptr;
        return false;
    }

    for (;;)
    {
        {
            std::unique_lock lock { mutex };
            condvar.wait (lock, [&] { return std::exchange (abortWait, false); });
        }

        // ------------------------ //
        //        THIS FAILS        //
        // ------------------------ //

        if (blockingMessage->wasAcquired())

        // ------------------------ //

        {
            mm->threadWithLock = Thread::getCurrentThreadId();
            return true;
        }

        if (! lockIsMandatory)
            break;
    }

    // we didn't get the lock

    blockingMessage->stopWaiting();
    blockingMessage = nullptr;
    return false;
}
2

Sidebar because this class makes me think of it again: It would be soooo cool, if JUCE would allow to create and destroy components off-message-thread, as long as they are not shown on any Desktop.

3

I’ve had the impression that Components can be destroyed in a worker thread after they’ve been removed from any parent. At least it has been working just fine before.

4

How much of a difference does this make in practice? If background thread locks the message manager during the destruction, then I’d imagine that the UI will stall for just as long as if the components were deleted directly on the main thread. In both cases, the main thread will have to wait while components are destroyed.

Unless the docs say otherwise, you should assume that all component member functions should only be called from the message thread, or (as a last resort) with a locked message manager.

5

I was thinking that too, but last I tried I for sure went into assertions that I was able to ignore safely, but could compromise my code if I wasn’t careful all the way trough.

For creation (and setup) I for sure run into the assertion in internalRepaint, which could be moved into the big if, when I can assure there won’t be any race conditions for the visible flag. I can’t remember from the top of my head if something similar was happening while destructing.

That is also what I wondered in this example. I could see the performance increasing in a sense that by the try-lock, the cleanup thread might only have very few chances to actually clean something up (also since it is always only one at a time). So the performance gain might be coming at the cost of stuff piling up in RAM (but just guessing while looking at the code)

6

This way the components can be destroyed one by one over time. I suppose the disposing could be spread over time in the message thread just as well, but my approach has been working just fine. Do you think this is the issue? That components simply shouldn’t be destroyed in a worker thread like this?

[EDIT] If i destroy them all at once in the message thread, the app halts for a noticeable time. I’m trying to achieve best performance, by allowing the components to destroy as quickly as possible, but in case a higher priority thread needs to do something, it can wait. RAM is not really an issue.

7

Is there any valid use case for doing this?

8

Which part? Destroying them all at once in the message thread is bad.

9

I’m not 100% sure what makes a locked message manager. Does this lock the message manager?

juce::MessageManager::Lock::ScopedTryLockType l (lock);
10

Ok… first time I hear that… (puzzled)

Is this because you are using enormous amounts of components?
Is this for deposing an editor, thus once? Or does this need to occur on a very regular basis?

11

Yes, there are a lot of them, and they’re disposed while a new bunch of them is being shown to the user. The user experience is awesome vs. horrible.

I know i could make a component pool or something, but i don’t know any straightforward way to reset a component to it’s defaults, and since this has worked…

12

Have you considered allocating them with unique pointers and after use transfer their ownership with std::move () to some structure (say a std::vector or stack) and let a low prio thread reset that structure on a regular basis?

Or is that exactly the bottleneck, in the sense that they remain connected to something in / with the message manager?

Just thinking aloud and trying to get a feeling for the situation.

13

The OwnedArray used here is essentially the same as a vector of unique_ptrs.

The problem is evidently that compnents need to talk to the the message manager when they’re destroyed. I’ll try and implement this with a juce::Timer unless some solution emerges.