From de819e579afe325247a8e5472dd35347ec237a65 Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Sun, 26 Apr 2020 20:25:31 +0200 Subject: [PATCH] Fix a deadlock (process_lock vs _update_latency_lock) The backend may call update_latency() while at the same time the auto-connect-port calls set_worst_io_latencies(). The latter already holds the process-lock, so update_latency() first needs to acquire it, as well. If one already holds the _update_latency_lock, one must not ask for the process-lock. --- Previously Ardour's connection manager first took the process_lock and then waited to the _update_latency_lock. Meanwhile jack calls latency_callback(), takes the _update_latency_lock and waits for the process_lock. Classic deadlock. --- libs/ardour/session.cc | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/libs/ardour/session.cc b/libs/ardour/session.cc index 9b6d027139..79638b718d 100644 --- a/libs/ardour/session.cc +++ b/libs/ardour/session.cc @@ -6560,17 +6560,17 @@ Session::update_latency (bool playback) } if (playback) { - Glib::Threads::Mutex::Lock lx (_update_latency_lock); - set_worst_output_latency (); - - /* With internal backends, AudioEngine::latency_callback () -> this method - * is called from the main_process_thread. + /* Processing needs to be blocked while re-configuring delaylines. * + * With internal backends, AudioEngine::latency_callback () -> this method + * is called from the main_process_thread (so the lock is not contended). * However jack2 can concurrently process and reconfigure port latencies. - * Processing needs to be blocked while re-configuring delaylines. */ Glib::Threads::Mutex::Lock lm (AudioEngine::instance()->process_lock ()); + /* prevent any concurrent latency updates */ + Glib::Threads::Mutex::Lock lx (_update_latency_lock); + set_worst_output_latency (); update_route_latency (true, /*apply_to_delayline*/ true); } else { @@ -6587,6 +6587,7 @@ void Session::set_worst_io_latencies () { DEBUG_TRACE (DEBUG::LatencyCompensation, "Session::set_worst_io_latencies\n"); + /* when this is called from the auto-connect thread, the process-lock is held */ Glib::Threads::Mutex::Lock lx (_update_latency_lock); set_worst_output_latency (); set_worst_input_latency (); @@ -6962,11 +6963,13 @@ Session::auto_connect_thread_run () while (g_atomic_int_get (&_ac_thread_active)) { if (!_auto_connect_queue.empty ()) { - // Why would we need the process lock ?? - // A: if ports are added while we're connecting, the backend's iterator may be invalidated: - // graph_order_callback() -> resort_routes() -> direct_feeds_according_to_reality () -> backend::connected_to() - // All ardour-internal backends use a std::vector xxxAudioBackend::find_port() - // We have control over those, but what does jack do? + /* Why would we need the process lock? + * + * A: if ports are added while connections change, + * the backend's iterator may be invalidated: + * graph_order_callback() -> resort_routes() -> direct_feeds_according_to_reality () -> backend::connected_to() + * Ardour::IO uses the process-lock to avoid concurrency, too + */ Glib::Threads::Mutex::Lock lm (AudioEngine::instance()->process_lock ()); Glib::Threads::Mutex::Lock lx (_auto_connect_queue_lock);