[Bug]: using ThreadSafeListener still triggers assert when calling from 2 threads

[benkuper]

Detailed steps on how to reproduce the bug

using ThreadSafeListener should prevent from triggering an assert when calling from multiple threads (as per indication in the assert comments), but it still trigger it.

EDIT : Reading the ListenerList function where it happens, the ScopedTryLock on line 233 of juce_ListenerList.h is intriguing to me. Shouldn't there be a function that blocks and ensure the lock is gained (so ScopedLock) when we're using CriticalSection ? Right now, I don't really see what's the actual function of ThreadSafe except telling me that it may not be safe.

Minimal test case :

• 3 Classes : TestClass, TestThread and TestListener
• TestClass extends TestListener, contains 2 instances of TestThread
• TestClass has a ThreadSafeListenerList<TestListener>, and added itself to this list.
• the 2 TestThread instances are continuously calling the testClass "notify()" function
• the notify() function in TestClass calls the listeners TestListener::notified() function
• This triggers the assert in juce_ListenerList line 232 warning about multiple threads calling at the same time and advising to use LightListener or ThreadSafeListener

This is a visual representation of the above description :

I believe this should not trigger the assert, and in scenarios where calls are done repeatedly, this makes impossible to debug as the assert will stop the program every time.

here is the full code :

#include <JuceHeader.h>
#include "MainComponent.h"

class ScriptTestApplication;

class ThreadTest : public juce::Thread
{

public:
	ScriptTestApplication* test;

	ThreadTest(ScriptTestApplication * test) : juce::Thread("ThreadTest"), test(test)
	{
		startThread();
	}

	~ThreadTest()
	{
		signalThreadShouldExit();
		waitForThreadToExit(-1);
	}

	void run() override;
};

class TestListener
{
public:
	virtual ~TestListener() {}
	virtual void notified() = 0;
};

class ScriptTestApplication : public juce::JUCEApplication, public TestListener
{
public:
	ScriptTestApplication() :
		threadTest1(this),
		threadTest2(this)
	{}

	const juce::String getApplicationName() override { return ProjectInfo::projectName; }
	const juce::String getApplicationVersion() override { return ProjectInfo::versionString; }
	bool moreThanOneInstanceAllowed() override { return true; }


	ThreadTest threadTest1;
	ThreadTest threadTest2;
	juce::ThreadSafeListenerList<TestListener> listeners;

	void initialise(const juce::String& commandLine) override
	{
		listeners.add(this);
	}

	void notify()
	{
		DBG("Notify : " << (int)juce::Thread::getCurrentThreadId());
		listeners.call(&TestListener::notified);
	}

	void notified()
	{
		DBG(" > Notified : " << (int)juce::Thread::getCurrentThreadId());
	}

	void shutdown() override
	{
	}

	//==============================================================================
	void systemRequestedQuit() override
	{
		quit();
	}

	void anotherInstanceStarted(const juce::String& commandLine) override
	{

	}

};
START_JUCE_APPLICATION(ScriptTestApplication)


void ThreadTest::run()
{
	while (!threadShouldExit())
	{
		test->notify();
		wait(2);
	}
}

Operating systems

Windows

What versions of the operating systems?

11

Architectures

64-bit

Stacktrace

No response

Plug-in formats (if applicable)

No response

Plug-in host applications (DAWs) (if applicable)

No response

Testing on the develop branch

The bug is present on the develop branch

Code of Conduct

• [X] I agree to follow the Code of Conduct

[benkuper]

I made a PR that proposes a ThreadSafeBlockListenerList, and that uses ScopedLock instead of ScopedTryLock After testing in both my minimal setup and my full-blown software with a lot of cross calls between listeners and threads, this seems very stable.

If you're interested in the context from which I find and publish all the issues, this is the software I develop and test Juce 8 against : https://benjamin.kuperberg.fr/chataigne