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<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.

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<AutomationList> alist() const {
 		return boost::dynamic_pointer_cast<AutomationList>(_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<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;
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<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);
 
 	{
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
@@ -92,6 +92,12 @@ 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) {
 			/* this was the "last" VCA added, so rewind the next number so