ListenerList crashes on AudioProcessor destruction

[rabbitalgorithms]

Hi all,

My plug-in has a DataModel class that acts as a wrapper around a ValueTree, with its own Listener class and ListenerList for callbacks when changes are made to the data model. It is set-up in a similar manner to the JUCE - SamplerPlugin demo project.
I have one main instance of the DataModel in my AudioProcessor which then gets copied around to other classes that then register as listeners to their copy. This way each class owns the DataModel object that it refers to and any Listeners and ListenerLists take care of themselves on destruction without any fuss (no dangling Listeners).

However, when I register the AudioProcessor as a Listener to the main DataModel instance it crashes (exc_bad_access) on destruction in this iterator within the ListenerList destructor:

/** Destructor. */
    ~ListenerList()
    {
        WrappedIterator::forEach (activeIterators, [&] (auto& iter)
        {
            iter.invalidate();
        });
    } 

Adding in a removeListener (this) to the processor’s destructor does not help either as a get
exc_bad_access during the iterator of the remove() function in ListenerList().

It is quite confusing because it works like a charm in every other GUI class on destruction.
Am I missing something obvious with the ListenerList clas?

Thanks a bunch!

-Cameron

[attila]

It seems that the ListenerList has already been deleted when its destructor is called. You say the DataModel has its own ListenerList, and the model gets copied around, but the ListenerList class being non-copyable I can’t piece together the full picture yet.

If you can share a minimal example that reproduces the issue, I am happy to take a look.

[rabbitalgorithms]

Hi Attila, thanks for the response.
Since posting I changed my code slightly to not copy the DataModel around in an attempt to get rid of this issue. Now my DataModel is working as a ValueTreeWrapper for the ValueTree in APVTS, which I am passing around to the GUI and other processors. Though that has helped I still get the crash every now and then at seemly random times.
Here is an example of how I am using the DataModel.

A simplified version of DataModel:

class SamplerParameters : public ValueTree::Listener
{
public:
    SamplerParameters(const ValueTree& ss) : samplerState (ss)
    {
        jassert (samplerState. hasType (IDs::SAMPLER));
        samplerState.addListener(this);
    }
    
    class Listener
    {
    public:
        virtual ~Listener() noexcept = default;
        virtual void exampleCallback (int a) {}
    };
    
    void addListener (Listener* listener)
    {
        listenerList.add (listener);
    }
private:
    ValueTree samplerState;
    ListenerList<Listener> listenerList;
};

Use case in the editor:

// PluginEditor.h
class ExampleAudioProcessorEditor : public juce::AudioProcessorEditor, 
public SamplerParameters::Listener
{
public:
ExampleAudioProcessorEditor (ExampleAudioProcessor& ap) :
audioProcessor (ap), samplerParams (audioProcessor.getSamplerState())
{
  samplerParams.addListener(this);
}

void exampleCallback (int a) override
{
}
......
private:
ExampleAudioProcessor& audioProcessor;
SamplerParameters samplerParams;
};

// getSamplerState() function in AudioProcessor
ValueTree getSamplerState()
{
    return apvts.state.getChildWithName(IDs::SAMPLER);
}

I am also using a similar approach in my samplerProcessor.h and .cpp files. Instead, I just pass a reference to the APVTS, instead of the AudioProcessor.

Am I missing something?

Thanks so much for your help!

-Cameron

[attila]

Your code sample is reasonable, and can’t see anything that would definitely cause a crash.

I’m guessing here, but in case you are accessing the ListenerList from the ValueTree::Listener callback, that might lead to a crash, as the ListenerList is destroyed before the ValueTree and you are not calling ValueTree::removeListener() in the destructor either.

I’m trying to ascertain whether there might be a bug in the ListenerList implementation but so far I haven’t been able to come up with a project that would produce a crash similar to yours, so for now, I’ll be assuming it’s fine.

[chkn]

I’m not sure if this has anything todo with it, but there is a potential race condition with the public access to AudioProcessorValueTreeState::state.

While any access to the value tree internally is secured by the “valueTreeChanging” CriticalSection, it is still possible to interact with it, without locking the CriticalSection.

[gustav-scholda]

@rabbitalgorithms Have you found a solution to this?

We are currently experiencing the same issue. For some reason removing the listener is fine but then we get a exc_bad_access in the WrappedIterator. I wonder how that can even happen?

Also why is the WrappedIterator even here in the first place? If I remove it and just use the array of listeners instead everything is fine. All it does is returning the listener objects anyway as far as I understand it. There is not a single comment about it though, so maybe anyone from JUCE can shed some light on this?

Thanks!

[reuk]

If you haven’t tested with Address Sanitizer yet, then I recommend trying that.

Often when undefined behaviour happens due to a bad memory access, the actual crash can happen some time after the faulty code was executed. Removing the WrappedIterator could well be masking the problem - I highly doubt that it’s a fix.

Address Sanitizer will print some diagnostics as soon as a bad memory access happens, so it should give you a much clearer picture of what’s going wrong.

[gustav-scholda]

Will do @reuk and post again here.

The interesting part is we don‘t get any crash at all if we remove WrappedIterator (which we still don‘t know why it‘s here in the first place).

[attila]

To shed some light on this, there is a clause in the documentation saying:

“It is guaranteed that every Listener is called during an iteration if it’s inside the ListenerList before the iteration starts and isn’t removed until its end. This guarantee holds even if some Listeners are removed or new ones are added during the iteration.”

The WrappedIterator’s purpose is providing this guarantee. This is a tricky area but I hope you can pinpoint the cause of your issue.

[yfede]

@attila

“It is guaranteed that every Listener is called during an iteration if it’s inside the ListenerList before the iteration starts and isn’t removed until its end. This guarantee holds even if some Listeners are removed or new ones are added during the iteration.”

Slightly OT (but relevant to the above quote):

What happens if a listener is removed during the iteration?

The rest of the doc after the quote you mentioned only says:

Listeners added during an iteration are guaranteed to be not called in that iteration.

…which deals with the case of listeners added during the iteration, and:

Sometimes, there’s a chance that invoking one of the callbacks might result in the list itself being deleted while it’s still iterating - to survive this situation, you can (etc.)

…which deals with the case of the whole list being destroyed while it’s iterating. But nothing is said regarding the case of a listener being removed from the list during the iteration.

What happens in that case?
Inferring from what’s said above, it would make sense that it gets called if and only if it gets iterated over before its removal from the list.
But in whatever case, it would be nice if the doc stated that clearly to cover all cases.

/OT

[gustav-scholda]

Thanks @attila I understand the idea now. It’s still a mystery to us why we crash though.

@yfede

But nothing is said regarding the case of a listener being removed from the list during the iteration.

What happens in that case?

This is what’s probably happening here. Address Sanitizer throws stack-use-after-return. Removing WrappedIterator results in no crash at all.

[dhilowitz]

I’m experiencing the same thing with DecentSampler. It seems to happen very intermittently. @gustav-scholda when you say you remove the WrapperIterator, are you saying you just comment out this code entirely?

        WrappedIterator::forEach (activeIterators, [&] (auto& iter)
        {
            if (0 <= index && index < iter.get().index)
                --iter.get().index;
        });

[attila]

Your conclusion is right, a listener is guaranteed not to be called after it has been removed.

It’s still not clear how these crashes are happening, or that they are caused by the ListenerList implementation, but I’ll see if I could repro a similar crash.

[attila]

I can’t repro this problem by running a randomised, iteration-insertion-deletion-nested-iteration test under address sanitizier, so my continued assumption is that the problem lies outside the JUCE implementation.

[dhilowitz]

The frustrating thing is it seems to be happening so intermittently. I can work with my plugin for days, and then suddenly I’ll see a crash when I’m shutting down the plugin or when I’m switching presets. It’s clearly a timing issue having to do with the order in which things are deleted but because I can’t reliably reproduce it, I can’t fix it…

[momentumm]

I’ve been encountering this as well.

Instead of using an AudioProcessor, I am just setting up my own “Program State” and running a ValueTree with some basic listeners (new state, state changed, etc). I tried to implement more specific callbacks by setting up the ValueTree Listener and running functions inside of those callbacks that go to my own “Program State” listener callbacks, and not only am I also encountering the removeListener() crash, The ListenerList that I was using inside of the Program State no longer seems to be working at all… (at least, when ValueTree::Listener is inherited)

Very strange behavior indeed. I’d welcome any suggestions for dealing with it with just some kind of kludgy workaround.

[cpr2323]

Anecdotal info, and probably not directly helpful, over the past few months my pet project has experienced a variety of ValueTree listener callback crashes, which have boiled down lack of destructors in one of my classes, resulting in the VT not being deleted. In my case, always solved with a simple = default dtor declaration. But, it’s never obvious in my crash stacks where it’s missing from.

[Verbonaut]

You can probably just use the version that was included in Juce 5/6. If you’re able to produce a simple test case that illustrates the problem with the new version that can be shared with the team it may help resolve it.

[bsangston]

I am also experiencing this intermittently for what seems like very simple/standard usage of the APVTS. Semi-consistently reproduceable when running pluginval on my actual projects, but I can’t seem to repro in an isolated test case and really have no clue why this is occurring.

This occurs in deletePluginAsync after completing fuzz tests and tearing down the plugin.

In my case, I simply have my AudioProcessor inheriting AudioProcessorValueTreeState::Listener, and the APVTS as a member variable. And only adding/removing parameter listeners in the constructor/destructor. Roughly:

class MyPluginProcessor : public AudioProcessor, public AudioProcessorValueTreeState::Listener
{
public:
    MyPluginProcessor() : vts(*this, ...)  { vts.addParameterListener(PARAM_ID, this);  }
    ~MyPluginProcessor() { vts.removeParameterListener(PARAM_ID, this); }
    void parameterChanged(const String & paramID, float newValue) override { ... }
private:
    AudioProcessorValueTreeState vts;
}

I am responding to some parameter changes via parameterChanged() callback and occasionally setting parameters from the processor by assigning values via getParameterAsValue() and/or getParameter() + setValueNotifyingHost() calls.

I get the access violation in the destructor of the APVTS’s ListenerList during the WrappedIterator::invalidate calls mentioned in the original post. Where, the WrappedIterator in the activeIterator list is pointing to a deleted node.

In my case, the parallel call stacks just show that every other thread is waiting. So, not much to go on.

[cpr2323]

While I did have another VT problem that I posted about previously, I too am seeing this. I am not using APVTS, just VT’s. It does not happen consistently, but is always in WrappedIterator::invalidate and happens at shutdown. I would gladly put time into debugging if juce devs could advise. The first advice could be about how to determine which ValueTree is throwing the error. Anecdotally, I think it doesn’t always happen in the same place.

>	light_plug.exe!juce::ListenerList<juce::ValueTree::Listener,juce::Array<juce::ValueTree::Listener *,juce::DummyCriticalSection,0>>::WrappedIterator::invalidate() Line 349	C++
 	light_plug.exe!<lambda_5f42dd01ee138921eddb4a4690ba0533>::operator()<juce::ListenerList<juce::ValueTree::Listener,juce::Array<juce::ValueTree::Listener *,juce::DummyCriticalSection,0>>::WrappedIterator>(juce::ListenerList<juce::ValueTree::Listener,juce::Array<juce::ValueTree::Listener *,juce::DummyCriticalSection,0>>::WrappedIterator & iter) Line 82	C++
 	light_plug.exe!juce::ListenerList<juce::ValueTree::Listener,juce::Array<juce::ValueTree::Listener *,juce::DummyCriticalSection,0>>::WrappedIterator::forEach<<lambda_5f42dd01ee138921eddb4a4690ba0533>>(juce::ListenerList<juce::ValueTree::Listener,juce::Array<juce::ValueTree::Listener *,juce::DummyCriticalSection,0>>::WrappedIterator * wrapped, juce::ListenerList<juce::ValueTree::Listener,juce::Array<juce::ValueTree::Listener *,juce::DummyCriticalSection,0>>::{dtor}::__l2::<lambda_5f42dd01ee138921eddb4a4690ba0533> && cb) Line 346	C++
 	light_plug.exe!juce::ListenerList<juce::ValueTree::Listener,juce::Array<juce::ValueTree::Listener *,juce::DummyCriticalSection,0>>::~ListenerList<juce::ValueTree::Listener,juce::Array<juce::ValueTree::Listener *,juce::DummyCriticalSection,0>>() Line 83	C++
 	light_plug.exe!juce::ValueTree::~ValueTree() Line 641	C++
 	[External Code]	
 	light_plug.exe!ControlPanel::~ControlPanel() Line 12	C++
 	[External Code]	
 	light_plug.exe!LightPlugMainWindow::~LightPlugMainWindow() Line 25	C++
 	light_plug.exe!LightPlugComponent::~LightPlugComponent() Line 12	C++
 	light_plug.exe!MainComponent::~MainComponent() Line 32	C++
 	[External Code]	
 	light_plug.exe!LightPlugAudioProcessorEditor::~LightPlugAudioProcessorEditor() Line 10	C++
 	[External Code]	
 	light_plug.exe!juce::StandaloneFilterWindow::MainContentComponent::~MainContentComponent() Line 928	C++
 	[External Code]	
 	light_plug.exe!juce::Component::SafePointer<juce::Component>::deleteAndZero() Line 2317	C++
 	light_plug.exe!juce::ResizableWindow::clearContentComponent() Line 108	C++
 	light_plug.exe!juce::StandaloneFilterWindow::~StandaloneFilterWindow() Line 800	C++
 	[External Code]	
 	light_plug.exe!juce::StandaloneFilterApp::shutdown() Line 117	C++
 	light_plug.exe!juce::JUCEApplicationBase::shutdownApp() Line 331	C++
 	light_plug.exe!juce::JUCEApplicationBase::main() Line 269	C++
 	light_plug.exe!WinMain(HINSTANCE__ * __formal, HINSTANCE__ * __formal, char * __formal, int __formal) Line 193	C++