Fixed a crash that could occur due to ValueWithDefault::onChange calling a deleted PropertyComponent object

modules/juce_gui_basics/properties/juce_ChoicePropertyComponent.cpp

@@ -73,9 +73,9 @@ class ChoicePropertyComponent::RemapperValueSourceWithDefault    : public Value:
                                                                    private Value::Listener
 {
 public:
-    RemapperValueSourceWithDefault (ValueWithDefault& vwd, const Array<var>& map)
+    RemapperValueSourceWithDefault (ValueWithDefault* vwd, const Array<var>& map)
         : valueWithDefault (vwd),
-          sourceValue (valueWithDefault.getPropertyAsValue()),
+          sourceValue (valueWithDefault->getPropertyAsValue()),
           mappings (map)
     {
         sourceValue.addListener (this);
@@ -83,7 +83,7 @@ public:
 
     var getValue() const override
     {
-        if (valueWithDefault.isUsingDefault())
+        if (valueWithDefault->isUsingDefault())
             return -1;
 
         auto targetValue = sourceValue.getValue();
@@ -101,24 +101,24 @@ public:
 
         if (newValueInt == -1)
         {
-            valueWithDefault.resetToDefault();
+            valueWithDefault->resetToDefault();
         }
         else
         {
             auto remappedVal = mappings [newValueInt - 1];
 
             if (! remappedVal.equalsWithSameType (sourceValue))
-                valueWithDefault = remappedVal;
+                *valueWithDefault = remappedVal;
         }
     }
 
 private:
-    ValueWithDefault& valueWithDefault;
+    void valueChanged (Value&) override { sendChangeMessage (true); }
+
+    ValueWithDefault* valueWithDefault;
     Value sourceValue;
     Array<var> mappings;
 
-    void valueChanged (Value&) override    { sendChangeMessage (true); }
-
     //==============================================================================
     JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (RemapperValueSourceWithDefault)
 };
@@ -161,17 +161,19 @@ ChoicePropertyComponent::ChoicePropertyComponent (ValueWithDefault& valueToContr
                                                   const Array<var>& correspondingValues)
     : ChoicePropertyComponent (name, choiceList, correspondingValues)
 {
-    createComboBoxWithDefault (choiceList [correspondingValues.indexOf (valueToControl.getDefault())]);
+    valueWithDefault = &valueToControl;
 
-    comboBox.getSelectedIdAsValue().referTo (Value (new RemapperValueSourceWithDefault (valueToControl,
+    createComboBoxWithDefault (choiceList [correspondingValues.indexOf (valueWithDefault->getDefault())]);
+
+    comboBox.getSelectedIdAsValue().referTo (Value (new RemapperValueSourceWithDefault (valueWithDefault,
                                                                                         correspondingValues)));
 
-    valueToControl.onDefaultChange = [this, &valueToControl, choiceList, correspondingValues]
+    valueWithDefault->onDefaultChange = [this, choiceList, correspondingValues]
     {
         auto selectedId = comboBox.getSelectedId();
 
         comboBox.clear();
-        createComboBoxWithDefault (choiceList [correspondingValues.indexOf (valueToControl.getDefault())]);
+        createComboBoxWithDefault (choiceList [correspondingValues.indexOf (valueWithDefault->getDefault())]);
 
         comboBox.setSelectedId (selectedId);
     };
@@ -182,17 +184,19 @@ ChoicePropertyComponent::ChoicePropertyComponent (ValueWithDefault& valueToContr
     : PropertyComponent (name),
       choices ({ "Enabled", "Disabled" })
 {
-    createComboBoxWithDefault (valueToControl.getDefault() ? "Enabled" : "Disabled");
+    valueWithDefault = &valueToControl;
+
+    createComboBoxWithDefault (valueWithDefault->getDefault() ? "Enabled" : "Disabled");
 
-    comboBox.getSelectedIdAsValue().referTo (Value (new RemapperValueSourceWithDefault (valueToControl,
+    comboBox.getSelectedIdAsValue().referTo (Value (new RemapperValueSourceWithDefault (valueWithDefault,
                                                                                        { true, false })));
 
-    valueToControl.onDefaultChange = [this, &valueToControl]
+    valueWithDefault->onDefaultChange = [this]
     {
         auto selectedId = comboBox.getSelectedId();
 
         comboBox.clear();
-        createComboBoxWithDefault (valueToControl.getDefault() ? "Enabled" : "Disabled");
+        createComboBoxWithDefault (valueWithDefault->getDefault() ? "Enabled" : "Disabled");
 
         comboBox.setSelectedId (selectedId);
     };
@@ -200,6 +204,8 @@ ChoicePropertyComponent::ChoicePropertyComponent (ValueWithDefault& valueToContr
 
 ChoicePropertyComponent::~ChoicePropertyComponent()
 {
+    if (valueWithDefault != nullptr)
+        valueWithDefault->onDefaultChange = nullptr;
 }
 
 //==============================================================================

modules/juce_gui_basics/properties/juce_ChoicePropertyComponent.h

@@ -50,6 +50,10 @@ namespace juce
 */
 class JUCE_API  ChoicePropertyComponent    : public PropertyComponent
 {
+private:
+    /** Delegating constructor. */
+    ChoicePropertyComponent (const String&, const StringArray&, const Array<var>&);
+
 protected:
     /** Creates the component.
         Your subclass's constructor must add a list of options to the choices member variable.
@@ -79,7 +83,8 @@ public:
     /** Creates the component using a ValueWithDefault object. This will add an item to the ComboBox for the
         default value with an ID of -1.
 
-        @param valueToControl       the ValueWithDefault object that contains the Value object that the combo box will read and control
+        @param valueToControl       the ValueWithDefault object that contains the Value object that the combo box will read and control.
+                                    NB: this object must outlive the ChoicePropertyComponent.
         @param propertyName         the name of the property
         @param choices              the list of possible values that the drop-down list will contain
         @param correspondingValues  a list of values corresponding to each item in the 'choices' StringArray.
@@ -135,20 +140,23 @@ protected:
     StringArray choices;
 
 private:
-    /** Delegating constructor. */
-    ChoicePropertyComponent (const String&, const StringArray&, const Array<var>&);
-
-    ComboBox comboBox;
-    bool isCustomClass = false;
-
+    //==============================================================================
     class RemapperValueSource;
     class RemapperValueSourceWithDefault;
 
+    //==============================================================================
     void createComboBox();
     void createComboBoxWithDefault (const String&);
 
     void changeIndex();
 
+    //==============================================================================
+    ComboBox comboBox;
+    bool isCustomClass = false;
+
+    ValueWithDefault* valueWithDefault = nullptr;
+
+    //==============================================================================
     JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (ChoicePropertyComponent)
 };
 

modules/juce_gui_basics/properties/juce_MultiChoicePropertyComponent.cpp

@@ -106,10 +106,10 @@ class MultiChoicePropertyComponent::MultiChoiceRemapperSourceWithDefault    : pu
                                                                               private Value::Listener
 {
 public:
-    MultiChoiceRemapperSourceWithDefault (ValueWithDefault& vwd, var v, int c, ToggleButton* b)
+    MultiChoiceRemapperSourceWithDefault (ValueWithDefault* vwd, var v, int c, ToggleButton* b)
         : valueWithDefault (vwd),
           varToControl (v),
-          sourceValue (valueWithDefault.getPropertyAsValue()),
+          sourceValue (valueWithDefault->getPropertyAsValue()),
           maxChoices (c),
           buttonToControl (b)
     {
@@ -118,7 +118,7 @@ public:
 
     var getValue() const override
     {
-        auto v = valueWithDefault.get();
+        auto v = valueWithDefault->get();
 
         if (auto* arr = v.getArray())
         {
@@ -134,11 +134,11 @@ public:
 
     void setValue (const var& newValue) override
     {
-        auto v = valueWithDefault.get();
+        auto v = valueWithDefault->get();
 
         OptionalScopedPointer<Array<var>> arrayToControl;
 
-        if (valueWithDefault.isUsingDefault())
+        if (valueWithDefault->isUsingDefault())
             arrayToControl.set (new Array<var>(), true); // use an empty array so the default values are overwritten
         else
             arrayToControl.set (v.getArray(), false);
@@ -149,7 +149,7 @@ public:
 
             bool newState = newValue;
 
-            if (valueWithDefault.isUsingDefault())
+            if (valueWithDefault->isUsingDefault())
             {
                 if (auto* defaultArray = v.getArray())
                 {
@@ -171,33 +171,34 @@ public:
             StringComparator c;
             temp.sort (c);
 
-            valueWithDefault = temp;
+            *valueWithDefault = temp;
 
             if (temp.size() == 0)
-                valueWithDefault.resetToDefault();
+                valueWithDefault->resetToDefault();
         }
     }
 
 private:
-    ValueWithDefault& valueWithDefault;
-    var varToControl;
-    Value sourceValue;
-
-    int maxChoices;
-
-    ToggleButton* buttonToControl;
-
     //==============================================================================
-    void valueChanged (Value&) override    { sendChangeMessage (true); }
+    void valueChanged (Value&) override { sendChangeMessage (true); }
 
     void updateButtonTickColour() const noexcept
     {
-        auto alpha = valueWithDefault.isUsingDefault() ? 0.4f : 1.0f;
+        auto alpha = valueWithDefault->isUsingDefault() ? 0.4f : 1.0f;
         auto baseColour = buttonToControl->findColour (ToggleButton::tickColourId);
 
         buttonToControl->setColour (ToggleButton::tickColourId, baseColour.withAlpha (alpha));
     }
 
+    //==============================================================================
+    ValueWithDefault* valueWithDefault;
+    var varToControl;
+    Value sourceValue;
+
+    int maxChoices;
+
+    ToggleButton* buttonToControl;
+
     //==============================================================================
     JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (MultiChoiceRemapperSourceWithDefault)
 };
@@ -254,16 +255,24 @@ MultiChoicePropertyComponent::MultiChoicePropertyComponent (ValueWithDefault& va
                                                             int maxChoices)
     : MultiChoicePropertyComponent (propertyName, choices, correspondingValues)
 {
+    valueWithDefault = &valueToControl;
+
     // The value to control must be an array!
-    jassert (valueToControl.get().isArray());
+    jassert (valueWithDefault->get().isArray());
 
     for (int i = 0; i < choiceButtons.size(); ++i)
-        choiceButtons[i]->getToggleStateValue().referTo (Value (new MultiChoiceRemapperSourceWithDefault (valueToControl,
+        choiceButtons[i]->getToggleStateValue().referTo (Value (new MultiChoiceRemapperSourceWithDefault (valueWithDefault,
                                                                                                           correspondingValues[i],
                                                                                                           maxChoices,
                                                                                                           choiceButtons[i])));
 
-    valueToControl.onDefaultChange = [this] { repaint(); };
+    valueWithDefault->onDefaultChange = [this] { repaint(); };
+}
+
+MultiChoicePropertyComponent::~MultiChoicePropertyComponent()
+{
+    if (valueWithDefault != nullptr)
+        valueWithDefault->onDefaultChange = nullptr;
 }
 
 void MultiChoicePropertyComponent::paint (Graphics& g)

modules/juce_gui_basics/properties/juce_MultiChoicePropertyComponent.h

@@ -41,6 +41,10 @@ namespace juce
 */
 class MultiChoicePropertyComponent    : public PropertyComponent
 {
+private:
+    /** Delegating constructor. */
+    MultiChoicePropertyComponent (const String&, const StringArray&, const Array<var>&);
+
 public:
     /** Creates the component. Note that the underlying var object that the Value refers to must be an array.
 
@@ -62,7 +66,8 @@ public:
 
     /** Creates the component using a ValueWithDefault object. This will select the default options.
 
-        @param valueToControl       the ValueWithDefault object that contains the Value object that the ToggleButtons will read and control
+        @param valueToControl       the ValueWithDefault object that contains the Value object that the ToggleButtons will read and control.
+                                    NB: This object must outlive the MultiChoicePropertyComponent.
         @param propertyName         the name of the property
         @param choices              the list of possible values that will be represented
         @param correspondingValues  a list of values corresponding to each item in the 'choices' StringArray.
@@ -78,6 +83,8 @@ public:
                                   const Array<var>& correspondingValues,
                                   int maxChoices = -1);
 
+    ~MultiChoicePropertyComponent();
+
     //==============================================================================
     /** Returns true if the list of options is expanded. */
     bool isExpanded() const noexcept    { return expanded; }
@@ -104,8 +111,7 @@ public:
     void refresh() override {}
 
 private:
-    MultiChoicePropertyComponent (const String&, const StringArray&, const Array<var>&);
-
+    //==============================================================================
     class MultiChoiceRemapperSource;
     class MultiChoiceRemapperSourceWithDefault;
 
@@ -113,6 +119,8 @@ private:
     void lookAndFeelChanged() override;
 
     //==============================================================================
+    ValueWithDefault* valueWithDefault;
+
     int maxHeight = 0;
     int numHidden = 0;
     bool expanded = false;

modules/juce_gui_basics/properties/juce_TextPropertyComponent.cpp

@@ -124,26 +124,26 @@ private:
 class TextPropertyComponent::RemapperValueSourceWithDefault    : public Value::ValueSource
 {
 public:
-    RemapperValueSourceWithDefault (const ValueWithDefault& vwd)
+    RemapperValueSourceWithDefault (ValueWithDefault* vwd)
         : valueWithDefault (vwd)
     {
     }
 
     var getValue() const override
     {
-        return valueWithDefault.isUsingDefault() ? var() : valueWithDefault.get();
+        return valueWithDefault->isUsingDefault() ? var() : valueWithDefault->get();
     }
 
     void setValue (const var& newValue) override
     {
         if (newValue.toString().isEmpty())
-            valueWithDefault.resetToDefault();
+            valueWithDefault->resetToDefault();
         else
-            valueWithDefault = newValue;
+            *valueWithDefault = newValue;
     }
 
 private:
-    ValueWithDefault valueWithDefault;
+    ValueWithDefault* valueWithDefault;
 
     //==============================================================================
     JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (RemapperValueSourceWithDefault)
@@ -171,18 +171,22 @@ TextPropertyComponent::TextPropertyComponent (ValueWithDefault& valueToControl,
                                               int maxNumChars, bool multiLine, bool isEditable)
     : TextPropertyComponent (name, maxNumChars, multiLine, isEditable)
 {
-    textEditor->getTextValue().referTo (Value (new RemapperValueSourceWithDefault (valueToControl)));
-    textEditor->setTextToDisplayWhenEmpty (valueToControl.getDefault(), 0.5f);
+    valueWithDefault = &valueToControl;
 
-    valueToControl.onDefaultChange = [this, &valueToControl]
+    textEditor->getTextValue().referTo (Value (new RemapperValueSourceWithDefault (valueWithDefault)));
+    textEditor->setTextToDisplayWhenEmpty (valueWithDefault->getDefault(), 0.5f);
+
+    valueWithDefault->onDefaultChange = [this]
     {
-        textEditor->setTextToDisplayWhenEmpty (valueToControl.getDefault(), 0.5f);
+        textEditor->setTextToDisplayWhenEmpty (valueWithDefault->getDefault(), 0.5f);
         repaint();
     };
 }
 
 TextPropertyComponent::~TextPropertyComponent()
 {
+    if (valueWithDefault != nullptr)
+        valueWithDefault->onDefaultChange = nullptr;
 }
 
 void TextPropertyComponent::setText (const String& newText)

modules/juce_gui_basics/properties/juce_TextPropertyComponent.h

@@ -74,7 +74,8 @@ public:
 
     /** Creates a text property component with a default value.
 
-        @param valueToControl The ValueWithDefault that is controlled by the TextPropertyComponent
+        @param valueToControl The ValueWithDefault that is controlled by the TextPropertyComponent.
+                              NB: this object must outlive the TextPropertyComponent.
         @param propertyName   The name of the property
         @param maxNumChars    If not zero, then this specifies the maximum allowable length of
                               the string. If zero, then the string will have no length limit.
@@ -168,19 +169,23 @@ public:
     virtual void textWasEdited();
 
 private:
-    bool isMultiLine;
-
     class RemapperValueSourceWithDefault;
-
     class LabelComp;
     friend class LabelComp;
 
+    //==============================================================================
+    void callListeners();
+    void createEditor (int maxNumChars, bool isEditable);
+
+    //==============================================================================
+    bool isMultiLine;
+
     std::unique_ptr<LabelComp> textEditor;
     ListenerList<Listener> listenerList;
 
-    void callListeners();
-    void createEditor (int maxNumChars, bool isEditable);
+    ValueWithDefault* valueWithDefault = nullptr;
 
+    //==============================================================================
     JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (TextPropertyComponent)
 };