fix processor -> reconfigure I/O || process concurrency

Add a ReaderLock to Route::process_output_buffers().

But process_output_buffers() is always called with processor-lock
held. To avoid deadlocks, a processor WriterLock must always imply
a process-lock (IFF reconfigure-I/O is called with _processor_lock).

Otherwise: e.g.
*  add_processor() -> takes processor-lock. set up and activate processor.
*  simult. audio-engine process, process-lock -> call process_output_buffers() -> wait for processor-lock
*  add_processor() continues -> calls reconfigure-io -> take process-lock -> deadlock.

diff --git a/libs/ardour/route.cc b/libs/ardour/route.cc
index 4171a9b2397cf2abdc45aee5e5e2946544247610..14a1041cba27640ae47e6ca864a1c1226c2f7a9b 100644 (file)
--- a/libs/ardour/route.cc
+++ b/libs/ardour/route.cc
@@ -180,8 +180,7 @@ Route::init ()
        Metering::Meter.connect_same_thread (*this, (boost::bind (&Route::meter, this)));
 
        {
-               /* run a configure so that the invisible processors get set up */
-               Glib::Threads::Mutex::Lock lm (AudioEngine::instance()->process_lock ());
+               Glib::Threads::Mutex::Lock lx (AudioEngine::instance()->process_lock ());
                configure_processors (0);
        }
 
@@ -420,6 +419,9 @@ Route::process_output_buffers (BufferSet& bufs,
                               framepos_t start_frame, framepos_t end_frame, pframes_t nframes,
                               int declick, bool gain_automation_ok)
 {
+       Glib::Threads::RWLock::ReaderLock lm (_processor_lock, Glib::Threads::TRY_LOCK);
+       assert(lm.locked());
+
        /* figure out if we're going to use gain automation */
        if (gain_automation_ok) {
                _amp->set_gain_automation_buffer (_session.gain_automation_buffer ());
@@ -945,9 +947,9 @@ Route::add_processor (boost::shared_ptr<Processor> processor, boost::shared_ptr<
        }
 
        {
+               Glib::Threads::Mutex::Lock lx (AudioEngine::instance()->process_lock ());
                Glib::Threads::RWLock::WriterLock lm (_processor_lock);
                ProcessorState pstate (this);
-               Glib::Threads::Mutex::Lock lx (AudioEngine::instance()->process_lock ());
 
                boost::shared_ptr<PluginInsert> pi;
                boost::shared_ptr<PortInsert> porti;
@@ -1114,9 +1116,9 @@ Route::add_processors (const ProcessorList& others, boost::shared_ptr<Processor>
        }
 
        {
+               Glib::Threads::Mutex::Lock lx (AudioEngine::instance()->process_lock ());
                Glib::Threads::RWLock::WriterLock lm (_processor_lock);
                ProcessorState pstate (this);
-               Glib::Threads::Mutex::Lock lx (AudioEngine::instance()->process_lock ());
 
                for (ProcessorList::const_iterator i = others.begin(); i != others.end(); ++i) {
 
@@ -1314,6 +1316,7 @@ Route::clear_processors (Placement p)
        }
 
        {
+               Glib::Threads::Mutex::Lock lx (AudioEngine::instance()->process_lock ());
                Glib::Threads::RWLock::WriterLock lm (_processor_lock);
                ProcessorList new_list;
                ProcessorStreams err;
@@ -1358,11 +1361,7 @@ Route::clear_processors (Placement p)
                }
 
                _processors = new_list;
-
-               {
-                       Glib::Threads::Mutex::Lock lm (AudioEngine::instance()->process_lock ());
-                       configure_processors_unlocked (&err); // this can't fail
-               }
+               configure_processors_unlocked (&err); // this can't fail
        }
 
        processor_max_streams.reset();
@@ -1378,7 +1377,7 @@ Route::clear_processors (Placement p)
 }
 
 int
-Route::remove_processor (boost::shared_ptr<Processor> processor, ProcessorStreams* err, bool need_process_lock)
+Route::remove_processor (boost::shared_ptr<Processor> processor, ProcessorStreams* err, bool)
 {
        // TODO once the export point can be configured properly, do something smarter here
        if (processor == _capturing_processor) {
@@ -1398,6 +1397,7 @@ Route::remove_processor (boost::shared_ptr<Processor> processor, ProcessorStream
        processor_max_streams.reset();
 
        {
+               Glib::Threads::Mutex::Lock lx (AudioEngine::instance()->process_lock ());
                Glib::Threads::RWLock::WriterLock lm (_processor_lock);
                ProcessorState pstate (this);
 
@@ -1438,22 +1438,11 @@ Route::remove_processor (boost::shared_ptr<Processor> processor, ProcessorStream
                        return 1;
                } 
 
-               if (need_process_lock) {
-                       Glib::Threads::Mutex::Lock lm (AudioEngine::instance()->process_lock ());
-
-                       if (configure_processors_unlocked (err)) {
-                               pstate.restore ();
-                               /* we know this will work, because it worked before :) */
-                               configure_processors_unlocked (0);
-                               return -1;
-                       }
-               } else {
-                       if (configure_processors_unlocked (err)) {
-                               pstate.restore ();
-                               /* we know this will work, because it worked before :) */
-                               configure_processors_unlocked (0);
-                               return -1;
-                       }
+               if (configure_processors_unlocked (err)) {
+                       pstate.restore ();
+                       /* we know this will work, because it worked before :) */
+                       configure_processors_unlocked (0);
+                       return -1;
                }
 
                _have_internal_generator = false;
@@ -1490,6 +1479,7 @@ Route::remove_processors (const ProcessorList& to_be_deleted, ProcessorStreams*
        processor_max_streams.reset();
 
        {
+               Glib::Threads::Mutex::Lock lx (AudioEngine::instance()->process_lock ());
                Glib::Threads::RWLock::WriterLock lm (_processor_lock);
                ProcessorState pstate (this);
 
@@ -1536,16 +1526,13 @@ Route::remove_processors (const ProcessorList& to_be_deleted, ProcessorStreams*
 
                _output->set_user_latency (0);
 
-               {
-                       Glib::Threads::Mutex::Lock lm (AudioEngine::instance()->process_lock ());
-
-                       if (configure_processors_unlocked (err)) {
-                               pstate.restore ();
-                               /* we know this will work, because it worked before :) */
-                               configure_processors_unlocked (0);
-                               return -1;
-                       }
+               if (configure_processors_unlocked (err)) {
+                       pstate.restore ();
+                       /* we know this will work, because it worked before :) */
+                       configure_processors_unlocked (0);
+                       return -1;
                }
+               //lx.unlock();
 
                _have_internal_generator = false;
 
@@ -1592,8 +1579,8 @@ Route::set_custom_panner_uri (std::string const panner_uri)
 
        /* reconfigure I/O -- re-initialize panner modules */
        {
-               Glib::Threads::RWLock::WriterLock lm (_processor_lock);
                Glib::Threads::Mutex::Lock lx (AudioEngine::instance()->process_lock ());
+               Glib::Threads::RWLock::WriterLock lm (_processor_lock);
 
                for (ProcessorList::iterator p = _processors.begin(); p != _processors.end(); ++p) {
                        boost::shared_ptr<Delivery> dl;
@@ -1815,6 +1802,7 @@ Route::reorder_processors (const ProcessorList& new_order, ProcessorStreams* err
        */
 
        {
+               Glib::Threads::Mutex::Lock lx (AudioEngine::instance()->process_lock ());
                Glib::Threads::RWLock::WriterLock lm (_processor_lock);
                ProcessorState pstate (this);
 
@@ -1876,13 +1864,9 @@ Route::reorder_processors (const ProcessorList& new_order, ProcessorStreams* err
                /* If the meter is in a custom position, find it and make a rough note of its position */
                maybe_note_meter_position ();
 
-               {
-                       Glib::Threads::Mutex::Lock lm (AudioEngine::instance()->process_lock ());
-
-                       if (configure_processors_unlocked (err)) {
-                               pstate.restore ();
-                               return -1;
-                       }
+               if (configure_processors_unlocked (err)) {
+                       pstate.restore ();
+                       return -1;
                }
        }
 
@@ -2609,11 +2593,11 @@ Route::set_processor_state (const XMLNode& node)
        }
 
        {
+               Glib::Threads::Mutex::Lock lx (AudioEngine::instance()->process_lock ());
                Glib::Threads::RWLock::WriterLock lm (_processor_lock);
                _processors = new_order;
 
                if (must_configure) {
-                       Glib::Threads::Mutex::Lock lm (AudioEngine::instance()->process_lock ());
                        configure_processors_unlocked (0);
                }
 
@@ -3194,17 +3178,14 @@ void
 Route::listen_position_changed ()
 {
        {
+               Glib::Threads::Mutex::Lock lx (AudioEngine::instance()->process_lock ());
                Glib::Threads::RWLock::WriterLock lm (_processor_lock);
                ProcessorState pstate (this);
 
-               {
-                       Glib::Threads::Mutex::Lock lm (AudioEngine::instance()->process_lock ());
-
-                       if (configure_processors_unlocked (0)) {
-                               pstate.restore ();
-                               configure_processors_unlocked (0); // it worked before we tried to add it ...
-                               return;
-                       }
+               if (configure_processors_unlocked (0)) {
+                       pstate.restore ();
+                       configure_processors_unlocked (0); // it worked before we tried to add it ...
+                       return;
                }
        }
 
@@ -3220,10 +3201,7 @@ Route::add_export_point()
                _capturing_processor.reset (new CapturingProcessor (_session));
                _capturing_processor->activate ();
 
-               {
-                       Glib::Threads::Mutex::Lock lm (AudioEngine::instance()->process_lock ());
-                       configure_processors (0);
-               }
+               configure_processors (0);
 
        }
 
@@ -4174,6 +4152,7 @@ Route::has_external_redirects () const
 boost::shared_ptr<Processor>
 Route::the_instrument () const
 {
+       Glib::Threads::Mutex::Lock lx (AudioEngine::instance()->process_lock ());
        Glib::Threads::RWLock::WriterLock lm (_processor_lock);
        return the_instrument_unlocked ();
 }
@@ -4202,6 +4181,7 @@ Route::non_realtime_locate (framepos_t pos)
        }
 
        {
+               //Glib::Threads::Mutex::Lock lx (AudioEngine::instance()->process_lock ());
                Glib::Threads::RWLock::WriterLock lm (_processor_lock);
                
                for (ProcessorList::iterator i = _processors.begin(); i != _processors.end(); ++i) {