From 4297a7b0a494c090b574fda13d3808c1ee30cefd Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Sun, 8 Jan 2017 14:28:58 +0100
Subject: [PATCH] lib/curl/Request: move exception handling out of the
 WRITEFUNCTION

libcurl's WRITEFUNCTION is pretty fragile; if we destroy the CURL*
instance or even unregister it using curl_multi_remove_handle(),
libcurl will crash instantly.  But still we need to be able to handle
exceptions from inside the WRITEFUNCTION, and call
CurlResponseHandler::OnError(), which may destroy the whole thing.  As
a workaround, I use DeferredMonitor to postpone the OnError() call
into a stack frame which is allowed to destroy the request.
---
 src/lib/curl/Request.cxx | 18 +++++++++++++++---
 src/lib/curl/Request.hxx | 17 ++++++++++++++++-
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/src/lib/curl/Request.cxx b/src/lib/curl/Request.cxx
index 8bbb27c2a..a5163dcaf 100644
--- a/src/lib/curl/Request.cxx
+++ b/src/lib/curl/Request.cxx
@@ -46,7 +46,8 @@
 
 CurlRequest::CurlRequest(CurlGlobal &_global, const char *url,
 			 CurlResponseHandler &_handler)
-	:global(_global), handler(_handler)
+	:DeferredMonitor(_global.GetEventLoop()),
+	 global(_global), handler(_handler)
 {
 	error_buffer[0] = 0;
 
@@ -226,8 +227,11 @@ CurlRequest::DataReceived(const void *ptr, size_t received_size)
 		return CURL_WRITEFUNC_PAUSE;
 	} catch (...) {
 		state = State::CLOSED;
-		handler.OnError(std::current_exception());
-		return 0;
+		/* move the CurlResponseHandler::OnError() call into a
+		   "safe" stack frame */
+		postponed_error = std::current_exception();
+		DeferredMonitor::Schedule();
+		return CURL_WRITEFUNC_PAUSE;
 	}
 
 }
@@ -243,3 +247,11 @@ CurlRequest::WriteFunction(void *ptr, size_t size, size_t nmemb, void *stream)
 
 	return c.DataReceived(ptr, size);
 }
+
+void
+CurlRequest::RunDeferred()
+{
+	assert(postponed_error);
+
+	handler.OnError(postponed_error);
+}
diff --git a/src/lib/curl/Request.hxx b/src/lib/curl/Request.hxx
index 0b0a17f11..93e254310 100644
--- a/src/lib/curl/Request.hxx
+++ b/src/lib/curl/Request.hxx
@@ -31,15 +31,17 @@
 #define CURL_REQUEST_HXX
 
 #include "Easy.hxx"
+#include "event/DeferredMonitor.hxx"
 
 #include <map>
 #include <string>
+#include <exception>
 
 struct StringView;
 class CurlGlobal;
 class CurlResponseHandler;
 
-class CurlRequest {
+class CurlRequest final : DeferredMonitor {
 	CurlGlobal &global;
 
 	CurlResponseHandler &handler;
@@ -55,6 +57,16 @@ class CurlRequest {
 
 	std::multimap<std::string, std::string> headers;
 
+	/**
+	 * An exception caught by DataReceived(), which will be
+	 * forwarded into a "safe" stack frame by
+	 * DeferredMonitor::RunDeferred().  This works around the
+	 * problem that libcurl crashes if you call
+	 * curl_multi_remove_handle() from within the WRITEFUNCTION
+	 * (i.e. DataReceived()).
+	 */
+	std::exception_ptr postponed_error;
+
 	/** error message provided by libcurl */
 	char error_buffer[CURL_ERROR_SIZE];
 
@@ -129,6 +141,9 @@ private:
 	/** called by curl when new data is available */
 	static size_t WriteFunction(void *ptr, size_t size, size_t nmemb,
 				    void *stream);
+
+	/* virtual methods from class DeferredMonitor */
+	void RunDeferred() override;
 };
 
 #endif