Merge branch 'v0.20.x'

Author: MaxKellermann

NEWS

@@ -6,6 +6,10 @@ ver 0.21 (not yet released)
 ver 0.20.5 (not yet released)
 * tags
   - id3: fix memory leak on corrupt ID3 tags
+* decoder
+  - sidplay: don't require libsidutils when building with libsidplayfp
+* mixer
+  - alsa: fix crash bug
 
 ver 0.20.4 (2017/02/01)
 * input

configure.ac

@@ -988,7 +988,7 @@ AM_CONDITIONAL(ENABLE_VORBIS_DECODER, test x$enable_vorbis = xyes || test x$enab
 dnl --------------------------------- sidplay ---------------------------------
 if test x$enable_sidplay != xno; then
 	dnl Check for libsidplayfp first
-	PKG_CHECK_MODULES([SIDPLAY], [libsidplayfp libsidutils],
+	PKG_CHECK_MODULES([SIDPLAY], [libsidplayfp],
 		[found_sidplayfp=yes],
 		[found_sidplayfp=no])
 	found_sidplay=$found_sidplayfp

src/event/Call.cxx

@@ -79,7 +79,7 @@ class BlockingCallMonitor final
 void
 BlockingCall(EventLoop &loop, std::function<void()> &&f)
 {
-	if (loop.IsInside()) {
+	if (loop.IsInsideOrNull()) {
 		/* we're already inside the loop - we can simply call
 		   the function */
 		f();

src/event/Loop.cxx

@@ -222,8 +222,9 @@ EventLoop::Run()
 #ifndef NDEBUG
 	assert(busy);
 	assert(thread.IsInside());
-	thread = ThreadId::Null();
 #endif
+
+	thread = ThreadId::Null();
 }
 
 void

src/event/Loop.hxx

@@ -212,12 +212,16 @@ public:
 	}
 #endif
 
-#ifndef NDEBUG
+	/**
+	 * Like IsInside(), but also returns true if the thread has
+	 * already ended (or was not started yet).  This is useful for
+	 * code which may run during startup or shutdown, when events
+	 * are not yet/anymore handled.
+	 */
 	gcc_pure
 	bool IsInsideOrNull() const {
 		return thread.IsNull() || thread.IsInside();
 	}
-#endif
 };
 
 #endif /* MAIN_NOTIFY_H */

src/event/MultiSocketMonitor.cxx

@@ -28,12 +28,18 @@
 #endif
 
 MultiSocketMonitor::MultiSocketMonitor(EventLoop &_loop)
-	:IdleMonitor(_loop), TimeoutMonitor(_loop), ready(false) {
+	:IdleMonitor(_loop), TimeoutMonitor(_loop) {
 }
 
-MultiSocketMonitor::~MultiSocketMonitor()
+void
+MultiSocketMonitor::Reset()
 {
-	// TODO
+	assert(GetEventLoop().IsInsideOrNull());
+
+	fds.clear();
+	IdleMonitor::Cancel();
+	TimeoutMonitor::Cancel();
+	ready = refresh = false;
 }
 
 void

src/event/MultiSocketMonitor.hxx

@@ -96,7 +96,19 @@ class MultiSocketMonitor : IdleMonitor, TimeoutMonitor
 
 	friend class SingleFD;
 
-	bool ready, refresh;
+	/**
+	 * DispatchSockets() should be called.
+	 */
+	bool ready = false;
+
+	/**
+	 * PrepareSockets() should be called.
+	 *
+	 * Note that this doesn't need to be initialized by the
+	 * constructor; this class is activated with the first
+	 * InvalidateSockets() call, which initializes this flag.
+	 */
+	bool refresh;
 
 	std::forward_list<SingleFD> fds;
 
@@ -107,11 +119,26 @@ public:
 	static constexpr unsigned HANGUP = SocketMonitor::HANGUP;
 
 	MultiSocketMonitor(EventLoop &_loop);
-	~MultiSocketMonitor();
 
 	using IdleMonitor::GetEventLoop;
 
-public:
+	/**
+	 * Clear the socket list and disable all #EventLoop
+	 * registrations.  Run this in the #EventLoop thread before
+	 * destroying this object.
+	 *
+	 * Later, this object can be reused and reactivated by calling
+	 * InvalidateSockets().
+	 *
+	 * Note that this class doesn't have a destructor which calls
+	 * this method, because this would be racy and thus pointless:
+	 * at the time ~MultiSocketMonitor() is called, our virtual
+	 * methods have been morphed to be pure again, and in the
+	 * meantime the #EventLoop thread could invoke those pure
+	 * methods.
+	 */
+	void Reset();
+
 	/**
 	 * Invalidate the socket list.  A call to PrepareSockets() is
 	 * scheduled which will then update the list.
@@ -121,12 +148,19 @@ public:
 		IdleMonitor::Schedule();
 	}
 
+	/**
+	 * Add one socket to the list of monitored sockets.
+	 *
+	 * May only be called from PrepareSockets().
+	 */
 	void AddSocket(int fd, unsigned events) {
 		fds.emplace_front(*this, fd, events);
 	}
 
 	/**
 	 * Remove all sockets.
+	 *
+	 * May only be called from PrepareSockets().
 	 */
 	void ClearSocketList();
 
@@ -135,6 +169,8 @@ public:
 	 * each one; its return value is the events bit mask.  A
 	 * return value of 0 means the socket will be removed from the
 	 * list.
+	 *
+	 * May only be called from PrepareSockets().
 	 */
 	template<typename E>
 	void UpdateSocketList(E &&e) {
@@ -157,15 +193,26 @@ public:
 	/**
 	 * Replace the socket list with the given file descriptors.
 	 * The given pollfd array will be modified by this method.
+	 *
+	 * May only be called from PrepareSockets().
 	 */
 	void ReplaceSocketList(pollfd *pfds, unsigned n);
 #endif
 
 protected:
 	/**
+	 * Override this method and update the socket registrations.
+	 * To do that, call AddSocket(), ClearSocketList(),
+	 * UpdateSocketList() and ReplaceSocketList().
+	 *
 	 * @return timeout or a negative value for no timeout
 	 */
 	virtual std::chrono::steady_clock::duration PrepareSockets() = 0;
+
+	/**
+	 * At least one socket is ready or the timeout has expired.
+	 * This method should be used to perform I/O.
+	 */
 	virtual void DispatchSockets() = 0;
 
 private:

src/input/plugins/AlsaInputPlugin.cxx

@@ -99,12 +99,8 @@ class AlsaInputStream final
 	}
 
 	~AlsaInputStream() {
-		/* ClearSocketList must be called from within the
-		   IOThread; if we don't do it manually here, the
-		   ~MultiSocketMonitor() will do it in the current
-		   thread */
 		BlockingCall(MultiSocketMonitor::GetEventLoop(), [this](){
-				ClearSocketList();
+				MultiSocketMonitor::Reset();
 			});
 
 		snd_pcm_close(capture_handle);
@@ -182,7 +178,7 @@ AlsaInputStream::PrepareSockets()
 	}
 
 	int count = snd_pcm_poll_descriptors_count(capture_handle);
-	if (count < 0) {
+	if (count <= 0) {
 		ClearSocketList();
 		return std::chrono::steady_clock::duration(-1);
 	}

src/mixer/plugins/AlsaMixerPlugin.cxx

@@ -23,6 +23,7 @@
 #include "output/OutputAPI.hxx"
 #include "event/MultiSocketMonitor.hxx"
 #include "event/DeferredMonitor.hxx"
+#include "event/Call.hxx"
 #include "util/ASCII.hxx"
 #include "util/ReusableArray.hxx"
 #include "util/Domain.hxx"
@@ -53,6 +54,12 @@ class AlsaMixerMonitor final : MultiSocketMonitor, DeferredMonitor {
 		DeferredMonitor::Schedule();
 	}
 
+	~AlsaMixerMonitor() {
+		BlockingCall(MultiSocketMonitor::GetEventLoop(), [this](){
+				MultiSocketMonitor::Reset();
+			});
+	}
+
 private:
 	virtual void RunDeferred() override {
 		InvalidateSockets();
@@ -102,7 +109,7 @@ AlsaMixerMonitor::PrepareSockets()
 	}
 
 	int count = snd_mixer_poll_descriptors_count(mixer);
-	if (count < 0)
+	if (count <= 0)
 		count = 0;
 
 	struct pollfd *pfds = pfd_buffer.Get(count);