Commit 4dbf73d8 authored by Max Kellermann's avatar Max Kellermann

output: protect audio_output.open with the mutex

There was a deadlock between the output thread and the player thread: when the output thread failed (and closed itself) while the player thread worked with the audio_output object, MPD could crash.
parent 71cd2495
......@@ -169,6 +169,19 @@ static void audio_output_wait_all(void)
notify_wait(&audio_output_client_notify);
}
static void
audio_output_reset_reopen(struct audio_output *ao)
{
g_mutex_lock(ao->mutex);
if (!ao->open && ao->fail_timer != NULL) {
g_timer_destroy(ao->fail_timer);
ao->fail_timer = NULL;
}
g_mutex_unlock(ao->mutex);
}
/**
* Resets the "reopen" flag on all audio devices. MPD should
* immediately retry to open the device instead of waiting for the
......@@ -180,10 +193,7 @@ audio_output_all_reset_reopen(void)
for (unsigned i = 0; i < num_audio_outputs; ++i) {
struct audio_output *ao = &audio_outputs[i];
if (!ao->open && ao->fail_timer != NULL) {
g_timer_destroy(ao->fail_timer);
ao->fail_timer = NULL;
}
audio_output_reset_reopen(ao);
}
}
......@@ -289,6 +299,9 @@ static bool
chunk_is_consumed_in(const struct audio_output *ao,
const struct music_chunk *chunk)
{
if (!ao->open)
return true;
if (ao->chunk == NULL)
return false;
......@@ -312,9 +325,6 @@ chunk_is_consumed(const struct music_chunk *chunk)
const struct audio_output *ao = &audio_outputs[i];
bool consumed;
if (!ao->open)
continue;
g_mutex_lock(ao->mutex);
consumed = chunk_is_consumed_in(ao, chunk);
g_mutex_unlock(ao->mutex);
......@@ -339,14 +349,15 @@ clear_tail_chunk(G_GNUC_UNUSED const struct music_chunk *chunk, bool *locked)
for (unsigned i = 0; i < num_audio_outputs; ++i) {
struct audio_output *ao = &audio_outputs[i];
locked[i] = ao->open;
if (!locked[i])
continue;
/* this mutex will be unlocked by the caller when it's
ready */
g_mutex_lock(ao->mutex);
locked[i] = ao->open;
if (!locked[i]) {
g_mutex_unlock(ao->mutex);
continue;
}
assert(ao->chunk == chunk);
assert(ao->chunk_finished);
......
......@@ -65,6 +65,11 @@ struct audio_output {
/**
* Is the device (already) open and functional?
*
* This attribute may only be modified by the output thread.
* It is protected with #mutex: write accesses inside the
* output thread and read accesses outside of it may only be
* performed while the lock is held.
*/
bool open;
......@@ -113,7 +118,7 @@ struct audio_output {
const struct music_pipe *pipe;
/**
* This mutex protects #chunk and #chunk_finished.
* This mutex protects #open, #chunk and #chunk_finished.
*/
GMutex *mutex;
......
......@@ -48,11 +48,11 @@ ao_close(struct audio_output *ao)
g_mutex_lock(ao->mutex);
ao->chunk = NULL;
ao->open = false;
g_mutex_unlock(ao->mutex);
ao_plugin_close(ao->plugin, ao->data);
pcm_convert_deinit(&ao->convert_state);
ao->open = false;
g_debug("closed plugin=%s name=\"%s\"", ao->plugin->name, ao->name);
}
......@@ -198,8 +198,10 @@ static gpointer audio_output_task(gpointer arg)
assert(!ao->open);
if (ret) {
pcm_convert_init(&ao->convert_state);
ao->open = true;
g_mutex_lock(ao->mutex);
ao->open = true;
g_mutex_unlock(ao->mutex);
g_debug("opened plugin=%s name=\"%s\" "
"audio_format=%u:%u:%u",
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment