App crashes on `Array::addIfNotAlreadyThere` of `new AudioProcessorValueTreeState::SliderAttachment`

I have the following code, which crashes on the Array::contains method inside of Array::addIfNotAlreadyThere:

new juce::AudioProcessorValueTreeState::SliderAttachment(
                                        *processorValueTreeState,
                                        state.second->clientParameter->getId(),
                                        *slider.second)

Here’s the callstack:

 	NoneApp.vst3!juce::Array<juce::AudioProcessorParameter::Listener *,juce::DummyCriticalSection,0>::contains(juce::AudioProcessorParameter::Listener * elementToLookFor) Line 407	C++
 	NoneApp.vst3!juce::Array<juce::AudioProcessorParameter::Listener *,juce::DummyCriticalSection,0>::addIfNotAlreadyThere(juce::AudioProcessorParameter::Listener * newElement) Line 526	C++
 	NoneApp.vst3!juce::AudioProcessorParameter::addListener(juce::AudioProcessorParameter::Listener * newListener) Line 1579	C++
 	NoneApp.vst3!juce::ParameterAttachment::ParameterAttachment(juce::RangedAudioParameter & param, std::function<void __cdecl(float)> parameterChangedCallback, juce::UndoManager * um) Line 36	C++
 	NoneApp.vst3!juce::SliderParameterAttachment::SliderParameterAttachment(juce::RangedAudioParameter & param, juce::Slider & s, juce::UndoManager * um) Line 117	C++
 	NoneApp.vst3!std::make_unique<juce::SliderParameterAttachment,juce::RangedAudioParameter &,juce::Slider &,juce::UndoManager * const &,0>(juce::RangedAudioParameter & <_Args_0>, juce::Slider & <_Args_1>, juce::UndoManager * const & <_Args_2>) Line 3416	C++
 	NoneApp.vst3!juce::makeAttachment<juce::SliderParameterAttachment,juce::Slider>(const juce::AudioProcessorValueTreeState & stateToUse, const juce::String & parameterID, juce::Slider & control) Line 493	C++
 	NoneApp.vst3!juce::AudioProcessorValueTreeState::SliderAttachment::SliderAttachment(juce::AudioProcessorValueTreeState & stateToUse, const juce::String & parameterID, juce::Slider & slider) Line 502	C++

The error, itself, is a read access violation for value e in Array::contains. This is because Array::values is apparently never set (has meaningless values and a bad data reference).

In what order do you construct the Slider and the SliderAttachment? The Slider must be fully constructed before you attempt to create a SliderAttachment.

I construct everything in the app first, all as an app object, and then I add the SliderAttachment, so it is fully constructed by then.

I checked in Visual Studio and the slider does appear to be well-constructed, all the values are fine and nothing looks uninitialized. Here’s what the variable looks like:

-		state.second	unique_ptr {stateManager=0x0000000009f0b458 {statesById={ size=5 } statesByName={ size=5 } sliderAttachments={ size=1 } ...} ...}	const std::unique_ptr<none::State,std::default_delete<none::State>>
-		[ptr]	0x000000000077ca80 {stateManager=0x0000000009f0b458 {statesById={ size=5 } statesByName={ size=5 } sliderAttachments=...} ...}	none::State *
+		none::StateObserver	{...}	none::StateObserver
+		stateManager	0x0000000009f0b458 {statesById={ size=5 } statesByName={ size=5 } sliderAttachments={ size=1 } ...}	none::StateManager *
-		component	0x00000000006f8e40 {resizable=true (205) }	juce::Component * {none::MainWindow<none::app::AppMainWindowState>}
+		[none::MainWindow<none::app::AppMainWindowState>]	{resizable=true (205) }	none::MainWindow<none::app::AppMainWindowState>
+		juce::MouseListener	{...}	juce::MouseListener
+		componentName	{text={data=0x00007ff98b40a118 "" } }	juce::String
+		componentID	{text={data=0x000000000073d6c0 "class none::app::FrequencySlider" } }	juce::String
+		parentComponent	0x000000000074d2b0 {...}	juce::Component * {none::app::AppMainWindow}
+		boundsRelativeToParent	{pos={x=50 y=30 } w=100 h=100 }	juce::Rectangle<int>
+		positioner	empty	std::unique_ptr<juce::Component::Positioner,std::default_delete<juce::Component::Positioner>>
+		affineTransform	empty	std::unique_ptr<juce::AffineTransform,std::default_delete<juce::AffineTransform>>
+		childComponentList	{values={elements={data=0x000000000073cdb0 {0x0000000009edfe50 {componentName={text={data=0x00007ff98b40a118 "" } } ...}} } ...} }	juce::Array<juce::Component *,juce::DummyCriticalSection,0>
+		lookAndFeel	{holder={referencedObject=0x0000000000000000 <NULL> } }	juce::WeakReference<juce::LookAndFeel,juce::ReferenceCountedObject>
+		cursor	{cursorHandle=0x0000000000000000 <NULL> leakDetector179={...} }	juce::MouseCursor
+		effect	0x0000000000000000 <NULL>	juce::ImageEffectFilter *
+		cachedImage	empty	std::unique_ptr<juce::CachedComponentImage,std::default_delete<juce::CachedComponentImage>>
+		mouseListeners	empty	std::unique_ptr<juce::Component::MouseListenerList,std::default_delete<juce::Component::MouseListenerList>>
+		keyListeners	empty	std::unique_ptr<juce::Array<juce::KeyListener *,juce::DummyCriticalSection,0>,std::default_delete<juce::Array<juce::KeyListener *,juce::DummyCriticalSection,0>>>
+		componentListeners	{listeners={values={elements={data=0x0000000000000000 {???} } numAllocated=0 numUsed=0 } } }	juce::ListenerList<juce::ComponentListener,juce::Array<juce::ComponentListener *,juce::DummyCriticalSection,0>>
+		properties	{values={values={elements={data=0x0000000000000000 <NULL> } numAllocated=0 numUsed=0 } } }	juce::NamedValueSet
+		masterReference	{sharedPointer={referencedObject=0x000000000c28e140 {owner=0x00000000006f8e40 {resizable=true (205) } } } }	juce::WeakReference<juce::Component,juce::ReferenceCountedObject>::Master
		componentFlags	2050	unsigned int
+		flags	{hasHeavyweightPeerFlag=false visibleFlag=true opaqueFlag=false ...}	juce::Component::ComponentFlags
		componentTransparency	0 '\0'	unsigned char
		leakDetector2388	{...}	juce::LeakedObjectDetector<juce::Component>
+		clientParameter	unique_ptr {longBufferIdx={ size=2305843009213689358 } maxLongBufferIdx=17 newBuffersSinceStart=31 ...}	std::unique_ptr<none::ParameterBase,std::default_delete<none::ParameterBase>>
+		clientParameterUpdateCallback	void <lambda>(float newValue){__this=0x00000000006f9058 {...} }	std::function<void __cdecl(float)>
+		[deleter]	default_delete	std::_Compressed_pair<std::default_delete<none::State>,none::State *,1>
+		[Raw View]	{_Mypair=default_delete }	const std::unique_ptr<none::State,std::default_delete<none::State>>

I was setting the pointer in the constructor of the slider, and changing this had no effect.

Alright. Is the APVTS definitely fully constructed, and are you definitely attaching to a parameter ID which has been previously added to the APVTS?

I found that the app was calling the function to add the slider and the parameter multiple times, so I added a check to make sure it only runs one time and it seems to be past that error. I think it had to do with pointer ownership. However, the process still doesn’t complete, leaving me with a new error:

Here’s the callstack from an FL Studio call:

 	NoneApp.vst3!juce::ValueTree::setPropertyExcludingListener(juce::ValueTree::Listener * listenerToExclude, const juce::Identifier & name, const juce::var & newValue, juce::UndoManager * undoManager) Line 766	C++
 	NoneApp.vst3!juce::ValueTree::setProperty(const juce::Identifier & name, const juce::var & newValue, juce::UndoManager * undoManager) Line 760	C++
>	NoneApp.vst3!juce::AudioProcessorValueTreeState::ParameterAdapter::flushToTree(const juce::Identifier & key, juce::UndoManager * um) Line 146	C++
 	NoneApp.vst3!juce::AudioProcessorValueTreeState::flushParameterValuesToValueTree() Line 473	C++
 	NoneApp.vst3!juce::AudioProcessorValueTreeState::timerCallback() Line 480	C++
 	NoneApp.vst3!juce::Timer::TimerThread::callTimers() Line 119	C++
 	NoneApp.vst3!juce::Timer::TimerThread::CallTimersMessage::messageCallback() Line 181	C++
 	NoneApp.vst3!juce::InternalMessageQueue::dispatchMessage(juce::MessageManager::MessageBase * message) Line 202	C++
 	NoneApp.vst3!juce::InternalMessageQueue::dispatchMessages() Line 240	C++
 	NoneApp.vst3!juce::InternalMessageQueue::messageWndProc(HWND__ * h, unsigned int message, unsigned __int64 wParam, __int64 lParam) Line 163	C++
 	user32.dll!UserCallWinProcCheckWow()	Unknown
 	user32.dll!DispatchMessageWorker()	Unknown
 	FLEngine_x64.dll!000000000282fae8()	Unknown

It crashes on this line:
jassert (object != nullptr); // Trying to add a property to a null ValueTree will fail!

Clearly, at this point the ValueTree is null. Two calls up from that, I see the tree as:

-		tree	{object={referencedObject=0x0000000000000000 <NULL> } listeners={listeners={values={elements={data=0x0000000000000000 {...} } ...} } } }	juce::ValueTree
-		object	{referencedObject=0x0000000000000000 <NULL> }	juce::ReferenceCountedObjectPtr<juce::ValueTree::SharedObject>
-		referencedObject	0x0000000000000000 <NULL>	juce::ValueTree::SharedObject *
+		juce::ReferenceCountedObject	<struct at NULL>	juce::ReferenceCountedObject
+		type	{name={text={data=??? } } }	const juce::Identifier
+		properties	{values={values={elements={data=??? } numAllocated=??? numUsed=??? } } }	juce::NamedValueSet
+		children	{values={elements={data=??? } numAllocated=??? numUsed=??? } }	juce::ReferenceCountedArray<juce::ValueTree::SharedObject,juce::DummyCriticalSection>
+		valueTreesWithListeners	{data={values={elements={data=??? } numAllocated=??? numUsed=??? } } }	juce::SortedSet<juce::ValueTree *,juce::DummyCriticalSection>
		parent	<Unable to read memory>	
		leakDetector574	{...}	juce::LeakedObjectDetector<juce::ValueTree::SharedObject>
-		listeners	{listeners={values={elements={data=0x0000000000000000 {???} } numAllocated=0 numUsed=0 } } }	juce::ListenerList<juce::ValueTree::Listener,juce::Array<juce::ValueTree::Listener *,juce::DummyCriticalSection,0>>
-		listeners	{values={elements={data=0x0000000000000000 {???} } numAllocated=0 numUsed=0 } }	juce::Array<juce::ValueTree::Listener *,juce::DummyCriticalSection,0>
-		values	{elements={data=0x0000000000000000 {???} } numAllocated=0 numUsed=0 }	juce::ArrayBase<juce::ValueTree::Listener *,juce::DummyCriticalSection>
		juce::DummyCriticalSection	{...}	juce::DummyCriticalSection
+		elements	{data=0x0000000000000000 {???} }	juce::HeapBlock<juce::ValueTree::Listener *,0>
		numAllocated	0	int
		numUsed	0	int

I construct the value tree in PluginProcessor.h/cpp, first with the variable:
juce::AudioProcessorValueTreeState processorValueTreeState;

and then with the initialization in the initilization list:
processorValueTreeState(*this, nullptr)

I send a pointer to that value to a function, which runs:

auto* rangedAudioParameter =
        static_cast<juce::RangedAudioParameter*>(*state.second->clientParameter);
std::unique_ptr<juce::RangedAudioParameter> parameterPtr =
        std::unique_ptr<juce::RangedAudioParameter>(rangedAudioParameter);
processorValueTreeState->createAndAddParameter(move(parameterPtr));

The parameter pointer is raw until casting it to a unique_ptr here, so I don’t see ownership issues. I have an overloaded cast for clientParameter that acts like a factory and returns the parameter object.

The parameter when added to the ValueTree seems to be well-formed as well, and looks like:

-		parameterPtr	unique_ptr {notifyAppCommand=void <lambda>(float newValue){__this=0x00000000013b4e10 {currentValue=1.00000000 defaultValue=1.00000000 ...} } ...}	std::unique_ptr<juce::RangedAudioParameter,std::default_delete<juce::RangedAudioParameter>>
-		[ptr]	0x000000000c826190 {notifyAppCommand=void <lambda>(float newValue){__this=0x00000000013b4e10 {currentValue=1.00000000 defaultValue=1.00000000 ...} } ...}	juce::RangedAudioParameter * {none::ParameterFloat}
-		[none::ParameterFloat]	{notifyAppCommand=void <lambda>(float newValue){__this=0x00000000013b4e10 {currentValue=1.00000000 defaultValue=1.00000000 ...} } ...}	none::ParameterFloat
+		juce::AudioParameterFloat	{range={start=0.00999999978 end=30.0000000 interval=0.00000000 ...} value=1.00000000 defaultValue=1.00000000 ...}	juce::AudioParameterFloat
+		notifyAppCommand	void <lambda>(float newValue){__this=0x00000000013b4e10 {currentValue=1.00000000 defaultValue=1.00000000 ...} }	std::function<void __cdecl(float)>
		minValue	0.00999999978	const float
		maxValue	30.0000000	const float
		defaultValue	1.00000000	const float
		currentValue	1.00000000	float
+		normalisableRange	{start=0.00999999978 end=30.0000000 interval=0.00000000 ...}	const juce::NormalisableRange<float>
+		parameterId	"FrequencyModSpeed"	std::string
-		juce::AudioProcessorParameterWithID	{paramID={text={data=0x000000000c8237f0 "FrequencyModSpeed" } } name={text={data=0x000000000c823320 "FrequencyModSpeed" } } ...}	juce::AudioProcessorParameterWithID
+		juce::AudioProcessorParameter	{processor=0x0000000000000000 <NULL> parameterIndex=-1 listenerLock={lock=0x000000000c8261a4  <Invalid characters in string.> } ...}	juce::AudioProcessorParameter
+		paramID	{text={data=0x000000000c8237f0 "FrequencyModSpeed" } }	const juce::String
+		name	{text={data=0x000000000c823320 "FrequencyModSpeed" } }	const juce::String
+		label	{text={data=0x00007ff98d04a118 "" } }	const juce::String
		category	genericParameter (0)	const juce::AudioProcessorParameter::Category
		leakDetector67	{...}	juce::LeakedObjectDetector<juce::AudioProcessorParameterWithID>
+		[deleter]	default_delete	std::_Compressed_pair<std::default_delete<juce::RangedAudioParameter>,juce::RangedAudioParameter *,1>
+		[Raw View]	{_Mypair=default_delete }	std::unique_ptr<juce::RangedAudioParameter,std::default_delete<juce::RangedAudioParameter>>

This won’t solve your problem, but you can write much more simply:

processorValueTreeState->createAndAddParameter ({*state.second->clientParameter});

Can you show of what type state.second->clientParameter is?

I think this method is not easy to get right. If you could create a simple static method that sets up the parameters and returns an AudioProcessorValueTreeState::ParameterLayout that would be much easier.

Cool idea! I’ll try it in a second.

Update: The code suggested wouldn’t compile. I tried making the overloaded RangedAudioParameter* cast implicit, and that didn’t solve it. I also tried the following code, which also said cannot convert argument 1 from 'initializer list' to 'std::unique_ptr<juce::...:

auto* rangedAudioParameter =
    static_cast<juce::RangedAudioParameter*>(*state.second->clientParameter);
processorValueTreeState->createAndAddParameter({ rangedAudioParameter });

The Parameter class is a special parameter class that returns a class derived from a JUCE parameter class, in this case, it returns an object named paramFloat derived from juce::AudioParameterFloat.

state.second->clientParameter, at the time of adding the parameter, is:

-		state.second->clientParameter	unique_ptr {currentValue=1.00000000 defaultValue=1.00000000 normalisableRange={start=0.00999999978 end=30.0000000 ...} }	std::unique_ptr<none::ParameterBase,std::default_delete<none::ParameterBase>>
-		[ptr]	0x000000000a9ab7d0 {currentValue=1.00000000 defaultValue=1.00000000 normalisableRange={start=0.00999999978 ...} }	none::ParameterBase * {none::Parameter<float>}
+		[none::Parameter<float>]	{currentValue=1.00000000 defaultValue=1.00000000 normalisableRange={start=0.00999999978 end=30.0000000 ...} }	none::Parameter<float>
+		__vfptr	0x00007ffdadbd4618 {NoneApp.vst3!void(* none::Parameter<float>::`vftable'[2])()} {0x00007ffdad4541c7 {NoneApp.vst3!none::Parameter<float>::`vector deleting destructor'(unsigned int)}}	void * *
		currentAttachmentId	1	unsigned __int64
+		id	"FrequencyModSpeed"	std::string
		_isRegisteredWithHost	false	bool
+		notifyAppCommand	void <lambda>(float newValue){__this=0x0000000001557430 {...} }	std::function<void __cdecl(float)>
		paramType	Float (0)	none::ParameterBase::ParamType
+		attachedSliders	{ size=1 }	std::map<unsigned __int64,juce::Slider *,std::less<unsigned __int64>,std::allocator<std::pair<unsigned __int64 const ,juce::Slider *>>>
+		attachedButtons	{ size=0 }	std::map<unsigned __int64,juce::Button *,std::less<unsigned __int64>,std::allocator<std::pair<unsigned __int64 const ,juce::Button *>>>
+		attachedComboBoxes	{ size=0 }	std::map<unsigned __int64,juce::ComboBox *,std::less<unsigned __int64>,std::allocator<std::pair<unsigned __int64 const ,juce::ComboBox *>>>
-		paramFloat	0x000000000aa12b00 {notifyAppCommand=void <lambda>(float newValue){__this=0x000000000a9ab7d0 {currentValue=1.00000000 defaultValue=1.00000000 ...} } ...}	none::ParameterFloat *
-		juce::AudioParameterFloat	{range={start=0.00999999978 end=30.0000000 interval=0.00000000 ...} value=1.00000000 defaultValue=1.00000000 ...}	juce::AudioParameterFloat
+		juce::RangedAudioParameter	{...}	juce::RangedAudioParameter
+		range	{start=0.00999999978 end=30.0000000 interval=0.00000000 ...}	juce::NormalisableRange<float>
+		value	1.00000000	std::atomic<float>
		defaultValue	1.00000000	const float
+		stringFromValueFunction	juce::String <lambda>(float v, int length){numDecimalPlacesToDisplay=7 }	std::function<juce::String __cdecl(float,int)>
+		valueFromStringFunction	float <lambda>(const juce::String & text){...}	std::function<float __cdecl(juce::String const &)>
		leakDetector113	{...}	juce::LeakedObjectDetector<juce::AudioParameterFloat>
+		notifyAppCommand	void <lambda>(float newValue){__this=0x000000000a9ab7d0 {currentValue=1.00000000 defaultValue=1.00000000 ...} }	std::function<void __cdecl(float)>
		minValue	0.00999999978	const float
		maxValue	30.0000000	const float
		defaultValue	1.00000000	const float
		currentValue	1.00000000	float
+		normalisableRange	{start=0.00999999978 end=30.0000000 interval=0.00000000 ...}	const juce::NormalisableRange<float>
+		parameterId	"FrequencyModSpeed"	std::string
+		paramInt	0x0000000000000000 <NULL>	none::ParameterInt *
+		paramBool	0x0000000000000000 <NULL>	none::ParameterBool *
+		[deleter]	default_delete	std::_Compressed_pair<std::default_delete<none::ParameterBase>,none::ParameterBase *,1>
+		[Raw View]	{_Mypair=default_delete }	std::unique_ptr<none::ParameterBase,std::default_delete<none::ParameterBase>>

The code that overloads the cast for a RangedAudioParameter* is:

        juce::RangedAudioParameter* getParamRanged() const {
            switch(paramType){
                case ParamType::Float:
                    return paramFloat;
                case ParamType::Int:
                    return paramInt;
                case ParamType::Bool:
                    return paramBool;
            }
            throw std::runtime_error("Invalid parameter type (paramType)");
        }

        explicit operator juce::RangedAudioParameter*() const {
            return getParamRanged();
        }

In this current case, it returns the property paramFloat. The Parameter<T> class has a property called paramType, and this depends on the template type given, which in this case is float for Parameter<float>, which sets paramType accordingly.

The ownership here looks a bit suspect. Where are paramFloat, paramInt etc. created? Which object is supposed to own them?

At the moment, it looks like getParamRanged() (and therefore the cast to a RangedAudioParameter) will return a pointer to an existing parameter. When the raw pointer is wrapped into a unique_ptr, there should be no other owners. Perhaps the Parameter object is also retaining ownership and the RangedAudioParameter instance is being deleted unexpectedly.

Consider the following:

  • Look up how to use Address Sanitizer on your platform, and try running your plugin with the sanitizer enabled (you may need to run as a Standalone, or run in a host that has also been built with Address Sanitizer, such as the JUCE AudioPluginHost). The sanitizer should be the first thing you try when you get segfaults or other memory-related issues. If the problem is a double-delete, the sanitizer will be able to tell you exactly where the parameter is being incorrectly deleted.
  • Update getParamRanged so that it returns a unique_ptr<RangedAudioParameter>. This may require making paramFloat, paramInt etc. into unique_ptrs too. Making this change will require you to think carefully about pointer ownership, and you might end up needing to change other parts of your design to accommodate this.
  • Unrelated to the issue at hand, but probably a worthwhile change: remove the operator juce::RangedAudioParameter* function completely and just call getParamRanged() directly instead. This will make it clearer to your readers that the operation is not a straightforward cast, and that some additional logic is involved. It will also allow you to completely avoid the ugly static_cast.

I added a new function and used that to supply processorValueTreeState->createAndAddParameter():

        std::unique_ptr<juce::RangedAudioParameter> getParamRangedUP() {
            if(!paramIsOwned){
                paramIsOwned = true;
                switch(paramType){
                    case ParamType::Float:
                        return std::unique_ptr<juce::RangedAudioParameter>(paramFloat);
                    case ParamType::Int:
                        return std::unique_ptr<juce::RangedAudioParameter>(paramInt);
                    case ParamType::Bool:
                        return std::unique_ptr<juce::RangedAudioParameter>(paramBool);
                }
                throw std::runtime_error("Invalid parameter type (paramType)");
            } else {
                throw std::runtime_error("Parameter is already owned");
            }
        }

I also used Visual Studio’s address sanitizer and tested it by trying to assign a value one past the end of an array (which it caught), and also ran the app in the JUCE AudioPluginHost, but there seems to be no errors. I wonder if something is happening to the AudioProcessorValueTreeState.

I uploaded the code online, for the full context. The aforementioned code is here on GitHub, and the code that calls that is here on GitHub.

I just tried running the NoneApp VST3 and Standalone on both macOS 10.15 and Windows 10. I don’t see the crash you mentioned on either platform. How are you triggering the crash?

There’s a commented line on line 29 in PluginProcessor.cpp, which is app.addParametersToJUCEVTS(&processorValueTreeState);. It’s was made to be an alternative to the function above it, app.addParametersToJUCE(this);, as the current one doesn’t support automation.

It’s app.addParametersToJUCEVTS(&processorValueTreeState); that results in the crash. To recreate the crash, just comment out the current addParametersToJUCE and uncomment addParametersToJUCEVTS.

I can repro the crash now. If you look at the stack trace at the point of the crash, you’ll see that it’s very long and repeats regularly. This normally means that you’ve introduced a recursive call with no base case.

Here, the problem is that FrequencySliderState::setFrequencyModExternally calls none::Parameter<float>::setValue. The SliderParameterAttachment listens to the parameter, and updates the attached slider. Then in the Slider’s valueChanged it calls FrequencySliderState::setFrequencyMod, and the cycle starts again.

The solution is to break the cycle at some point. Ideally, your UI should update without side-effects. That is, a change to the parameter value should cause the UI to repaint to display the current parameter state, but it should not generate any listener callbacks of its own. Unfortunately the SliderParameterAttachment does send listener callbacks, but it also has internal machinery to automatically ignore these. If you’re using a SliderParameterAttachment, there should be no need to manually update parameter value in response to a slider change, so you may be able to fix the problem by removing the parameter update from FrequencySlider::valueChanged.