From dedc4b4b101a49a21f37fb7f9132c0ec085f3dcd Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Fri, 26 Apr 2019 18:47:22 +0200
Subject: [PATCH] player/Control: pass std::unique_lock<> to Cond::wait()

---
 src/player/Control.cxx | 51 ++++++++++++++++++++++--------------------
 src/player/Control.hxx | 36 ++++++++++++++++-------------
 src/player/Thread.cxx  |  8 +++----
 3 files changed, 51 insertions(+), 44 deletions(-)

diff --git a/src/player/Control.cxx b/src/player/Control.cxx
index b2042f1e5..2ba4f29df 100644
--- a/src/player/Control.cxx
+++ b/src/player/Control.cxx
@@ -45,11 +45,12 @@ PlayerControl::~PlayerControl() noexcept
 }
 
 bool
-PlayerControl::WaitOutputConsumed(unsigned threshold) noexcept
+PlayerControl::WaitOutputConsumed(std::unique_lock<Mutex> &lock,
+				  unsigned threshold) noexcept
 {
 	bool result = outputs.CheckPipe() < threshold;
 	if (!result && command == PlayerCommand::NONE) {
-		Wait();
+		Wait(lock);
 		result = outputs.CheckPipe() < threshold;
 	}
 
@@ -64,13 +65,13 @@ PlayerControl::Play(std::unique_ptr<DetachedSong> song)
 
 	assert(song != nullptr);
 
-	const std::lock_guard<Mutex> protect(mutex);
-	SeekLocked(std::move(song), SongTime::zero());
+	std::unique_lock<Mutex> lock(mutex);
+	SeekLocked(lock, std::move(song), SongTime::zero());
 
 	if (state == PlayerState::PAUSE)
 		/* if the player was paused previously, we need to
 		   unpause it */
-		PauseLocked();
+		PauseLocked(lock);
 }
 
 void
@@ -116,10 +117,10 @@ PlayerControl::Kill() noexcept
 }
 
 void
-PlayerControl::PauseLocked() noexcept
+PlayerControl::PauseLocked(std::unique_lock<Mutex> &lock) noexcept
 {
 	if (state != PlayerState::STOP) {
-		SynchronousCommand(PlayerCommand::PAUSE);
+		SynchronousCommand(lock, PlayerCommand::PAUSE);
 		idle_add(IDLE_PLAYER);
 	}
 }
@@ -127,8 +128,8 @@ PlayerControl::PauseLocked() noexcept
 void
 PlayerControl::LockPause() noexcept
 {
-	const std::lock_guard<Mutex> protect(mutex);
-	PauseLocked();
+	std::unique_lock<Mutex> lock(mutex);
+	PauseLocked(lock);
 }
 
 void
@@ -137,7 +138,7 @@ PlayerControl::LockSetPause(bool pause_flag) noexcept
 	if (!thread.IsDefined())
 		return;
 
-	const std::lock_guard<Mutex> protect(mutex);
+	std::unique_lock<Mutex> lock(mutex);
 
 	switch (state) {
 	case PlayerState::STOP:
@@ -145,12 +146,12 @@ PlayerControl::LockSetPause(bool pause_flag) noexcept
 
 	case PlayerState::PLAY:
 		if (pause_flag)
-			PauseLocked();
+			PauseLocked(lock);
 		break;
 
 	case PlayerState::PAUSE:
 		if (!pause_flag)
-			PauseLocked();
+			PauseLocked(lock);
 		break;
 	}
 }
@@ -167,9 +168,9 @@ PlayerControl::LockGetStatus() noexcept
 {
 	PlayerStatus status;
 
-	const std::lock_guard<Mutex> protect(mutex);
+	std::unique_lock<Mutex> lock(mutex);
 	if (!occupied && thread.IsDefined())
-		SynchronousCommand(PlayerCommand::REFRESH);
+		SynchronousCommand(lock, PlayerCommand::REFRESH);
 
 	status.state = state;
 
@@ -233,23 +234,25 @@ PlayerControl::LockEnqueueSong(std::unique_ptr<DetachedSong> song) noexcept
 	assert(thread.IsDefined());
 	assert(song != nullptr);
 
-	const std::lock_guard<Mutex> protect(mutex);
-	EnqueueSongLocked(std::move(song));
+	std::unique_lock<Mutex> lock(mutex);
+	EnqueueSongLocked(lock, std::move(song));
 }
 
 void
-PlayerControl::EnqueueSongLocked(std::unique_ptr<DetachedSong> song) noexcept
+PlayerControl::EnqueueSongLocked(std::unique_lock<Mutex> &lock,
+				 std::unique_ptr<DetachedSong> song) noexcept
 {
 	assert(song != nullptr);
 	assert(next_song == nullptr);
 
 	next_song = std::move(song);
 	seek_time = SongTime::zero();
-	SynchronousCommand(PlayerCommand::QUEUE);
+	SynchronousCommand(lock, PlayerCommand::QUEUE);
 }
 
 void
-PlayerControl::SeekLocked(std::unique_ptr<DetachedSong> song, SongTime t)
+PlayerControl::SeekLocked(std::unique_lock<Mutex> &lock,
+			  std::unique_ptr<DetachedSong> song, SongTime t)
 {
 	assert(song != nullptr);
 
@@ -258,21 +261,21 @@ PlayerControl::SeekLocked(std::unique_ptr<DetachedSong> song, SongTime t)
 	/* optimization TODO: if the decoder happens to decode that
 	   song already, don't cancel that */
 	if (next_song != nullptr)
-		SynchronousCommand(PlayerCommand::CANCEL);
+		SynchronousCommand(lock, PlayerCommand::CANCEL);
 
 	assert(next_song == nullptr);
 
 	ClearError();
 	next_song = std::move(song);
 	seek_time = t;
-	SynchronousCommand(PlayerCommand::SEEK);
+	SynchronousCommand(lock, PlayerCommand::SEEK);
 
 	assert(next_song == nullptr);
 
 	/* the SEEK command is asynchronous; until completion, the
 	   "seeking" flag is set */
 	while (seeking)
-		ClientWait();
+		ClientWait(lock);
 
 	if (error_type != PlayerError::NONE) {
 		assert(error);
@@ -290,8 +293,8 @@ PlayerControl::LockSeek(std::unique_ptr<DetachedSong> song, SongTime t)
 
 	assert(song != nullptr);
 
-	const std::lock_guard<Mutex> protect(mutex);
-	SeekLocked(std::move(song), t);
+	std::unique_lock<Mutex> lock(mutex);
+	SeekLocked(lock, std::move(song), t);
 }
 
 void
diff --git a/src/player/Control.hxx b/src/player/Control.hxx
index c3f134549..51861d6ec 100644
--- a/src/player/Control.hxx
+++ b/src/player/Control.hxx
@@ -368,10 +368,10 @@ private:
 	 * valid in the player thread.  The object must be locked
 	 * prior to calling this function.
 	 */
-	void Wait() noexcept {
+	void Wait(std::unique_lock<Mutex> &lock) noexcept {
 		assert(thread.IsInside());
 
-		cond.wait(mutex);
+		cond.wait(lock);
 	}
 
 	/**
@@ -391,10 +391,10 @@ private:
 	 *
 	 * Caller must lock the object.
 	 */
-	void ClientWait() noexcept {
+	void ClientWait(std::unique_lock<Mutex> &lock) noexcept {
 		assert(!thread.IsInside());
 
-		client_cond.wait(mutex);
+		client_cond.wait(lock);
 	}
 
 	/**
@@ -426,11 +426,12 @@ private:
 	 * @param threshold the maximum number of chunks in the pipe
 	 * @return true if there are less than #threshold chunks in the pipe
 	 */
-	bool WaitOutputConsumed(unsigned threshold) noexcept;
+	bool WaitOutputConsumed(std::unique_lock<Mutex> &lock,
+				unsigned threshold) noexcept;
 
 	bool LockWaitOutputConsumed(unsigned threshold) noexcept {
-		const std::lock_guard<Mutex> protect(mutex);
-		return WaitOutputConsumed(threshold);
+		std::unique_lock<Mutex> lock(mutex);
+		return WaitOutputConsumed(lock, threshold);
 	}
 
 	/**
@@ -439,9 +440,9 @@ private:
 	 * To be called from the main thread.  Caller must lock the
 	 * object.
 	 */
-	void WaitCommandLocked() noexcept {
+	void WaitCommandLocked(std::unique_lock<Mutex> &lock) noexcept {
 		while (command != PlayerCommand::NONE)
-			ClientWait();
+			ClientWait(lock);
 	}
 
 	/**
@@ -451,12 +452,13 @@ private:
 	 * To be called from the main thread.  Caller must lock the
 	 * object.
 	 */
-	void SynchronousCommand(PlayerCommand cmd) noexcept {
+	void SynchronousCommand(std::unique_lock<Mutex> &lock,
+				PlayerCommand cmd) noexcept {
 		assert(command == PlayerCommand::NONE);
 
 		command = cmd;
 		Signal();
-		WaitCommandLocked();
+		WaitCommandLocked(lock);
 	}
 
 	/**
@@ -467,11 +469,11 @@ private:
 	 * object.
 	 */
 	void LockSynchronousCommand(PlayerCommand cmd) noexcept {
-		const std::lock_guard<Mutex> protect(mutex);
-		SynchronousCommand(cmd);
+		std::unique_lock<Mutex> lock(mutex);
+		SynchronousCommand(lock, cmd);
 	}
 
-	void PauseLocked() noexcept;
+	void PauseLocked(std::unique_lock<Mutex> &lock) noexcept;
 
 	void ClearError() noexcept {
 		error_type = PlayerError::NONE;
@@ -535,12 +537,14 @@ private:
 	 */
 	std::unique_ptr<DetachedSong> ReadTaggedSong() noexcept;
 
-	void EnqueueSongLocked(std::unique_ptr<DetachedSong> song) noexcept;
+	void EnqueueSongLocked(std::unique_lock<Mutex> &lock,
+			       std::unique_ptr<DetachedSong> song) noexcept;
 
 	/**
 	 * Throws on error.
 	 */
-	void SeekLocked(std::unique_ptr<DetachedSong> song, SongTime t);
+	void SeekLocked(std::unique_lock<Mutex> &lock,
+			std::unique_ptr<DetachedSong> song, SongTime t);
 
 	/**
 	 * Caller must lock the object.
diff --git a/src/player/Thread.cxx b/src/player/Thread.cxx
index 8d424e53e..8ef61b6b9 100644
--- a/src/player/Thread.cxx
+++ b/src/player/Thread.cxx
@@ -515,7 +515,7 @@ Player::CheckDecoderStartup(std::unique_lock<Mutex> &lock) noexcept
 		/* the decoder is ready and ok */
 
 		if (output_open &&
-		    !pc.WaitOutputConsumed(1))
+		    !pc.WaitOutputConsumed(lock, 1))
 			/* the output devices havn't finished playing
 			   all chunks yet - wait for that */
 			return true;
@@ -1037,7 +1037,7 @@ Player::Run() noexcept
 
 		if (paused) {
 			if (pc.command == PlayerCommand::NONE)
-				pc.Wait();
+				pc.Wait(lock);
 		} else if (!pipe->IsEmpty()) {
 			/* at least one music chunk is ready - send it
 			   to the audio output */
@@ -1128,7 +1128,7 @@ try {
 
 	MusicBuffer buffer(buffer_chunks);
 
-	const std::lock_guard<Mutex> lock(mutex);
+	std::unique_lock<Mutex> lock(mutex);
 
 	while (1) {
 		switch (command) {
@@ -1201,7 +1201,7 @@ try {
 			break;
 
 		case PlayerCommand::NONE:
-			Wait();
+			Wait(lock);
 			break;
 		}
 	}