A ChangeBroadcaster / ChangeListener version with WeakReferences


#1

Hi everybody,

 

after chasing down the 1000th dangling pointer access of a ChangeBroadcaster (which is incredibly painful, since there is no stack trace for asynchronous callbacks), I gave up and wrote a small class that uses a list of WeakReferences.

I know if you deregister the ChangeListener in its destructor, everything is fine, but if you forget it one time, hell breaks loose and you spent hours trying to find the culprit.

So here is the class:


class SafeChangeBroadcaster;
/** A class for message communication between objects.
*
*    This class has the same functionality as the JUCE ChangeListener class, but it uses a weak reference for the internal list, 
*    so deleted listeners will not crash the application. 
*/
class SafeChangeListener
{
public:
    virtual ~SafeChangeListener()
    {
        masterReference.clear();
    }
    /** Overwrite this and handle the message. */
    virtual void changeListenerCallback(SafeChangeBroadcaster *b) = 0;
private:
    friend class WeakReference<SafeChangeListener>;
    WeakReference<SafeChangeListener>::Master masterReference;
};


/** A drop in replacement for the ChangeBroadcaster class from JUCE but with weak references.
*
*    If you use the normal class and forget to unregister a listener in its destructor, it will crash the application.
*    This class uses a weak reference (but still throws an assertion so you still recognize if something is funky), so it handles this case much more gracefully.
*
*    Also you can add a string to your message for debugging purposes (with the JUCE class you have no way of knowing what caused the message if you call it asynchronously.
*/
class SafeChangeBroadcaster
{
public:
    SafeChangeBroadcaster():
        dispatcher(this)
    {};
    virtual ~SafeChangeBroadcaster()
    {
        dispatcher.cancelPendingUpdate();
    };
    /** Sends a synchronous change message to all the registered listeners.
    *
    *    This will immediately call all the listeners that are registered. For thread-safety reasons, you must only call this method on the main message thread.
    */
    void sendSynchronousChangeMessage(const String &identifier = String::empty)
    {
        currentString = identifier;
        // Ooops, only call this in the message thread.
        // Use sendChangeMessage() if you need to send a message from elsewhere.
        jassert(MessageManager::getInstance()->isThisTheMessageThread());
        ScopedLock sl(listeners.getLock());
        for(int i = 0; i < listeners.size(); i++)
        {
            if(listeners[i].get() != nullptr)
            {
                listeners[i]->changeListenerCallback(this);
            }
            else
            {
                // Ooops, you called an deleted listener. 
                // Normally, it would crash now, but since this is really lame, this class only throws an assert!
                jassertfalse;
                listeners.remove(i--);
            }
        }
    };
    /** Registers a listener to receive change callbacks from this broadcaster.
    *
    *    Trying to add a listener that's already on the list will have no effect.
    */
    void addChangeListener(SafeChangeListener *listener)
    {
        ScopedLock sl(listeners.getLock());
        listeners.addIfNotAlreadyThere(listener);
    }
    /**    Unregisters a listener from the list.
    *
    *    If the listener isn't on the list, this won't have any effect.
    */
    void removeChangeListener(SafeChangeListener *listener)
    {
        ScopedLock sl(listeners.getLock());
        listeners.removeAllInstancesOf(listener);
    }
    /** Removes all listeners from the list. */
    void removeAllChangeListeners()
    {
        dispatcher.cancelPendingUpdate();
        ScopedLock sl(listeners.getLock());
        listeners.clear();
    }
    /** Causes an asynchronous change message to be sent to all the registered listeners.
    *
    *    The message will be delivered asynchronously by the main message thread, so this method will return immediately. 
    *    To call the listeners synchronously use sendSynchronousChangeMessage().
    */
    void sendChangeMessage(const String &identifier=String::empty)
    {
        currentString = identifier;
        dispatcher.triggerAsyncUpdate();
    };
private:
    class AsyncBroadcaster: public AsyncUpdater
    {
    public:
        AsyncBroadcaster(SafeChangeBroadcaster *parent_):
            parent(parent_)
        {}
        void handleAsyncUpdate() override
        {
            parent->sendSynchronousChangeMessage(parent->currentString);
        }
        SafeChangeBroadcaster *parent;
    };
    AsyncBroadcaster dispatcher;
    String currentString;
    Array<WeakReference<SafeChangeListener>, CriticalSection> listeners;
};

It is supposed to be a drop in replacement for the original ChangeBroadcaster. Simply run the following search & replace operations on your codebase:

"public ChangeBroadcaster" -> "public SafeChangeBroadcaster" (changes the inheritance for ChangeBroadcaster)

"public ChangeListener" -> "public SafeChangeListener" (changes the inheritance for ChangeListeners)

"changeListenerCallback(ChangeBroadcaster *" -> changeListenerCallback(SafeChangeBroadcaster *" (changes the virtual function signature for ChangeListeners)

These S&R operations worked for me (but there's no guarantee that they will work with your code!). Beware that if you use JUCE classes which are ChangeBroadcasters (eg. AudioThumbnail), you have do revert the search and replace for classes which listen to those ChangeBroadcasters (or use multiple inheritance in case you need both types)

 

It has basic thread safety (it assumes that adding / removing listeners happens in the message thread). And I didn't implement all methods of the original ChangeBroadcaster / Listener (because I didn't need them until now, but it should be trivial to do this.)

Let me know if you find this helpful or if you have a smarter solution to this problem (maybe there is something really easy I didn't see).

It still throws an assert if you call a dangling pointer (so you don't get too lazy), but it doesn't crash anymore, so everytime you hit the assert, lean back and enjoy the subtle breeze of a non-crashing application :)