Allow sync HTTP fetches to be interrupted to fix hanging

[grorp]

Fixes #12815, "if I try to start a game while I'm downloading a mod, Minetest hangs", "if I try to close Minetest while I'm downloading a mod, Minetest hangs", etc. This issue is a common cause of ANRs on Android.

This PR is an adoption of #12983. It works by making sync HTTP fetches interruptible if they are not on the main thread. I removed the FreeThread thing which was included in the original PR because of a suggestion to make this simpler and it still seems to work, but I have no idea what I'm doing here. Please review this carefully.

Alternative PR: #14411

Pros of this PR

• simpler
• allows keeping stuff like JSON decoding, ZIP unpacking, etc. in a worker thread without extra effort

Cons of this PR

• I don't know what I'm doing

To do

This PR is a Ready for Review.

• Decide whether this or the alternative PR is the right approach
• Fix attribution to TurkeyMcMac

How to test

Start downloading a package from ContentDB and try to close Minetest. Verify that it doesn't hang.

[sfan5]

My test code:

diff --git a/builtin/mainmenu/init.lua b/builtin/mainmenu/init.lua
index 41885e298..cbfd22b95 100644
--- a/builtin/mainmenu/init.lua
+++ b/builtin/mainmenu/init.lua
@@ -127,3 +127,10 @@ local function init_globals()
 end
 
 init_globals()
+
+core.handle_async(function()
+	local x = core.get_http_api().fetch_sync({
+		url = "https://httpbin.org/delay/10",
+	})
+	print("returned " .. dump(x))
+end, nil, function() end)
diff --git a/src/httpfetch.cpp b/src/httpfetch.cpp
index a6b86e98d..ac088f77c 100644
--- a/src/httpfetch.cpp
+++ b/src/httpfetch.cpp
@@ -747,9 +747,11 @@ static void httpfetch_request_clear(u64 caller)
 {
 	if (g_httpfetch_thread->isRunning()) {
 		Event event;
+		rawstream << "request clear + wait" << std::endl;
 		g_httpfetch_thread->requestClear(caller, &event);
 		event.wait();
 	} else {
+		rawstream << "request clear" << std::endl;
 		g_httpfetch_thread->requestClear(caller, nullptr);
 	}
 }
@@ -776,6 +778,7 @@ bool httpfetch_sync_interruptible(const HTTPFetchRequest &fetch_request,
 		httpfetch_async(req);
 		do {
 			if (thread->stopRequested()) {
+				rawstream << "interrupted" << std::endl;
 				httpfetch_caller_free(req.caller);
 				return false;
 			}
@@ -783,6 +786,7 @@ bool httpfetch_sync_interruptible(const HTTPFetchRequest &fetch_request,
 		} while (!httpfetch_async_get(req.caller, fetch_result));
 		httpfetch_caller_free(req.caller);
 	} else {
+		rawstream << "not interruptible!" << std::endl;
 		httpfetch_sync(fetch_request, fetch_result);
 	}
 	return true;

[sfan5]

works otherwise

[grorp]

Fixed attribution to TurkeyMcMac and rebased. Also added this change from the original PR that I apparently had missed: d9a52bb

[nerzhul]

as you removed http_fetch_sync, i think we can call this httpfetch_sync which is the standard and have a default value

[grorp]

Well, httpfetch_sync still exists, it's just private to httpfetch.cpp now.
I'd have to rename httpfetch_sync to something else. Do you have a suggestion?
In the end, it might be cleaner to keep the names as is.