From 4ad525d939397933f1785a0ae8635a073c202950 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 1 Oct 2020 15:24:52 +0200 Subject: [PATCH] output/alsa: implement Interrupt() This allows canceling the blocking method LockWaitWriteAvailable(), and thus allows breaking free of misbehaving ALSA drivers, avoiding a MPD lockup. Closes https://github.com/MusicPlayerDaemon/MPD/issues/966 --- NEWS | 2 ++ src/output/plugins/AlsaOutputPlugin.cxx | 47 +++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/NEWS b/NEWS index b6238378c..8e438ec1a 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,6 @@ ver 0.22.1 (not yet released) +* output + - alsa: don't deadlock when the ALSA driver is buggy ver 0.22 (2020/09/23) * protocol diff --git a/src/output/plugins/AlsaOutputPlugin.cxx b/src/output/plugins/AlsaOutputPlugin.cxx index b41b405c5..bca78fd9f 100644 --- a/src/output/plugins/AlsaOutputPlugin.cxx +++ b/src/output/plugins/AlsaOutputPlugin.cxx @@ -25,6 +25,7 @@ #include "lib/alsa/PeriodBuffer.hxx" #include "lib/alsa/Version.hxx" #include "../OutputAPI.hxx" +#include "../Error.hxx" #include "mixer/MixerList.hxx" #include "pcm/Export.hxx" #include "system/PeriodClock.hxx" @@ -177,6 +178,15 @@ class AlsaOutput final bool drain; + /** + * Was Interrupt() called? This will unblock + * LockWaitWriteAvailable(). It will be reset by Cancel() and + * Pause(), as documented by the #AudioOutput interface. + * + * Only initialized while the output is open. + */ + bool interrupted; + /** * This buffer gets allocated after opening the ALSA device. * It contains silence samples, enough to fill one period (see @@ -237,9 +247,12 @@ private: void Open(AudioFormat &audio_format) override; void Close() noexcept override; + void Interrupt() noexcept override; + size_t Play(const void *chunk, size_t size) override; void Drain() override; void Cancel() noexcept override; + bool Pause() noexcept override; /** * Set up the snd_pcm_t object which was opened by the caller. @@ -728,6 +741,7 @@ AlsaOutput::Open(AudioFormat &audio_format) out_frame_size = pcm_export->GetOutputFrameSize(); drain = false; + interrupted = false; size_t period_size = period_frames * out_frame_size; ring_buffer = new boost::lockfree::spsc_queue(period_size * 4); @@ -741,6 +755,18 @@ AlsaOutput::Open(AudioFormat &audio_format) error = {}; } +void +AlsaOutput::Interrupt() noexcept +{ + std::unique_lock lock(mutex); + + /* the "interrupted" flag will prevent + LockWaitWriteAvailable() from actually waiting, and will + instead throw AudioOutputInterrupted */ + interrupted = true; + cond.notify_one(); +} + inline int AlsaOutput::Recover(int err) noexcept { @@ -912,6 +938,11 @@ AlsaOutput::CancelInternal() noexcept void AlsaOutput::Cancel() noexcept { + { + std::unique_lock lock(mutex); + interrupted = false; + } + if (!LockIsActive()) { /* early cancel, quick code path without thread synchronization */ @@ -928,6 +959,17 @@ AlsaOutput::Cancel() noexcept }); } +bool +AlsaOutput::Pause() noexcept +{ + std::unique_lock lock(mutex); + interrupted = false; + + /* not implemented - this override exists only to reset the + "interrupted" flag */ + return false; +} + void AlsaOutput::Close() noexcept { @@ -956,6 +998,11 @@ AlsaOutput::LockWaitWriteAvailable() if (error) std::rethrow_exception(error); + if (interrupted) + /* a CANCEL command is in flight - don't block + here */ + throw AudioOutputInterrupted{}; + size_t write_available = ring_buffer->write_available(); if (write_available >= min_available) { /* reserve room for one extra block, just in