From 6900facef2a80e4e0cc81303beb985db762b39c8 Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Wed, 2 Feb 2022 14:16:33 +0100 Subject: [PATCH] Fix potential deadlock block_processing() may hold the process-lock, waiting for the latency-lock. at the same time audio-engine may hold the latter, trying to acquire the former. --- libs/ardour/ardour/session.h | 2 +- libs/ardour/audioengine.cc | 25 ++++++++++++++++--------- libs/ardour/session.cc | 30 ++++++++++++++++++------------ 3 files changed, 35 insertions(+), 22 deletions(-) diff --git a/libs/ardour/ardour/session.h b/libs/ardour/ardour/session.h index fe2689ffa7..167419ed9a 100644 --- a/libs/ardour/ardour/session.h +++ b/libs/ardour/ardour/session.h @@ -1469,7 +1469,7 @@ private: samplecnt_t calc_preroll_subcycle (samplecnt_t) const; - void block_processing() { g_atomic_int_set (&_processing_prohibited, 1); } + void block_processing(); void unblock_processing() { g_atomic_int_set (&_processing_prohibited, 0); } bool processing_blocked() const { return g_atomic_int_get (&_processing_prohibited); } diff --git a/libs/ardour/audioengine.cc b/libs/ardour/audioengine.cc index 6e019ee7f0..3762dcb326 100644 --- a/libs/ardour/audioengine.cc +++ b/libs/ardour/audioengine.cc @@ -308,25 +308,32 @@ AudioEngine::process_callback (pframes_t nframes) lc = true; } if (lp || lc) { - Glib::Threads::Mutex::Lock ll (_latency_lock); - tm.release (); - /* re-check after releasing lock */ - if (_session->processing_blocked ()) { + /* synchronize with Session::processing_blocked + * This lock is not contended by this thread, but acts + * as barrier for Session::block_processing (Session::write_one_track). + */ + Glib::Threads::Mutex::Lock ll (_latency_lock, Glib::Threads::TRY_LOCK); + /* re-check after talking latency-lock */ + if (!ll.locked () || _session->processing_blocked ()) { if (lc) { - g_atomic_int_set (&_pending_capture_latency_callback, 1); + queue_latency_update (false); } if (lp) { - g_atomic_int_set (&_pending_playback_latency_callback, 1); + queue_latency_update (true); } } else { + /* release process-lock to call Session::update_latency */ + tm.release (); if (lc) { _session->update_latency (false); } if (lp) { _session->update_latency (true); } + /* release latency lock, **before** reacquiring process-lock */ + ll.release (); + tm.acquire (); } - tm.acquire (); } } @@ -1485,8 +1492,8 @@ AudioEngine::latency_callback (bool for_playback) * async to connect/disconnect or port creation/deletion. * All is fine. */ - Glib::Threads::Mutex::Lock ll (_latency_lock); - if (_session->processing_blocked ()) { + Glib::Threads::Mutex::Lock ll (_latency_lock, Glib::Threads::TRY_LOCK); + if (!ll.locked () || _session->processing_blocked ()) { /* Except Session::write_one_track() might just have called block_processing() */ queue_latency_update (for_playback); } else { diff --git a/libs/ardour/session.cc b/libs/ardour/session.cc index 9071d4c9da..3603b7c8c0 100644 --- a/libs/ardour/session.cc +++ b/libs/ardour/session.cc @@ -846,6 +846,21 @@ Session::destroy () BOOST_SHOW_POINTERS (); } + +void +Session::block_processing() +{ + g_atomic_int_set (&_processing_prohibited, 1); + + /* processing_blocked() is only checked at the beginning + * of the next cycle. So wait until any ongoing + * process-callback returns. + */ + Glib::Threads::Mutex::Lock lm (_engine.process_lock()); + /* latency callback may be in process, wait until it completed */ + Glib::Threads::Mutex::Lock lx (_engine.latency_lock()); +} + void Session::setup_ltc () { @@ -5752,20 +5767,11 @@ Session::write_one_track (Track& track, samplepos_t start, samplepos_t end, return result; } - // block all process callback handling - + /* block all process callback handling, so that thread-buffers + * are available here. + */ block_processing (); - { - // synchronize with AudioEngine::process_callback() - // make sure processing is not currently running - // and processing_blocked() is honored before - // acquiring thread buffers - Glib::Threads::Mutex::Lock lm (_engine.process_lock()); - /* latency callback may be in process, wait until complete */ - Glib::Threads::Mutex::Lock lx (_engine.latency_lock()); - } - _bounce_processing_active = true; /* call tree *MUST* hold route_lock */