From a1b4f9b8abf2887b17bce05403ea7eab2421f26a Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Fri, 9 Jun 2017 22:54:09 +0200 Subject: [PATCH] Fix deletion of VCA with slaved controls. 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 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 | 2 +- libs/ardour/ardour/slavable_automation_control.h | 4 ++-- libs/ardour/slavable_automation_control.cc | 7 +++++-- libs/ardour/vca.cc | 6 ++++++ 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/libs/ardour/ardour/automation_control.h b/libs/ardour/ardour/automation_control.h index cb3b6085f4..e338cb13c1 100644 --- a/libs/ardour/ardour/automation_control.h +++ b/libs/ardour/ardour/automation_control.h @@ -62,7 +62,7 @@ class LIBARDOUR_API AutomationControl PBD::Controllable::Flag flags=PBD::Controllable::Flag (0) ); - ~AutomationControl (); + virtual ~AutomationControl (); boost::shared_ptr alist() const { return boost::dynamic_pointer_cast(_list); diff --git a/libs/ardour/ardour/slavable_automation_control.h b/libs/ardour/ardour/slavable_automation_control.h index f89d29c0d2..acb5ad7471 100644 --- a/libs/ardour/ardour/slavable_automation_control.h +++ b/libs/ardour/ardour/slavable_automation_control.h @@ -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 Masters; Masters _masters; - PBD::ScopedConnectionList masters_connections; + std::map, PBD::ScopedConnection> masters_connections; void master_going_away (boost::weak_ptr); double get_value_locked() const; diff --git a/libs/ardour/slavable_automation_control.cc b/libs/ardour/slavable_automation_control.cc index 40f4cb486f..5f24a8786f 100644 --- a/libs/ardour/slavable_automation_control.cc +++ b/libs/ardour/slavable_automation_control.cc @@ -222,8 +222,10 @@ SlavableAutomationControl::add_master (boost::shared_ptr 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(m))); + assert (masters_connections.find (boost::weak_ptr(m)) == masters_connections.end()); + PBD::ScopedConnection con; + m->DropReferences.connect_same_thread (con, boost::bind (&SlavableAutomationControl::master_going_away, this, boost::weak_ptr(m))); + masters_connections[boost::weak_ptr(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 void SlavableAutomationControl::remove_master (boost::shared_ptr m) { + masters_connections.erase (boost::weak_ptr(m)); pre_remove_master (m); { diff --git a/libs/ardour/vca.cc b/libs/ardour/vca.cc index 3eff1a6b45..8ec3a55ac4 100644 --- a/libs/ardour/vca.cc +++ b/libs/ardour/vca.cc @@ -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(li->second)->drop_references (); + } + } { Glib::Threads::Mutex::Lock lm (number_lock); if (_number == next_number - 1) { -- 2.30.2