Fix deletion of VCA with slaved controls.
authorRobin Gareus <robin@gareus.org>
Fri, 9 Jun 2017 20:54:09 +0000 (22:54 +0200)
committerRobin Gareus <robin@gareus.org>
Fri, 9 Jun 2017 21:25:42 +0000 (23:25 +0200)
The crashed previously because:
 A VCA is-a Automatable is-a Evoral::ControlSet

 After VCA's d'tor completes ~Automatable runs and emits signal to
 DropReferences of all master-controls.

 This in turn calls SlavableAutomationControl::master_going_away()
 for slaved parameters for the given master-control

 In ::master_going_away() the weak-pointer reference to the master's
 AutomationControl (owned by the destroyed VCA) is still valid.

 Execution is in the d'tor of Automatable which is-a ControlSet and
 the ContolSet keeps a reference to the Control and hence also to the
 AutomationControl which is-a Evoral::Control.

 So master_going_away() locks a boost::shared_ptr<ARDOUR::AutomationControl>
 which is actually the MuteControl owned by the VCA.
 It calls SlavableAutomationControl::remove_master() which
 in turn calls MuteControl::pre_remove_master() which uses the
 MuteMaster API to retrieve the value. The MuteMaster however is the
 VCA that has just been destroyed.

The solution is twofold:
 1) emit "drop_references" from the VCA d'tor itself,
    before the VCA is destroyed.

 2) disconnect a slaved control from the master's drop_references signal
    when un-assigning a master-control.

libs/ardour/ardour/automation_control.h
libs/ardour/ardour/slavable_automation_control.h
libs/ardour/slavable_automation_control.cc
libs/ardour/vca.cc

index cb3b6085f4ab00570f941f519810ce5ca179a0b5..e338cb13c13b0d67705ad654118474503f1a6ca3 100644 (file)
@@ -62,7 +62,7 @@ class LIBARDOUR_API AutomationControl
                          PBD::Controllable::Flag                   flags=PBD::Controllable::Flag (0)
                );
 
-       ~AutomationControl ();
+       virtual ~AutomationControl ();
 
        boost::shared_ptr<AutomationList> alist() const {
                return boost::dynamic_pointer_cast<AutomationList>(_list);
index f89d29c0d25bc50c891fe2404c53f5a3c683753f..acb5ad747185fcbd90d89ea2034aebb8c8e89724 100644 (file)
@@ -36,7 +36,7 @@ class LIBARDOUR_API SlavableAutomationControl : public AutomationControl
                                  PBD::Controllable::Flag                   flags=PBD::Controllable::Flag (0)
                );
 
-       ~SlavableAutomationControl ();
+       virtual ~SlavableAutomationControl ();
 
        double get_value () const;
 
@@ -110,7 +110,7 @@ class LIBARDOUR_API SlavableAutomationControl : public AutomationControl
        mutable Glib::Threads::RWLock master_lock;
        typedef std::map<PBD::ID,MasterRecord> Masters;
        Masters _masters;
-       PBD::ScopedConnectionList masters_connections;
+       std::map<boost::weak_ptr<AutomationControl>, PBD::ScopedConnection> masters_connections;
 
        void   master_going_away (boost::weak_ptr<AutomationControl>);
        double get_value_locked() const;
index 40f4cb486f5ba3c92483cc95c03a9d29c4e47e84..5f24a8786fbe8e9decc2ab79ab1c9a42c56404eb 100644 (file)
@@ -222,8 +222,10 @@ SlavableAutomationControl::add_master (boost::shared_ptr<AutomationControl> m, b
                           avoiding holding a reference to the control in the binding
                           itself.
                        */
-
-                       m->DropReferences.connect_same_thread (masters_connections, boost::bind (&SlavableAutomationControl::master_going_away, this, boost::weak_ptr<AutomationControl>(m)));
+                       assert (masters_connections.find (boost::weak_ptr<AutomationControl>(m)) == masters_connections.end());
+                       PBD::ScopedConnection con;
+                       m->DropReferences.connect_same_thread (con, boost::bind (&SlavableAutomationControl::master_going_away, this, boost::weak_ptr<AutomationControl>(m)));
+                       masters_connections[boost::weak_ptr<AutomationControl>(m)] = con;
 
                        /* Store the connection inside the MasterRecord, so
                           that when we destroy it, the connection is destroyed
@@ -326,6 +328,7 @@ SlavableAutomationControl::master_going_away (boost::weak_ptr<AutomationControl>
 void
 SlavableAutomationControl::remove_master (boost::shared_ptr<AutomationControl> m)
 {
+       masters_connections.erase (boost::weak_ptr<AutomationControl>(m));
        pre_remove_master (m);
 
        {
index 3eff1a6b4554512f1274a5ef99ada88dffda9927..8ec3a55ac4aea11bf98f48b0ca3a142239e179a9 100644 (file)
@@ -91,6 +91,12 @@ VCA::init ()
 VCA::~VCA ()
 {
        DEBUG_TRACE (DEBUG::Destruction, string_compose ("delete VCA %1\n", number()));
+       {
+               Glib::Threads::Mutex::Lock lm (_control_lock);
+               for (Controls::const_iterator li = _controls.begin(); li != _controls.end(); ++li) {
+                       boost::dynamic_pointer_cast<AutomationControl>(li->second)->drop_references ();
+               }
+       }
        {
                Glib::Threads::Mutex::Lock lm (number_lock);
                if (_number == next_number - 1) {