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.

diff --git a/libs/ardour/ardour/audioengine.h b/libs/ardour/ardour/audioengine.h
index cb5b82d0588148bc9d785560b74fb56b0434fa45..f297e993b3c03cf30f9f2d5d3f63a3d6e16ffb41 100644 (file)
--- 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 528063ca66f03e5df809e50ee0e685605a34e23c..03484c598b9f061afe7d869b4a96eb0a4fe400fd 100644 (file)
--- 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 8aae4d5879ca7dd9518a2b87c0617bd5dd48a8b3..a156a4243c51b980744c680573dc5697f56fd57d 100644 (file)
--- 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 b8dcc3fce33b1ccff9f48de8056b8563f92a5eea..cbae19ff51847e4f0c742de4a20cc132c2fce1f7 100644 (file)
--- 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 c490fb95709286bbe064e3d71e15ed18adfdf751..f3f5378f5bf4622aade26c2231cf00fe2eaca01e 100644 (file)
--- 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 408e7804603eb73961284584529f3ee63f08fd7b..f057b2ffa2decaca65d3dde81b2d9a5048d7a675 100644 (file)
--- a/libs/ardour/port_manager.cc
+++ b/libs/ardour/port_manager.cc
@@ -47,9 +47,20 @@ 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 ()
 {
@@ -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 431cadfca0628cc0a8e2edd99f480db5295fe721..65e810f48e50ba52cfa76f69c72937dfcfa0427a 100644 (file)
--- 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);