diff --git a/modules/juce_audio_devices/native/juce_Midi_windows.cpp b/modules/juce_audio_devices/native/juce_Midi_windows.cpp index 2f88d1d005..de7806c5fc 100644 --- a/modules/juce_audio_devices/native/juce_Midi_windows.cpp +++ b/modules/juce_audio_devices/native/juce_Midi_windows.cpp @@ -1235,7 +1235,7 @@ private: //============================================================================== struct Listener { - virtual ~Listener() {}; + virtual ~Listener() = default; virtual void bleDeviceAdded (const String& containerID) = 0; virtual void bleDeviceDisconnected (const String& containerID) = 0; }; diff --git a/modules/juce_core/containers/juce_ListenerList.h b/modules/juce_core/containers/juce_ListenerList.h index 2b45c35f09..4856f321b0 100644 --- a/modules/juce_core/containers/juce_ListenerList.h +++ b/modules/juce_core/containers/juce_ListenerList.h @@ -37,51 +37,33 @@ namespace juce //============================================================================== /** - Holds a set of objects and can invoke a member function callback on each - object in the set with a single call. + Holds a set of objects and can invoke a member function callback on each object in the set with + a single call. - Use a ListenerList to manage a set of objects which need a callback, and you - can invoke a member function by simply calling call(), callChecked(), or - callExcluding(). + It is safe to add listeners, remove listeners, clear the listeners, and even delete the + ListenerList itself during any listener callback. If you don't need these extra guarantees + consider using a LightweightListenerList instead. - E.g. - @code - class MyListenerType - { - public: - void myCallbackMethod (int foo, bool bar); - }; + If a Listener is added during a callback, it is guaranteed not to be called in the same + iteration. - ListenerList listeners; - listeners.add (someCallbackObjects...); + If a Listener is removed during a callback, it is guaranteed not to be called if it hasn't + already been called. - // This will invoke myCallbackMethod (1234, true) on each of the objects - // in the list... - listeners.call ([] (MyListenerType& l) { l.myCallbackMethod (1234, true); }); - @endcode + If the ListenerList is cleared or deleted during a callback, it is guaranteed that no more + listeners will be called. - It is safe to add listeners, remove listeners, clear the listeners, and - delete the ListenerList itself during any listener callback. + It is NOT safe to make concurrent calls to the listeners without a mutex. If you need this + functionality, either use a LightweightListenerList or a ThreadSafeListenerList. - If a Listener is added during a callback, it is guaranteed not to be called - in the same iteration. - - If a Listener is removed during a callback, it is guaranteed not to be - called if it hasn't already been called. - - If the ListenerList is cleared or deleted during a callback, it is - guaranteed that no more listeners will be called. - - By default a ListenerList is not thread safe. If thread-safety is required, - use the ThreadSafeListenerList type. - - When calling listeners the iteration can be escaped early by using a - "BailOutChecker". A BailOutChecker is a type that has a public member function - with the following signature: + When calling listeners the iteration can be escaped early by using a "BailOutChecker". + A BailOutChecker is a type that has a public member function with the following signature: @code bool shouldBailOut() const @endcode This function will be called before making a call to each listener. For an example see the DummyBailOutChecker. + @see LightweightListenerList, ThreadSafeListenerList + @tags{Core} */ template size(); } + [[nodiscard]] int size() const noexcept { return ! initialised() ? 0 : listeners->size(); } /** Returns true if no listeners are registered, false otherwise. */ - bool isEmpty() const noexcept { return ! initialised() || listeners->isEmpty(); } + [[nodiscard]] bool isEmpty() const noexcept { return ! initialised() || listeners->isEmpty(); } /** Clears the list. - If the ListenerList is cleared during a callback, it is guaranteed that - no more listeners will be called. + If the ListenerList is cleared during a callback, it is guaranteed that no more + listeners will be called. */ void clear() { @@ -181,7 +163,7 @@ public: } /** Returns true if the specified listener has been added to the list. */ - bool contains (ListenerClass* listener) const noexcept + [[nodiscard]] bool contains (ListenerClass* listener) const noexcept { return initialised() && listeners->contains (listener); @@ -191,12 +173,12 @@ public: Any attempt to mutate the array may result in undefined behaviour. - If the array uses a mutex/CriticalSection, reading from the array without first - obtaining the lock may potentially result in undefined behaviour. + If the array uses a mutex/CriticalSection, reading from the array without first obtaining + the lock may potentially result in undefined behaviour. @see add, remove, clear, contains */ - const ArrayType& getListeners() const noexcept + [[nodiscard]] const ArrayType& getListeners() const noexcept { const_cast (this)->initialiseIfNeeded(); return *listeners; @@ -212,8 +194,8 @@ public: std::forward (callback)); } - /** Calls an invokable object for each listener in the list, except for the - listener specified by listenerToExclude. + /** Calls an invokable object for each listener in the list, except for the listener specified + by listenerToExclude. */ template void callExcluding (ListenerClass* listenerToExclude, Callback&& callback) @@ -224,8 +206,8 @@ public: } - /** Calls an invokable object for each listener in the list, additionally - checking the bail-out checker before each call. + /** Calls an invokable object for each listener in the list, additionally checking the bail-out + checker before each call. See the class description for info about writing a bail-out checker. */ @@ -237,9 +219,8 @@ public: std::forward (callback)); } - /** Calls an invokable object for each listener in the list, except for the - listener specified by listenerToExclude, additionally checking the - bail-out checker before each call. + /** Calls an invokable object for each listener in the list, except for the listener specified + by listenerToExclude, additionally checking the bail-out checker before each call. See the class description for info about writing a bail-out checker. */ @@ -248,6 +229,16 @@ public: const BailOutCheckerType& bailOutChecker, Callback&& callback) { + #if JUCE_ASSERTIONS_ENABLED_OR_LOGGED + const ScopedTryLock callCheckedExcludingLock (*callCheckedExcludingMutex); + + // If you hit this assertion it means you're trying to call the listeners from multiple + // threads concurrently. If you need to do this either use a LightweightListenerList, for a + // lock free option, or a ThreadSafeListenerList if you also need the extra guarantees + // provided by ListenerList. See the class descriptions for more details. + jassert (callCheckedExcludingLock.isLocked()); + #endif + if (! initialised()) return; @@ -289,8 +280,8 @@ public: std::forward (args)...); } - /** Calls a specific listener method for each listener in the list, except - for the listener specified by listenerToExclude. + /** Calls a specific listener method for each listener in the list, except for the listener + specified by listenerToExclude. */ template void callExcluding (ListenerClass* listenerToExclude, @@ -404,6 +395,12 @@ private: std::this_thread::yield(); } + #if JUCE_ASSERTIONS_ENABLED_OR_LOGGED + // using a unique_ptr helps keep the size of this class down to prevent excessive stack sizes + // due to objects that contain a ListenerList being created on the stack + std::unique_ptr callCheckedExcludingMutex = std::make_unique(); + #endif + //============================================================================== JUCE_DECLARE_NON_COPYABLE (ListenerList) }; @@ -412,9 +409,255 @@ private: /** A thread safe version of the ListenerList class. - @see ListenerList + @see ListenerList, LightweightListenerList + + @tags{Core} */ template using ThreadSafeListenerList = ListenerList>; +//============================================================================== +/** + A lightweight version of the ListenerList that doesn't provide any guarantees when mutating the + list from a callback, but allows callbacks to be triggered concurrently without a mutex. + + @see ListenerList, ThreadSafeListenerList + + @tags{Core} +*/ +template +class LightweightListenerList +{ +public: + //============================================================================== + /** Creates an empty list. */ + LightweightListenerList() = default; + + /** Destructor. */ + ~LightweightListenerList() + { + // If you hit this jassert it means you're trying to delete the list while iterating through + // the listeners! If you need to handle this situation gracefully use a ListenerList or + // ThreadSafeListenerList. + jassert (numCallsInProgress == 0); + } + + //============================================================================== + /** Adds a listener to the list. + A listener can only be added once, so if the listener is already in the list, this method + has no effect. + + If you need to add a Listener during a callback, use the ListenerList type. + + @see remove + */ + void add (ListenerClass* listenerToAdd) + { + // If you hit this jassert it means you're trying to add a listener while iterating through + // the listeners! If you need to handle this situation gracefully use a ListenerList or + // ThreadSafeListenerList. + jassert (numCallsInProgress == 0); + + if (listenerToAdd != nullptr) + listeners.addIfNotAlreadyThere (listenerToAdd); + else + jassertfalse; // Listeners can't be null pointers! + } + + /** Removes a listener from the list. + If the listener wasn't in the list, this has no effect. + + If you need to remove a Listener during a callback, use the ListenerList type. + */ + void remove (ListenerClass* listenerToRemove) + { + // If you hit this jassert it means you're trying to remove a listener while iterating + // through the listeners! If you need to handle this situation gracefully use a ListenerList + // or ThreadSafeListenerList. + jassert (numCallsInProgress == 0); + + jassert (listenerToRemove != nullptr); // Listeners can't be null pointers! + + listeners.removeFirstMatchingValue (listenerToRemove); + } + + /** Adds a listener that will be automatically removed when the Guard is destroyed. + + Be very careful to ensure that the ErasedScopeGuard is destroyed or released before the + ListenerList is destroyed, otherwise the ErasedScopeGuard may attempt to dereference a + dangling pointer when it is destroyed, which will result in a crash. + */ + [[nodiscard]] ErasedScopeGuard addScoped (ListenerClass& listenerToAdd) + { + add (&listenerToAdd); + return ErasedScopeGuard { [this, &listenerToAdd] { remove (&listenerToAdd); } }; + } + + /** Returns the number of registered listeners. */ + [[nodiscard]] int size() const noexcept { return listeners.size(); } + + /** Returns true if no listeners are registered, false otherwise. */ + [[nodiscard]] bool isEmpty() const noexcept { return listeners.isEmpty(); } + + /** Clears the list. + + If you need to clear the list during a callback, use the ListenerList type. + */ + void clear() + { + // If you hit this jassert it means you're trying to clear the listener list while iterating + // through the listeners! If you need to handle this situation gracefully use a ListenerList + // or ThreadSafeListenerList. + jassert (numCallsInProgress == 0); + + listeners.clear(); + } + + /** Returns true if the specified listener has been added to the list. */ + [[nodiscard]] bool contains (ListenerClass* listener) const noexcept + { + return listeners.contains (listener); + } + + //============================================================================== + /** Calls an invokable object for each listener in the list. */ + template + void call (Callback&& callback) const + { + callCheckedExcluding (nullptr, + DummyBailOutChecker{}, + std::forward (callback)); + } + + /** Calls an invokable object for each listener in the list, except for the listener specified + by listenerToExclude. + */ + template + void callExcluding (ListenerClass* listenerToExclude, Callback&& callback) const + { + callCheckedExcluding (listenerToExclude, + DummyBailOutChecker{}, + std::forward (callback)); + + } + + /** Calls an invokable object for each listener in the list, additionally checking the bail-out + checker before each call. + + See the class description for info about writing a bail-out checker. + */ + template + void callChecked (const BailOutCheckerType& bailOutChecker, Callback&& callback) const + { + callCheckedExcluding (nullptr, + bailOutChecker, + std::forward (callback)); + } + + /** Calls an invokable object for each listener in the list, except for the listener specified + by listenerToExclude, additionally checking the bail-out checker before each call. + + See the class description for info about writing a bail-out checker. + */ + template + void callCheckedExcluding (ListenerClass* listenerToExclude, + const BailOutCheckerType& bailOutChecker, + Callback&& callback) const + { + #if JUCE_ASSERTIONS_ENABLED_OR_LOGGED + ++numCallsInProgress; + const ScopeGuard decrementPerformingCallbackCount { [&] { --numCallsInProgress; }}; + #endif + + for (auto* listener : listeners) + { + if (bailOutChecker.shouldBailOut()) + return; + + if (listener == listenerToExclude) + continue; + + callback (*listener); + } + } + + //============================================================================== + /** Calls a specific listener method for each listener in the list. */ + template + void call (void (ListenerClass::*callbackFunction) (MethodArgs...), Args&&... args) const + { + callCheckedExcluding (nullptr, + DummyBailOutChecker{}, + callbackFunction, + std::forward (args)...); + } + + /** Calls a specific listener method for each listener in the list, except for the listener + specified by listenerToExclude. + */ + template + void callExcluding (ListenerClass* listenerToExclude, + void (ListenerClass::*callbackFunction) (MethodArgs...), + Args&&... args) const + { + callCheckedExcluding (listenerToExclude, + DummyBailOutChecker{}, + callbackFunction, + std::forward (args)...); + } + + /** Calls a specific listener method for each listener in the list, additionally checking the + bail-out checker before each call. + + See the class description for info about writing a bail-out checker. + */ + template + void callChecked (const BailOutCheckerType& bailOutChecker, + void (ListenerClass::*callbackFunction) (MethodArgs...), + Args&&... args) const + { + callCheckedExcluding (nullptr, + bailOutChecker, + callbackFunction, + std::forward (args)...); + } + + /** Calls a specific listener method for each listener in the list, except for the listener + specified by listenerToExclude, additionally checking the bail-out checker before each call. + + See the class description for info about writing a bail-out checker. + */ + template + void callCheckedExcluding (ListenerClass* listenerToExclude, + const BailOutCheckerType& bailOutChecker, + void (ListenerClass::*callbackFunction) (MethodArgs...), + Args&&... args) const + { + callCheckedExcluding (listenerToExclude, bailOutChecker, [&] (ListenerClass& l) + { + (l.*callbackFunction) (args...); + }); + } + + //============================================================================== + /** A dummy bail-out checker that always returns false. + See the class description for info about writing a bail-out checker. + */ + using DummyBailOutChecker = typename ListenerList::DummyBailOutChecker; + + //============================================================================== + using ThisType = LightweightListenerList; + using ListenerType = ListenerClass; + +private: + #if JUCE_ASSERTIONS_ENABLED_OR_LOGGED + mutable std::atomic numCallsInProgress { 0 }; + #endif + + Array listeners; + + //============================================================================== + JUCE_DECLARE_NON_COPYABLE (LightweightListenerList) +}; + } // namespace juce diff --git a/modules/juce_core/containers/juce_ListenerList_test.cpp b/modules/juce_core/containers/juce_ListenerList_test.cpp index e8da6236d7..e7f8bda2d9 100644 --- a/modules/juce_core/containers/juce_ListenerList_test.cpp +++ b/modules/juce_core/containers/juce_ListenerList_test.cpp @@ -103,7 +103,7 @@ public: //============================================================================== ListenerListTests() : UnitTest ("ListenerList", UnitTestCategories::containers) {} - void runTest() override + void runTest() final { // This is a test that the pre-iterator adjustment implementation should pass too beginTest ("All non-removed listeners should be called - removing an already called listener"); @@ -499,6 +499,263 @@ private: } }; +class LightweightListenerListTests final : public UnitTest +{ +public: + LightweightListenerListTests() : UnitTest ("LightweightListenerList", UnitTestCategories::containers) {} + + void runTest() final + { + class Listener + { + public: + void triggerCallback() { ++numCallbacksTriggered; } + int getNumCallbacksTriggered() const { return numCallbacksTriggered; } + + private: + int numCallbacksTriggered = 0; + }; + + beginTest ("Default list is empty"); + { + LightweightListenerList listeners; + expect (listeners.isEmpty()); + expect (listeners.size() == 0); + } + + beginTest ("Adding a listener"); + { + LightweightListenerList listeners; + Listener listener; + + expect (listener.getNumCallbacksTriggered() == 0); + expect (! listeners.contains (&listener)); + + listeners.add (&listener); + expect (! listeners.isEmpty()); + expect (listeners.size() == 1); + expect (listeners.contains (&listener)); + expect (listener.getNumCallbacksTriggered() == 0); + + listeners.call (&Listener::triggerCallback); + expect (listener.getNumCallbacksTriggered() == 1); + expect (! listeners.isEmpty()); + expect (listeners.size() == 1); + + listeners.call (&Listener::triggerCallback); + + expect (listener.getNumCallbacksTriggered() == 2); + } + + beginTest ("Adding the same listener twice"); + { + LightweightListenerList listeners; + Listener listener; + + listeners.add (&listener); + listeners.add (&listener); + + expect (! listeners.isEmpty()); + expect (listeners.size() == 1); + expect (listeners.contains (&listener)); + expect (listener.getNumCallbacksTriggered() == 0); + + listeners.call (&Listener::triggerCallback); + expect (listener.getNumCallbacksTriggered() == 1); + } + + beginTest ("Adding multiple listeners"); + { + LightweightListenerList listeners; + Listener listener1; + Listener listener2; + Listener listener3; + + expect (! listeners.contains (&listener1)); + expect (! listeners.contains (&listener2)); + expect (! listeners.contains (&listener3)); + + listeners.add (&listener1); + expect ( listeners.contains (&listener1)); + expect (! listeners.contains (&listener2)); + expect (! listeners.contains (&listener3)); + + listeners.call (&Listener::triggerCallback); + expect (listener1.getNumCallbacksTriggered() == 1); + expect (listener2.getNumCallbacksTriggered() == 0); + expect (listener3.getNumCallbacksTriggered() == 0); + + listeners.add (&listener2); + expect (! listeners.isEmpty()); + expect (listeners.size() == 2); + expect ( listeners.contains (&listener1)); + expect ( listeners.contains (&listener2)); + expect (! listeners.contains (&listener3)); + + listeners.call (&Listener::triggerCallback); + expect (listener1.getNumCallbacksTriggered() == 2); + expect (listener2.getNumCallbacksTriggered() == 1); + expect (listener3.getNumCallbacksTriggered() == 0); + + listeners.add (&listener3); + expect (! listeners.isEmpty()); + expect (listeners.size() == 3); + expect (listeners.contains (&listener1)); + expect (listeners.contains (&listener2)); + expect (listeners.contains (&listener3)); + + listeners.call (&Listener::triggerCallback); + expect (listener1.getNumCallbacksTriggered() == 3); + expect (listener2.getNumCallbacksTriggered() == 2); + expect (listener3.getNumCallbacksTriggered() == 1); + } + + beginTest ("Removing a listener"); + { + LightweightListenerList listeners; + Listener listener1; + Listener listener2; + Listener listener3; + + listeners.add (&listener1); + listeners.add (&listener2); + listeners.add (&listener3); + + listeners.remove (&listener2); + expect (listeners.size() == 2); + expect (! listeners.contains (&listener2)); + + listeners.call (&Listener::triggerCallback); + expect (listener1.getNumCallbacksTriggered() == 1); + expect (listener2.getNumCallbacksTriggered() == 0); + expect (listener3.getNumCallbacksTriggered() == 1); + + listeners.remove (&listener1); + expect (listeners.size() == 1); + expect (! listeners.contains (&listener1)); + + listeners.call (&Listener::triggerCallback); + expect (listener1.getNumCallbacksTriggered() == 1); + expect (listener2.getNumCallbacksTriggered() == 0); + expect (listener3.getNumCallbacksTriggered() == 2); + + listeners.remove (&listener3); + expect (listeners.size() == 0); + expect (! listeners.contains (&listener3)); + + listeners.call (&Listener::triggerCallback); + expect (listener1.getNumCallbacksTriggered() == 1); + expect (listener2.getNumCallbacksTriggered() == 0); + expect (listener3.getNumCallbacksTriggered() == 2); + } + + beginTest ("Adding a scoped listener"); + { + LightweightListenerList listeners; + Listener listener; + + { + const auto scopeGuard = listeners.addScoped (listener); + expect (! listeners.isEmpty()); + expect (listeners.size() == 1); + expect (listeners.contains (&listener)); + } + + expect (listeners.isEmpty()); + expect (listeners.size() == 0); + expect (! listeners.contains (&listener)); + } + + beginTest ("Clear the listeners"); + { + LightweightListenerList listeners; + Listener listener1; + Listener listener2; + Listener listener3; + + listeners.add (&listener1); + listeners.add (&listener2); + listeners.add (&listener3); + + listeners.clear(); + expect (listeners.isEmpty()); + expect (listeners.size() == 0); + + listeners.call (&Listener::triggerCallback); + expect (listener1.getNumCallbacksTriggered() == 0); + expect (listener2.getNumCallbacksTriggered() == 0); + expect (listener3.getNumCallbacksTriggered() == 0); + } + + beginTest ("Call excluding"); + { + LightweightListenerList listeners; + Listener listener1; + Listener listener2; + Listener listener3; + + listeners.add (&listener1); + listeners.add (&listener2); + listeners.add (&listener3); + + listeners.callExcluding (&listener1, &Listener::triggerCallback); + expect (listener1.getNumCallbacksTriggered() == 0); + expect (listener2.getNumCallbacksTriggered() == 1); + expect (listener3.getNumCallbacksTriggered() == 1); + + listeners.callExcluding (&listener2, &Listener::triggerCallback); + expect (listener1.getNumCallbacksTriggered() == 1); + expect (listener2.getNumCallbacksTriggered() == 1); + expect (listener3.getNumCallbacksTriggered() == 2); + + listeners.callExcluding (&listener3, &Listener::triggerCallback); + expect (listener1.getNumCallbacksTriggered() == 2); + expect (listener2.getNumCallbacksTriggered() == 2); + expect (listener3.getNumCallbacksTriggered() == 2); + + } + + beginTest ("Call with bail-out checker"); + { + LightweightListenerList listeners; + Listener listener1; + Listener listener2; + Listener listener3; + + listeners.add (&listener1); + listeners.add (&listener2); + listeners.add (&listener3); + + struct BailOutChecker + { + bool shouldBailOut() const { return f(); } + std::function f; + }; + + const BailOutChecker bailOutChecker { [&] { return listener2.getNumCallbacksTriggered() == 2; } }; + + // all the listeners should be called + listeners.callChecked (bailOutChecker, &Listener::triggerCallback); + expect (listener1.getNumCallbacksTriggered() == 1); + expect (listener2.getNumCallbacksTriggered() == 1); + expect (listener3.getNumCallbacksTriggered() == 1); + + // only listeners 1 and 2 should be called + listeners.callChecked (bailOutChecker, &Listener::triggerCallback); + expect (listener1.getNumCallbacksTriggered() == 2); + expect (listener2.getNumCallbacksTriggered() == 2); + expect (listener3.getNumCallbacksTriggered() == 1); + + // none of the listeners should be called + listeners.callChecked (bailOutChecker, &Listener::triggerCallback); + expect (listener1.getNumCallbacksTriggered() == 2); + expect (listener2.getNumCallbacksTriggered() == 2); + expect (listener3.getNumCallbacksTriggered() == 1); + } + } +}; + static ListenerListTests listenerListTests; +static LightweightListenerListTests lightweightListenerListTests; } // namespace juce