aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMax Kellermann <max@duempel.org>2009-03-25 17:07:15 +0100
committerMax Kellermann <max@duempel.org>2009-03-25 17:07:15 +0100
commit4dbf73d88bf5ab2da154ac2f537aa28a67c5f8b6 (patch)
tree56158acb8882f2879af37ed4345dc571c272150c
parent71cd24954a34bc9fb0fdf6505616ba79b8320a5a (diff)
downloadmpd-4dbf73d88bf5ab2da154ac2f537aa28a67c5f8b6.tar.gz
mpd-4dbf73d88bf5ab2da154ac2f537aa28a67c5f8b6.tar.xz
mpd-4dbf73d88bf5ab2da154ac2f537aa28a67c5f8b6.zip
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.
-rw-r--r--src/output_all.c35
-rw-r--r--src/output_internal.h7
-rw-r--r--src/output_thread.c6
3 files changed, 33 insertions, 15 deletions
diff --git a/src/output_all.c b/src/output_all.c
index bf9202060..2e7d3a720 100644
--- a/src/output_all.c
+++ b/src/output_all.c
@@ -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);
diff --git a/src/output_internal.h b/src/output_internal.h
index 3ec0485b2..9fc2425c2 100644
--- a/src/output_internal.h
+++ b/src/output_internal.h
@@ -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;
diff --git a/src/output_thread.c b/src/output_thread.c
index b9325c073..62b893074 100644
--- a/src/output_thread.c
+++ b/src/output_thread.c
@@ -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",