new scheme for managing port deletion

shared_ptr<Port> now uses a deleter functor which pushes Port* to a lock-free FIFO so that the Port is
always deleted (and thus unregistered with the PortEngine/backend) in a safe context w.r.t. various
callbacks in the host. Currently the auto_connect_thread in Session has been tasked with doing these
deletions.

7 files changed, 76 insertions, 12 deletions

diff --git a/libs/ardour/ardour/audioengine.h b/libs/ardour/ardour/audioengine.h
index cb5b82d058..f297e993b3 100644
--- a/libs/ardour/ardour/audioengine.h
+++ b/libs/ardour/ardour/audioengine.h
@@ -247,6 +247,8 @@ class LIBARDOUR_API AudioEngine : public PortManager, public SessionHandlePtr
 	PBD::Signal0<void> BecameSilent;
 	void reset_silence_countdown ();
 
+	void add_pending_port_deletion (Port*);
+
   private:
 	AudioEngine ();
 
diff --git a/libs/ardour/ardour/port_manager.h b/libs/ardour/ardour/port_manager.h
index 528063ca66..03484c598b 100644
--- a/libs/ardour/ardour/port_manager.h
+++ b/libs/ardour/ardour/port_manager.h
@@ -30,6 +30,7 @@
 #include <boost/shared_ptr.hpp>
 
 #include "pbd/rcu.h"
+#include "pbd/ringbuffer.h"
 
 #include "ardour/chan_count.h"
 #include "ardour/midiport_manager.h"
@@ -96,6 +97,9 @@ class LIBARDOUR_API PortManager
 	int get_ports (DataType, PortList&);
 
 	void remove_all_ports ();
+	void clear_pending_port_deletions ();
+	virtual void add_pending_port_deletion (Port*) = 0;
+	RingBuffer<Port*>& port_deletions_pending () { return _port_deletions_pending; }
 
 	/* per-Port monitoring */
 
@@ -141,6 +145,7 @@ class LIBARDOUR_API PortManager
 	boost::shared_ptr<AudioBackend> _backend;
 	SerializedRCUManager<Ports> ports;
 	bool _port_remove_in_progress;
+	RingBuffer<Port*> _port_deletions_pending;
 
 	boost::shared_ptr<Port> register_port (DataType type, const std::string& portname, bool input, bool async = false, PortFlags extra_flags = PortFlags (0));
 	void port_registration_failure (const std::string& portname);
diff --git a/libs/ardour/ardour/session.h b/libs/ardour/ardour/session.h
index 8aae4d5879..a156a4243c 100644
--- a/libs/ardour/ardour/session.h
+++ b/libs/ardour/ardour/session.h
@@ -1149,6 +1149,8 @@ class LIBARDOUR_API Session : public PBD::StatefulDestructible, public PBD::Scop
 
 	VCAManager& vca_manager() { return *_vca_manager; }
 
+	void auto_connect_thread_wakeup ();
+
   protected:
 	friend class AudioEngine;
 	void set_block_size (pframes_t nframes);
diff --git a/libs/ardour/audioengine.cc b/libs/ardour/audioengine.cc
index b8dcc3fce3..cbae19ff51 100644
--- a/libs/ardour/audioengine.cc
+++ b/libs/ardour/audioengine.cc
@@ -1481,3 +1481,19 @@ AudioEngine::set_latency_input_port (const string& name)
 {
 	_latency_input_name = name;
 }
+
+void
+AudioEngine::add_pending_port_deletion (Port* p)
+{
+	if (_session) {
+		std::cerr << "Adding " << p->name() << " to pending port deletion list\n";
+		if (_port_deletions_pending.write (&p, 1) != 1) {
+			error << string_compose (_("programming error: port %1 could not be placed on the pending deletion queue\n"), p->name()) << endmsg;
+		}
+		_session->auto_connect_thread_wakeup ();
+	} else {
+		std::cerr << "Directly delete port\n";
+		delete p;
+	}
+}
+
diff --git a/libs/ardour/midi_port.cc b/libs/ardour/midi_port.cc
index c490fb9570..f3f5378f5b 100644
--- a/libs/ardour/midi_port.cc
+++ b/libs/ardour/midi_port.cc
@@ -49,7 +49,8 @@ MidiPort::MidiPort (const std::string& name, PortFlags flags)
 MidiPort::~MidiPort()
 {
 	if (_shadow_port) {
-		_shadow_port->disconnect_all ();
+		AudioEngine::instance()->unregister_port (_shadow_port);
+		_shadow_port.reset ();
 	}
 
 	delete _buffer;
diff --git a/libs/ardour/port_manager.cc b/libs/ardour/port_manager.cc
index 408e780460..f057b2ffa2 100644
--- a/libs/ardour/port_manager.cc
+++ b/libs/ardour/port_manager.cc
@@ -47,10 +47,21 @@ using std::vector;
 PortManager::PortManager ()
 	: ports (new Ports)
 	, _port_remove_in_progress (false)
+	, _port_deletions_pending (8192) /* ick, arbitrary sizing */
 {
 }
 
 void
+PortManager::clear_pending_port_deletions ()
+{
+	Port* p;
+
+	while (_port_deletions_pending.read (&p, 1) == 1) {
+		delete p;
+	}
+}
+
+void
 PortManager::remove_all_ports ()
 {
 	/* make sure that JACK callbacks that will be invoked as we cleanup
@@ -72,6 +83,13 @@ PortManager::remove_all_ports ()
 
 	ports.flush ();
 
+	/* clear out pending port deletion list. we know this is safe because
+	 * the auto connect thread in Session is already dead when this is
+	 * done. It doesn't use shared_ptr<Port> anyway.
+	 */
+
+	_port_deletions_pending.reset ();
+
 	_port_remove_in_progress = false;
 }
 
@@ -300,6 +318,13 @@ PortManager::port_registration_failure (const std::string& portname)
 	throw PortRegistrationFailure (string_compose (_("AudioEngine: cannot register port \"%1\": %2"), portname, reason).c_str());
 }
 
+struct PortDeleter
+{
+	void operator() (Port* p) {
+		AudioEngine::instance()->add_pending_port_deletion (p);
+	}
+};
+
 boost::shared_ptr<Port>
 PortManager::register_port (DataType dtype, const string& portname, bool input, bool async, PortFlags flags)
 {
@@ -313,16 +338,19 @@ PortManager::register_port (DataType dtype, const string& portname, bool input,
 		if (dtype == DataType::AUDIO) {
 			DEBUG_TRACE (DEBUG::Ports, string_compose ("registering AUDIO port %1, input %2\n",
 								   portname, input));
-			newport.reset (new AudioPort (portname, PortFlags ((input ? IsInput : IsOutput) | flags)));
+			newport.reset (new AudioPort (portname, PortFlags ((input ? IsInput : IsOutput) | flags)),
+			               PortDeleter());
 		} else if (dtype == DataType::MIDI) {
 			if (async) {
 				DEBUG_TRACE (DEBUG::Ports, string_compose ("registering ASYNC MIDI port %1, input %2\n",
 									   portname, input));
-				newport.reset (new AsyncMIDIPort (portname, PortFlags ((input ? IsInput : IsOutput) | flags)));
+				newport.reset (new AsyncMIDIPort (portname, PortFlags ((input ? IsInput : IsOutput) | flags)),
+				               PortDeleter());
 			} else {
 				DEBUG_TRACE (DEBUG::Ports, string_compose ("registering MIDI port %1, input %2\n",
 									   portname, input));
-				newport.reset (new MidiPort (portname, PortFlags ((input ? IsInput : IsOutput) | flags)));
+				newport.reset (new MidiPort (portname, PortFlags ((input ? IsInput : IsOutput) | flags)),
+				               PortDeleter());
 			}
 		} else {
 			throw PortRegistrationFailure("unable to create port (unknown type)");
diff --git a/libs/ardour/session.cc b/libs/ardour/session.cc
index 431cadfca0..65e810f48e 100644
--- a/libs/ardour/session.cc
+++ b/libs/ardour/session.cc
@@ -6896,6 +6896,12 @@ Session::auto_connect_route (boost::shared_ptr<Route> route, bool connect_inputs
 				input_start, output_start,
 				input_offset, output_offset));
 
+	auto_connect_thread_wakeup ();
+}
+
+void
+Session::auto_connect_thread_wakeup ()
+{
 	if (pthread_mutex_trylock (&_auto_connect_mutex) == 0) {
 		pthread_cond_signal (&_auto_connect_cond);
 		pthread_mutex_unlock (&_auto_connect_mutex);
@@ -6906,10 +6912,7 @@ void
 Session::queue_latency_recompute ()
 {
 	g_atomic_int_inc (&_latency_recompute_pending);
-	if (pthread_mutex_trylock (&_auto_connect_mutex) == 0) {
-		pthread_cond_signal (&_auto_connect_cond);
-		pthread_mutex_unlock (&_auto_connect_mutex);
-	}
+	auto_connect_thread_wakeup ();
 }
 
 void
@@ -7031,10 +7034,7 @@ Session::auto_connect_thread_terminate ()
 		}
 	}
 
-	if (pthread_mutex_lock (&_auto_connect_mutex) == 0) {
-		pthread_cond_signal (&_auto_connect_cond);
-		pthread_mutex_unlock (&_auto_connect_mutex);
-	}
+	auto_connect_thread_wakeup ();
 
 	void *status;
 	pthread_join (_auto_connect_thread, &status);
@@ -7090,6 +7090,16 @@ Session::auto_connect_thread_run ()
 			}
 		}
 
+		std::cerr << "Autoconnect thread checking port deletions ...\n";
+
+		RingBuffer<Port*>& ports (AudioEngine::instance()->port_deletions_pending());
+		Port* p;
+
+		while (ports.read (&p, 1) == 1) {
+			std::cerr << "autoconnect deletes " << p->name() << std::endl;
+			delete p;
+		}
+
 		pthread_cond_wait (&_auto_connect_cond, &_auto_connect_mutex);
 	}
 	pthread_mutex_unlock (&_auto_connect_mutex);