output/pulse: use a RTTI lock guard

Make all the locks exception-safe.

Makefile.am

@@ -1457,6 +1457,7 @@ libmixer_plugins_a_SOURCES += \
 
 noinst_LIBRARIES += libpulse.a
 libpulse_a_SOURCES = \
+	src/lib/pulse/LockGuard.hxx \
 	src/lib/pulse/LogError.cxx src/lib/pulse/LogError.hxx \
 	src/lib/pulse/Error.cxx src/lib/pulse/Error.hxx \
 	src/lib/pulse/Domain.cxx src/lib/pulse/Domain.hxx

src/lib/pulse/LockGuard.hxx

@@ -0,0 +1,46 @@
+/*
+ * Copyright 2003-2016 The Music Player Daemon Project
+ * http://www.musicpd.org
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifndef MPD_PULSE_LOCK_GUARD_HXX
+#define MPD_PULSE_LOCK_GUARD_HXX
+
+#include <pulse/thread-mainloop.h>
+
+namespace Pulse {
+
+class LockGuard {
+	struct pa_threaded_mainloop *const mainloop;
+
+public:
+	explicit LockGuard(struct pa_threaded_mainloop *_mainloop)
+		:mainloop(_mainloop) {
+		pa_threaded_mainloop_lock(mainloop);
+	}
+
+	~LockGuard() {
+		pa_threaded_mainloop_unlock(mainloop);
+	}
+
+	LockGuard(const LockGuard &) = delete;
+	LockGuard &operator=(const LockGuard &) = delete;
+};
+
+};
+
+#endif

src/mixer/plugins/PulseMixerPlugin.cxx

@@ -21,6 +21,7 @@
 #include "PulseMixerPlugin.hxx"
 #include "lib/pulse/Domain.hxx"
 #include "lib/pulse/LogError.hxx"
+#include "lib/pulse/LockGuard.hxx"
 #include "mixer/MixerInternal.hxx"
 #include "mixer/Listener.hxx"
 #include "output/plugins/PulseOutputPlugin.hxx"
@@ -181,12 +182,9 @@ PulseMixer::~PulseMixer()
 int
 PulseMixer::GetVolume(gcc_unused Error &error)
 {
-	pulse_output_lock(output);
+	Pulse::LockGuard lock(pulse_output_get_mainloop(output));
 
-	int result = GetVolumeInternal(error);
+	return GetVolumeInternal(error);
-	pulse_output_unlock(output);
-
-	return result;
 }
 
 /**
@@ -203,10 +201,9 @@ PulseMixer::GetVolumeInternal(gcc_unused Error &error)
 bool
 PulseMixer::SetVolume(unsigned new_volume, Error &error)
 {
-	pulse_output_lock(output);
+	Pulse::LockGuard lock(pulse_output_get_mainloop(output));
 
 	if (!online) {
-		pulse_output_unlock(output);
 		error.Set(pulse_domain, "disconnected");
 		return false;
 	}
@@ -218,7 +215,6 @@ PulseMixer::SetVolume(unsigned new_volume, Error &error)
 	if (success)
 		volume = cvolume;
 
-	pulse_output_unlock(output);
 	return success;
 }
 

src/output/plugins/PulseOutputPlugin.cxx

@@ -22,6 +22,7 @@
 #include "lib/pulse/Domain.hxx"
 #include "lib/pulse/Error.hxx"
 #include "lib/pulse/LogError.hxx"
+#include "lib/pulse/LockGuard.hxx"
 #include "../OutputAPI.hxx"
 #include "../Wrapper.hxx"
 #include "mixer/MixerList.hxx"
@@ -75,12 +76,8 @@ public:
 
 	bool SetVolume(const pa_cvolume &volume, Error &error);
 
-	void Lock() {
+	struct pa_threaded_mainloop *GetMainloop() {
-		pa_threaded_mainloop_lock(mainloop);
+		return mainloop;
-	}
-
-	void Unlock() {
-		pa_threaded_mainloop_unlock(mainloop);
 	}
 
 	void OnContextStateChanged(pa_context_state_t new_state);
@@ -180,16 +177,10 @@ private:
 	bool StreamPause(bool pause, Error &error);
 };
 
-void
+struct pa_threaded_mainloop *
-pulse_output_lock(PulseOutput &po)
+pulse_output_get_mainloop(PulseOutput &po)
 {
-	po.Lock();
+	return po.GetMainloop();
-}
-
-void
-pulse_output_unlock(PulseOutput &po)
-{
-	po.Unlock();
 }
 
 inline void
@@ -202,7 +193,7 @@ PulseOutput::SetMixer(PulseMixer &_mixer)
 	if (mainloop == nullptr)
 		return;
 
-	pa_threaded_mainloop_lock(mainloop);
+	Pulse::LockGuard lock(mainloop);
 
 	if (context != nullptr &&
 	    pa_context_get_state(context) == PA_CONTEXT_READY) {
@@ -212,8 +203,6 @@ PulseOutput::SetMixer(PulseMixer &_mixer)
 		    pa_stream_get_state(stream) == PA_STREAM_READY)
 			pulse_mixer_on_change(_mixer, context, stream);
 	}
-
-	pa_threaded_mainloop_unlock(mainloop);
 }
 
 void
@@ -653,7 +642,7 @@ PulseOutput::Open(AudioFormat &audio_format, Error &error)
 {
 	assert(mainloop != nullptr);
 
-	pa_threaded_mainloop_lock(mainloop);
+	Pulse::LockGuard lock(mainloop);
 
 	if (context != nullptr) {
 		switch (pa_context_get_state(context)) {
@@ -674,10 +663,8 @@ PulseOutput::Open(AudioFormat &audio_format, Error &error)
 		}
 	}
 
-	if (!WaitConnection(error)) {
+	if (!WaitConnection(error))
-		pa_threaded_mainloop_unlock(mainloop);
 		return false;
-	}
 
 	/* Use the sample formats that our version of PulseAudio and MPD
 	   have in common, otherwise force MPD to send 16 bit */
@@ -708,10 +695,8 @@ PulseOutput::Open(AudioFormat &audio_format, Error &error)
 
 	/* create a stream .. */
 
-	if (!SetupStream(ss, error)) {
+	if (!SetupStream(ss, error))
-		pa_threaded_mainloop_unlock(mainloop);
 		return false;
-	}
 
 	/* .. and connect it (asynchronously) */
 
@@ -722,11 +707,9 @@ PulseOutput::Open(AudioFormat &audio_format, Error &error)
 
 		SetPulseError(error, context,
 			      "pa_stream_connect_playback() has failed");
-		pa_threaded_mainloop_unlock(mainloop);
 		return false;
 	}
 
-	pa_threaded_mainloop_unlock(mainloop);
 	return true;
 }
 
@@ -735,7 +718,7 @@ PulseOutput::Close()
 {
 	assert(mainloop != nullptr);
 
-	pa_threaded_mainloop_lock(mainloop);
+	Pulse::LockGuard lock(mainloop);
 
 	if (pa_stream_get_state(stream) == PA_STREAM_READY) {
 		pa_operation *o =
@@ -753,8 +736,6 @@ PulseOutput::Close()
 	if (context != nullptr &&
 	    pa_context_get_state(context) != PA_CONTEXT_READY)
 		DeleteContext();
-
-	pa_threaded_mainloop_unlock(mainloop);
 }
 
 bool
@@ -806,7 +787,7 @@ PulseOutput::StreamPause(bool pause, Error &error)
 inline unsigned
 PulseOutput::Delay()
 {
-	pa_threaded_mainloop_lock(mainloop);
+	Pulse::LockGuard lock(mainloop);
 
 	unsigned result = 0;
 	if (base.pause && pa_stream_is_corked(stream) &&
@@ -814,8 +795,6 @@ PulseOutput::Delay()
 		/* idle while paused */
 		result = 1000;
 
-	pa_threaded_mainloop_unlock(mainloop);
-
 	return result;
 }
 
@@ -825,29 +804,24 @@ PulseOutput::Play(const void *chunk, size_t size, Error &error)
 	assert(mainloop != nullptr);
 	assert(stream != nullptr);
 
-	pa_threaded_mainloop_lock(mainloop);
+	Pulse::LockGuard lock(mainloop);
 
 	/* check if the stream is (already) connected */
 
-	if (!WaitStream(error)) {
+	if (!WaitStream(error))
-		pa_threaded_mainloop_unlock(mainloop);
 		return 0;
-	}
 
 	assert(context != nullptr);
 
 	/* unpause if previously paused */
 
-	if (pa_stream_is_corked(stream) && !StreamPause(false, error)) {
+	if (pa_stream_is_corked(stream) && !StreamPause(false, error))
-		pa_threaded_mainloop_unlock(mainloop);
 		return 0;
-	}
 
 	/* wait until the server allows us to write */
 
 	while (writable == 0) {
 		if (pa_stream_is_suspended(stream)) {
-			pa_threaded_mainloop_unlock(mainloop);
 			error.Set(pulse_domain, "suspended");
 			return 0;
 		}
@@ -855,7 +829,6 @@ PulseOutput::Play(const void *chunk, size_t size, Error &error)
 		pa_threaded_mainloop_wait(mainloop);
 
 		if (pa_stream_get_state(stream) != PA_STREAM_READY) {
-			pa_threaded_mainloop_unlock(mainloop);
 			error.Set(pulse_domain, "disconnected");
 			return 0;
 		}
@@ -871,7 +844,6 @@ PulseOutput::Play(const void *chunk, size_t size, Error &error)
 
 	int result = pa_stream_write(stream, chunk, size, nullptr,
 				     0, PA_SEEK_RELATIVE);
-	pa_threaded_mainloop_unlock(mainloop);
 	if (result < 0) {
 		SetPulseError(error, context, "pa_stream_write() failed");
 		return 0;
@@ -886,12 +858,11 @@ PulseOutput::Cancel()
 	assert(mainloop != nullptr);
 	assert(stream != nullptr);
 
-	pa_threaded_mainloop_lock(mainloop);
+	Pulse::LockGuard lock(mainloop);
 
 	if (pa_stream_get_state(stream) != PA_STREAM_READY) {
 		/* no need to flush when the stream isn't connected
 		   yet */
-		pa_threaded_mainloop_unlock(mainloop);
 		return;
 	}
 
@@ -902,12 +873,10 @@ PulseOutput::Cancel()
 					  this);
 	if (o == nullptr) {
 		LogPulseError(context, "pa_stream_flush() has failed");
-		pa_threaded_mainloop_unlock(mainloop);
 		return;
 	}
 
 	pulse_wait_for_operation(mainloop, o);
-	pa_threaded_mainloop_unlock(mainloop);
 }
 
 inline bool
@@ -916,13 +885,12 @@ PulseOutput::Pause()
 	assert(mainloop != nullptr);
 	assert(stream != nullptr);
 
-	pa_threaded_mainloop_lock(mainloop);
+	Pulse::LockGuard lock(mainloop);
 
 	/* check if the stream is (already/still) connected */
 
 	Error error;
 	if (!WaitStream(error)) {
-		pa_threaded_mainloop_unlock(mainloop);
 		LogError(error);
 		return false;
 	}
@@ -932,12 +900,10 @@ PulseOutput::Pause()
 	/* cork the stream */
 
 	if (!pa_stream_is_corked(stream) && !StreamPause(true, error)) {
-		pa_threaded_mainloop_unlock(mainloop);
 		LogError(error);
 		return false;
 	}
 
-	pa_threaded_mainloop_unlock(mainloop);
 	return true;
 }
 

src/output/plugins/PulseOutputPlugin.hxx

@@ -27,11 +27,8 @@ class Error;
 
 extern const struct AudioOutputPlugin pulse_output_plugin;
 
-void
+struct pa_threaded_mainloop *
-pulse_output_lock(PulseOutput &po);
+pulse_output_get_mainloop(PulseOutput &po);
-
-void
-pulse_output_unlock(PulseOutput &po);
 
 void
 pulse_output_set_mixer(PulseOutput &po, PulseMixer &pm);