Allow sync HTTP fetches to be interrupted to fix hanging (#14412)

Co-authored-by: Jude Melton-Houghton <jwmhjwmh@gmail.com>

src/gui/guiEngine.cpp

@@ -629,13 +629,15 @@ bool GUIEngine::downloadFile(const std::string &url, const std::string &target)
 	fetch_request.caller = HTTPFETCH_SYNC;
 	fetch_request.timeout = std::max(MIN_HTTPFETCH_TIMEOUT,
 		(long)g_settings->getS32("curl_file_download_timeout"));
-	httpfetch_sync(fetch_request, fetch_result);
+	bool completed = httpfetch_sync_interruptible(fetch_request, fetch_result);
 
-	if (!fetch_result.succeeded) {
+	if (!completed || !fetch_result.succeeded) {
 		target_file.close();
 		fs::DeleteSingleFileOrEmptyDirectory(target);
 		return false;
 	}
+	// TODO: directly stream the response data into the file instead of first
+	// storing the complete response in memory
 	target_file << fetch_result.data;
 
 	return true;

src/httpfetch.cpp

@@ -30,6 +30,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "exceptions.h"
 #include "debug.h"
 #include "log.h"
+#include "porting.h"
 #include "util/container.h"
 #include "util/thread.h"
 #include "version.h"
@@ -753,7 +754,7 @@ static void httpfetch_request_clear(u64 caller)
 	}
 }
 
-void httpfetch_sync(const HTTPFetchRequest &fetch_request,
+static void httpfetch_sync(const HTTPFetchRequest &fetch_request,
 		HTTPFetchResult &fetch_result)
 {
 	// Create ongoing fetch data and make a cURL handle
@@ -766,6 +767,28 @@ void httpfetch_sync(const HTTPFetchRequest &fetch_request,
 	fetch_result = *ongoing.complete(res);
 }
 
+bool httpfetch_sync_interruptible(const HTTPFetchRequest &fetch_request,
+		HTTPFetchResult &fetch_result, long interval)
+{
+	if (Thread *thread = Thread::getCurrentThread()) {
+		HTTPFetchRequest req = fetch_request;
+		req.caller = httpfetch_caller_alloc_secure();
+		httpfetch_async(req);
+		do {
+			if (thread->stopRequested()) {
+				httpfetch_caller_free(req.caller);
+				fetch_result = HTTPFetchResult(fetch_request);
+				return false;
+			}
+			sleep_ms(interval);
+		} while (!httpfetch_async_get(req.caller, fetch_result));
+		httpfetch_caller_free(req.caller);
+	} else {
+		httpfetch_sync(fetch_request, fetch_result);
+	}
+	return true;
+}
+
 #else  // USE_CURL
 
 /*
@@ -795,13 +818,14 @@ static void httpfetch_request_clear(u64 caller)
 {
 }
 
-void httpfetch_sync(const HTTPFetchRequest &fetch_request,
+bool httpfetch_sync_interruptible(const HTTPFetchRequest &fetch_request,
-		HTTPFetchResult &fetch_result)
+		HTTPFetchResult &fetch_result, long interval)
 {
-	errorstream << "httpfetch_sync: unable to fetch " << fetch_request.url
+	errorstream << "httpfetch_sync_interruptible: unable to fetch " << fetch_request.url
 			<< " because USE_CURL=0" << std::endl;
 
 	fetch_result = HTTPFetchResult(fetch_request); // sets succeeded = false etc.
+	return false;
 }
 
 #endif  // USE_CURL

src/httpfetch.h

@@ -135,6 +135,9 @@ u64 httpfetch_caller_alloc_secure();
 // to stop any ongoing fetches for the given caller.
 void httpfetch_caller_free(u64 caller);
 
-// Performs a synchronous HTTP request. This blocks and therefore should
+// Performs a synchronous HTTP request that is interruptible if the current
-// only be used from background threads.
+// thread is a Thread object. interval is the completion check interval in ms.
-void httpfetch_sync(const HTTPFetchRequest &fetch_request, HTTPFetchResult &fetch_result);
+// This blocks and therefore should only be used from background threads.
+// Returned is whether the request completed without interruption.
+bool httpfetch_sync_interruptible(const HTTPFetchRequest &fetch_request,
+		HTTPFetchResult &fetch_result, long interval = 100);

src/script/lua_api/l_http.cpp

@@ -114,9 +114,9 @@ int ModApiHttp::l_http_fetch_sync(lua_State *L)
 	infostream << "Mod performs HTTP request with URL " << req.url << std::endl;
 
 	HTTPFetchResult res;
-	httpfetch_sync(req, res);
+	bool completed = httpfetch_sync_interruptible(req, res);
 
-	push_http_fetch_result(L, res, true);
+	push_http_fetch_result(L, res, completed);
 
 	return 1;
 }

src/threading/thread.cpp

@@ -62,6 +62,9 @@ DEALINGS IN THE SOFTWARE.
 // See https://msdn.microsoft.com/en-us/library/hh920601.aspx#thread__native_handle_method
 #define win32_native_handle() ((HANDLE) getThreadHandle())
 
+thread_local Thread *current_thread = nullptr;
+
+
 Thread::Thread(const std::string &name) :
 	m_name(name),
 	m_request_stop(false),
@@ -177,6 +180,8 @@ void Thread::threadProc(Thread *thr)
 	thr->m_kernel_thread_id = thread_self();
 #endif
 
+	current_thread = thr;
+
 	thr->setName(thr->m_name);
 
 	g_logger.registerThread(thr->m_name);
@@ -197,6 +202,12 @@ void Thread::threadProc(Thread *thr)
 }
 
 
+Thread *Thread::getCurrentThread()
+{
+	return current_thread;
+}
+
+
 void Thread::setName(const std::string &name)
 {
 #if defined(__linux__)

src/threading/thread.h

@@ -119,6 +119,11 @@ public:
 	 */
 	bool setPriority(int prio);
 
+	/*
+	 * Returns the thread object of the current thread if it exists.
+	 */
+	static Thread *getCurrentThread();
+
 	/*
 	 * Sets the currently executing thread's name to where supported; useful
 	 * for debugging.