AudioServer: Fix deadlock when playing two audio streams

Previously, AudioServer would deadlock when trying to play another audio
stream, i.e. creating a queue. The m_pending_cond condition was used
improperly, and the condition wait now happens independently of querying
the pending queue for new clients if the mixer is running.

To make the mixer's concurrency-safety code more readable, the use of
raw POSIX mutex and condition syscalls is replaced with Threading::Mutex
and Threading::ConditionVariable.
This commit is contained in:
kleines Filmröllchen 2021-09-02 20:05:21 +02:00 committed by Andreas Kling
parent c7104e7512
commit 3f067f8457
Notes: sideshowbarker 2024-07-18 04:04:41 +09:00
2 changed files with 18 additions and 15 deletions

View File

@ -6,6 +6,7 @@
*/ */
#include "Mixer.h" #include "Mixer.h"
#include "AK/Format.h"
#include <AK/Array.h> #include <AK/Array.h>
#include <AK/MemoryStream.h> #include <AK/MemoryStream.h>
#include <AK/NumericLimits.h> #include <AK/NumericLimits.h>
@ -36,9 +37,6 @@ Mixer::Mixer(NonnullRefPtr<Core::ConfigFile> config)
return; return;
} }
pthread_mutex_init(&m_pending_mutex, nullptr);
pthread_cond_init(&m_pending_cond, nullptr);
m_muted = m_config->read_bool_entry("Master", "Mute", false); m_muted = m_config->read_bool_entry("Master", "Mute", false);
m_main_volume = static_cast<double>(m_config->read_num_entry("Master", "Volume", 100)) / 100.0; m_main_volume = static_cast<double>(m_config->read_num_entry("Master", "Volume", 100)) / 100.0;
@ -52,11 +50,14 @@ Mixer::~Mixer()
NonnullRefPtr<ClientAudioStream> Mixer::create_queue(ClientConnection& client) NonnullRefPtr<ClientAudioStream> Mixer::create_queue(ClientConnection& client)
{ {
auto queue = adopt_ref(*new ClientAudioStream(client)); auto queue = adopt_ref(*new ClientAudioStream(client));
pthread_mutex_lock(&m_pending_mutex); m_pending_mutex.lock();
m_pending_mixing.append(*queue); m_pending_mixing.append(*queue);
m_added_queue = true;
pthread_cond_signal(&m_pending_cond); m_pending_mutex.unlock();
pthread_mutex_unlock(&m_pending_mutex); // Signal the mixer thread to start back up, in case nobody was connected before.
m_mixing_necessary.signal();
return queue; return queue;
} }
@ -65,13 +66,14 @@ void Mixer::mix()
decltype(m_pending_mixing) active_mix_queues; decltype(m_pending_mixing) active_mix_queues;
for (;;) { for (;;) {
if (active_mix_queues.is_empty() || m_added_queue) { m_pending_mutex.lock();
pthread_mutex_lock(&m_pending_mutex); // While we have nothing to mix, wait on the condition.
pthread_cond_wait(&m_pending_cond, &m_pending_mutex); m_mixing_necessary.wait_while([this, &active_mix_queues]() { return m_pending_mixing.is_empty() && active_mix_queues.is_empty(); });
if (!m_pending_mixing.is_empty()) {
active_mix_queues.extend(move(m_pending_mixing)); active_mix_queues.extend(move(m_pending_mixing));
pthread_mutex_unlock(&m_pending_mutex); m_pending_mixing.clear();
m_added_queue = false;
} }
m_pending_mutex.unlock();
active_mix_queues.remove_all_matching([&](auto& entry) { return !entry->client(); }); active_mix_queues.remove_all_matching([&](auto& entry) { return !entry->client(); });

View File

@ -19,8 +19,10 @@
#include <LibAudio/Buffer.h> #include <LibAudio/Buffer.h>
#include <LibCore/File.h> #include <LibCore/File.h>
#include <LibCore/Timer.h> #include <LibCore/Timer.h>
#include <LibThreading/ConditionVariable.h>
#include <LibThreading/Mutex.h> #include <LibThreading/Mutex.h>
#include <LibThreading/Thread.h> #include <LibThreading/Thread.h>
#include <sys/types.h>
namespace AudioServer { namespace AudioServer {
@ -125,9 +127,8 @@ private:
void request_setting_sync(); void request_setting_sync();
Vector<NonnullRefPtr<ClientAudioStream>> m_pending_mixing; Vector<NonnullRefPtr<ClientAudioStream>> m_pending_mixing;
Atomic<bool> m_added_queue { false }; Threading::Mutex m_pending_mutex;
pthread_mutex_t m_pending_mutex; Threading::ConditionVariable m_mixing_necessary { m_pending_mutex };
pthread_cond_t m_pending_cond;
RefPtr<Core::File> m_device; RefPtr<Core::File> m_device;