diff options
author | Max Kellermann <max@duempel.org> | 2013-01-25 23:53:43 +0100 |
---|---|---|
committer | Max Kellermann <max@duempel.org> | 2013-01-25 23:53:43 +0100 |
commit | a9b62a2ece7f8b05bbb785edb47be8f37a19284e (patch) | |
tree | 690960bb7cc7a72b373b5182ebb9f7beff95d87c /src | |
parent | 49567f1f3eab7ac04e4c3c818212e4e3cb437e0f (diff) | |
download | mpd-a9b62a2ece7f8b05bbb785edb47be8f37a19284e.tar.gz mpd-a9b62a2ece7f8b05bbb785edb47be8f37a19284e.tar.xz mpd-a9b62a2ece7f8b05bbb785edb47be8f37a19284e.zip |
PlayerControl: add second Cond object
This fixes a deadlock bug introduced by 18076ac9. After all, the
second Cond was necessary.
The problem: two threads can wait for a signal at the same time. The
player thread waits for the output thread to finish playback. The
main thread waits for the player thread to complete a command. The
output thread finishes playback, and sends a signal, which
unfortunately does not wake up the player thread, but the main
thread. The main thread sees that the command is still not finished,
and waits again. The signal is lost forever, and MPD is deadlocked.
Diffstat (limited to 'src')
-rw-r--r-- | src/PlayerControl.cxx | 2 | ||||
-rw-r--r-- | src/PlayerControl.hxx | 32 | ||||
-rw-r--r-- | src/PlayerThread.cxx | 2 |
3 files changed, 34 insertions, 2 deletions
diff --git a/src/PlayerControl.cxx b/src/PlayerControl.cxx index 3805f776f..01a9c7e81 100644 --- a/src/PlayerControl.cxx +++ b/src/PlayerControl.cxx @@ -60,7 +60,7 @@ static void player_command_wait_locked(struct player_control *pc) { while (pc->command != PLAYER_COMMAND_NONE) - pc->cond.wait(pc->mutex); + pc->ClientWait(); } static void diff --git a/src/PlayerControl.hxx b/src/PlayerControl.hxx index 2b25177fb..de05e17ab 100644 --- a/src/PlayerControl.hxx +++ b/src/PlayerControl.hxx @@ -108,6 +108,13 @@ struct player_control { */ Cond cond; + /** + * This object gets signalled when the player thread has + * finished the #command. It wakes up the client that waits + * (i.e. the main thread). + */ + Cond client_cond; + enum player_command command; enum player_state state; @@ -191,10 +198,35 @@ struct player_control { * prior to calling this function. */ void Wait() { + assert(thread == g_thread_self()); + cond.wait(mutex); } /** + * Wake up the client waiting for command completion. + * + * Caller must lock the object. + */ + void ClientSignal() { + assert(thread == g_thread_self()); + + client_cond.signal(); + } + + /** + * The client calls this method to wait for command + * completion. + * + * Caller must lock the object. + */ + void ClientWait() { + assert(thread != g_thread_self()); + + client_cond.wait(mutex); + } + + /** * @param song the song to be queued; the given instance will * be owned and freed by the player */ diff --git a/src/PlayerThread.cxx b/src/PlayerThread.cxx index 23487953f..ac2cb84bd 100644 --- a/src/PlayerThread.cxx +++ b/src/PlayerThread.cxx @@ -147,7 +147,7 @@ player_command_finished_locked(struct player_control *pc) assert(pc->command != PLAYER_COMMAND_NONE); pc->command = PLAYER_COMMAND_NONE; - pc->cond.signal(); + pc->ClientSignal(); } static void |