Thread-Safety of ListenerList

[lalala]

+1 for a note in the documentation regarding AudioProcessorValueTreeState::Listeners that shouldn’t be added at runtime!

@reuk

@vallant

All attachments derive from AttachedControlBase , which just adds itself as a listener to AudioProcessorValueTreeState::ParameterAdapter::listeners . Different class, same problem.

This has changed on the juce6 branch, where attachments listen to parameters directly.

Are you considering to push that in juce 5?
otherwise we can’t use any of the ButtonAttachment, etc. classes without getting big thread safety troubles, correct?

[lalala]

gentle bump

[reuk]

I’ve updated the ListenerList in the APVTS::ParameterAdapter so that it is protected by a CriticalSection. This should fix the thread safety issues, although priority inversions may cause audio dropouts if listeners are regularly added and removed during playback. For this reason I recommend attaching/detaching parameter listeners as infrequently as possible (ideally just in the AudioProcessor constructor and destructor).

APVTS: Make adding/removing listeners threadsafe

[daniel]

@reuk

For this reason I recommend attaching/detaching parameter listeners as infrequently as possible (ideally just in the AudioProcessor constructor and destructor).

Thanks for the fix. However, don’t we need the parameter Listeners for GUI attachments as well, regardless of using the AudioProcessorValueTreeState or the new ones?

My understanding so far was, that the APVTS was exactly responsible for that decoupling…

What is the new advice to avoid priority inversion when the user opens an editor?

[reuk]

Unfortunately I’m not sure we have an out-of-the-box solution to connect/disconnect UI controls to parameters in a realtime-safe way, at the moment. My advice to avoid priority inversion when opening/closing the editor would be to have the editor attach to a secondary ListenerList (i.e. not attach directly to the parameter). Individual parameters would trigger this secondary ListenerList by sending async updates (listeners for these updates could be attached in the plugin processor constructor, rather than the constructor of the editor). This sort of just moves the problem around, though, because sending an async message still isn’t technically realtime safe.

[daniel]

Seems to me like an entity like the APVTS should be a listener during the whole lifetime of the processor, and the UI can then listen to the APVTS, which is decoupled from the parameters, e.g. with a Timer fetching the updates all at once (which is a centralised version of what you proposed).

Wasn’t that the initial setup before the refactoring? Seems to me we found the reason why Fabian added the APVTS in the first place…

I have to consider now if I build my own APVTS to povide that listener decoupling :-/

[reuk]

I checked, and it looks like the APVTS has had per-parameter ListenerLists since before I first made any modifications (going back to 2015!) so I don’t think the APVTS has ever been a ‘central listener’ like you describe.

juce-framework/JUCE/blob/b1a8470514b3d998c62a9f5f06b47c1e09b5743f/modules/juce_audio_processors/utilities/juce_AudioProcessorValueTreeState.cpp#L146

[daniel]

Cool, thanks for checking! I must confused that then with something else. Should have read the source myself.

So would it make sense to handle it in the future that way, having the listeners attached for the lifetime in the APVTS? Or is that a silly idea at all?

[reuk]

Perhaps, but such an approach would once again force all GUI listeners to have knowledge of the APVTS, rather than individual parameters, which would not be ideal.

[daniel]

That’s right. On the other hand considering the ideas for that problem shown by @dave96 in his talk at the meetup it could be a way to have only one timer running for all RealTimeAsyncUpdaters creating a realtime safe solution without priority inversion.

Anyway, thanks for your thoughts!

[vallant]

Thanks you for the quick fix and the insights!
What implications does this have for JUCE6? You mentioned that the attachments in juce6 are attached directly to the parameter, so is this whole parameter+message handling thread-safe in the new major?

[reuk]

I applied the fix above to both JUCE 5 and 6, so adding/removing APVTS and AudioProcessorParameter listeners should be threadsafe for both.

[vallant]

Awesome, thanks!

[Rincewind]

@reuk following up on this (a few years later): I think the ListenerList should be compatible with a spinning ReadWriteLock, and by that might be a good candidate to improve the AudioProcessorParameter. At the moment, the main issue I’m having with AudioParameters and the ListenerList itself is, that they involve a CriticalSection when I want to call an event (or the parameter value changes), even though it fairly save to assume, that especially in smaller plugin applications (which seem to be the main product JUCE is used for), add and remove listener are hardly ever called, but parameter values or events might be emitted all the time by automations or MIDI. At the moment, this seems to be the prime example of priority inversion even when nothing happens on the array size. The thread notifying about changes (possibly the audio thread via MIDI or some other time critical thread by the DAW) is performing a system call for the sole purpose to guard a change of listeners even though that hardly ever changes. Possibly only when the application starts and stops and for the rest of the maybe 10% (guessed - don’t sue me :D) while the user is doing stuff in the UI that involves more than changing a few knobs fiddling with the sound.
Do you have any thoughts on this? Is there some obvious reason I’m not seeing why that wasn’t implemented that way originally?