From 73013a3c044fef94a6cdc9a422f85b8c86f27c1a Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Thu, 15 Mar 2018 19:23:31 +0100
Subject: [PATCH] input/thread: move code to Stop()

Fixes crash due to "pure virtual method called" in the "mms" input
plugin.  Closes #253
---
 NEWS                                 |  2 +-
 src/input/ThreadInputStream.cxx      |  7 ++++++-
 src/input/ThreadInputStream.hxx      | 20 +++++++++++++++++++-
 src/input/plugins/MmsInputPlugin.cxx |  4 ++++
 4 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/NEWS b/NEWS
index 1a56cee5b..f422b2239 100644
--- a/NEWS
+++ b/NEWS
@@ -2,7 +2,7 @@ ver 0.20.19 (not yet released)
 * protocol
   - validate absolute seek time, reject negative values
 * input
-  - mms: fix lockup bug
+  - mms: fix lockup bug and a crash bug
 * macOS: fix crash bug
 
 ver 0.20.18 (2018/02/24)
diff --git a/src/input/ThreadInputStream.cxx b/src/input/ThreadInputStream.cxx
index a5e5e17d5..49a0b0a66 100644
--- a/src/input/ThreadInputStream.cxx
+++ b/src/input/ThreadInputStream.cxx
@@ -26,8 +26,12 @@
 #include <assert.h>
 #include <string.h>
 
-ThreadInputStream::~ThreadInputStream()
+void
+ThreadInputStream::Stop() noexcept
 {
+	if (!thread.IsDefined())
+		return;
+
 	{
 		const std::lock_guard<Mutex> lock(mutex);
 		close = true;
@@ -42,6 +46,7 @@ ThreadInputStream::~ThreadInputStream()
 		buffer->Clear();
 		HugeFree(buffer->Write().data, buffer_size);
 		delete buffer;
+		buffer = nullptr;
 	}
 }
 
diff --git a/src/input/ThreadInputStream.hxx b/src/input/ThreadInputStream.hxx
index 046e0821c..cbf378926 100644
--- a/src/input/ThreadInputStream.hxx
+++ b/src/input/ThreadInputStream.hxx
@@ -27,6 +27,7 @@
 
 #include <exception>
 
+#include <assert.h>
 #include <stdint.h>
 
 template<typename T> class CircularBuffer;
@@ -39,6 +40,11 @@ template<typename T> class CircularBuffer;
  * manages the thread and the buffer.
  *
  * This works only for "streams": unknown length, no seeking, no tags.
+ *
+ * The implementation must call Stop() before its destruction
+ * completes.  This cannot be done in ~ThreadInputStream() because at
+ * this point, the class has been morphed back to #ThreadInputStream
+ * and the still-running thread will crash due to pure method call.
  */
 class ThreadInputStream : public InputStream {
 	const char *const plugin;
@@ -76,7 +82,13 @@ public:
 		 thread(BIND_THIS_METHOD(ThreadFunc)),
 		 buffer_size(_buffer_size) {}
 
-	virtual ~ThreadInputStream();
+#ifndef NDEBUG
+	~ThreadInputStream() override {
+		/* Stop() must have been called already */
+		assert(!thread.IsDefined());
+		assert(buffer == nullptr);
+	}
+#endif
 
 	/**
 	 * Initialize the object and start the thread.
@@ -90,6 +102,12 @@ public:
 	size_t Read(void *ptr, size_t size) override final;
 
 protected:
+	/**
+	 * Stop the thread and free the buffer.  This must be called
+	 * before destruction of this object completes.
+	 */
+	void Stop() noexcept;
+
 	void SetMimeType(const char *_mime) {
 		assert(thread.IsInside());
 
diff --git a/src/input/plugins/MmsInputPlugin.cxx b/src/input/plugins/MmsInputPlugin.cxx
index c06748adf..5f420af05 100644
--- a/src/input/plugins/MmsInputPlugin.cxx
+++ b/src/input/plugins/MmsInputPlugin.cxx
@@ -39,6 +39,10 @@ public:
 				   MMS_BUFFER_SIZE) {
 	}
 
+	~MmsInputStream() noexcept override {
+		Stop();
+	}
+
 protected:
 	virtual void Open() override;
 	virtual size_t ThreadRead(void *ptr, size_t size) override;