http: Make Cancel() actually stop an HTTPS transfer

Cancel() worked on the plain HTTP path - cancelled_ is threaded down as
progress->cancelled and checked while connecting and reading - but on the naett
path it only set a flag that nothing looked at. The transfer ran to completion
and cancelling just relabelled the result afterwards. Cancelling a store icon
that scrolled away, or a homebrew download the user gave up on, kept using the
bandwidth either way.

naett has one hook for this: a body writer that takes less than it was given
fails the request. So HTTPSRequest installs its own writer, which refuses
everything once cancelled. That works on all four backends and needs nothing
from naettClose, which is only really safe on Android.

The catch is lifetime. The writer runs on naett's transfer thread, and a request
that's still going when we're torn down would then be writing into a destroyed
HTTPSRequest - which is why the buffer it writes into is a separate refcounted
sink rather than a member. Join() on an unfinished request parks the sink where
it won't be freed and lets the request go, so a chunk that lands afterwards
writes somewhere that still exists.

That path is shutdown-only: RequestManager only cancels from its destructor, and
Update() waits for Done() before joining. Join now also notices a request that
finished while nobody was polling, and closes it properly instead of abandoning
it. What's left leaking at that point is a request still in flight as the
process exits, which is what already happened, just deliberate now and logged as
such rather than as an error.

Common/Net/HTTPNaettRequest.cpp

@@ -1,6 +1,9 @@
 #ifndef HTTPS_NOT_AVAILABLE
 
+#include <atomic>
 #include <cstring>
+#include <memory>
+#include <vector>
 
 #include "Common/Net/HTTPRequest.h"
 #include "Common/Net/HTTPNaettRequest.h"
@@ -20,6 +23,48 @@ HTTPSRequest::~HTTPSRequest() {
 	HTTPSRequest::Join();
 }
 
+// The response body, and the flag that stops it arriving. naett hands us chunks on its own
+// transfer thread, and there's no way to make it stop and be sure it has: naettClose only really
+// cancels on Android, and waiting for the others would mean blocking shutdown. So the buffer it
+// writes into is refcounted separately from the request - if we go away first, the sink stays
+// alive and a late chunk lands somewhere harmless instead of in a destroyed object.
+struct NaettBodySink {
+	Buffer buffer;
+	int length = 0;
+	// Written by us, read by the transfer thread.
+	std::atomic<bool> cancelled{false};
+};
+
+// Sinks belonging to requests that hadn't finished when they were torn down, which only happens
+// at shutdown - RequestManager cancels from its destructor. The naett objects can't be freed
+// while a callback might still be in flight, and neither can these, so both are deliberately
+// leaked. Allocated with new and never deleted, so it can't be destroyed out from under a late
+// callback during static destruction either.
+static std::vector<std::shared_ptr<NaettBodySink>> *g_abandonedSinks = new std::vector<std::shared_ptr<NaettBodySink>>();
+
+int HTTPSRequest::WriteBodyThunk(const void *source, int bytes, void *userData) {
+	NaettBodySink *sink = (NaettBodySink *)userData;
+	if (sink->cancelled) {
+		// Taking less than we were given fails the request, which is how naett lets us stop a
+		// transfer. Without this, cancelling only relabelled the result once it finished anyway.
+		return 0;
+	}
+	if (bytes <= 0) {
+		return 0;
+	}
+	char *dest = sink->buffer.Append((size_t)bytes);
+	memcpy(dest, source, bytes);
+	sink->length += bytes;
+	return bytes;
+}
+
+void HTTPSRequest::Cancel() {
+	Request::Cancel();
+	if (sink_) {
+		sink_->cancelled = true;
+	}
+}
+
 void HTTPSRequest::Start() {
 	_dbg_assert_(!req_);
 	_dbg_assert_(!res_);
@@ -41,6 +86,10 @@ void HTTPSRequest::Start() {
 	}
 	// 30 s timeout - not sure what's reasonable?
 	options.push_back(naettTimeout(30 * 1000));  // milliseconds
+	// Our own writer, so that Cancel() can actually stop a transfer rather than just relabelling
+	// it once it finishes.
+	sink_ = std::make_shared<NaettBodySink>();
+	options.push_back(naettBodyWriter(&HTTPSRequest::WriteBodyThunk, sink_.get()));
 
 	const naettOption **opts = (const naettOption **)options.data();
 	req_ = naettRequestWithOptions(url_.c_str(), (int)options.size(), opts);
@@ -52,15 +101,28 @@ void HTTPSRequest::Start() {
 void HTTPSRequest::Join() {
 	if (!res_ || !req_)
 		return;  // No pending operation.
-	// Tear down.
-	if (completed_) {
-		_dbg_assert_(req_);
+	// Tear down. A request that finished while nobody was polling Done() can still be closed
+	// properly - it's only one that's genuinely still running that can't be.
+	if (completed_ || naettComplete(res_)) {
 		naettClose(res_);
 		naettFree(req_);
 		res_ = nullptr;
 		req_ = nullptr;
+		sink_.reset();
 	} else {
-		ERROR_LOG(Log::HTTP, "HTTPSRequest::Join called before completion");
+		// Only reachable at shutdown, since RequestManager cancels from its destructor and
+		// otherwise waits for Done(). Closing a response naett is still working on isn't safe on
+		// three of the four backends, and there's nothing of ours to wait on, so let the request
+		// go and keep its sink alive - a chunk arriving after this point then writes somewhere
+		// that still exists. The process is on its way out; this is the last word on it.
+		WARN_LOG(Log::HTTP, "Abandoning an unfinished request to '%s' - shutting down", url_.c_str());
+		if (sink_) {
+			sink_->cancelled = true;
+			g_abandonedSinks->push_back(sink_);
+			sink_.reset();
+		}
+		res_ = nullptr;
+		req_ = nullptr;
 	}
 }
 
@@ -79,10 +141,11 @@ bool HTTPSRequest::Done() {
 
 	// -1000 is a code specified by us to represent cancellation, that is unlikely to ever collide with naett error codes.
 	resultCode_ = IsCancelled() ? -1000 : naettGetStatus(res_);
-	int bodyLength;
-	const void *body = naettGetBody(res_, &bodyLength);
-	char *dest = buffer_.Append(bodyLength);
-	memcpy(dest, body, bodyLength);
+	// The body arrived in the sink as it was read; take it over now that nothing else will touch it.
+	const int bodyLength = sink_ ? sink_->length : 0;
+	if (sink_ && bodyLength > 0) {
+		buffer_.Append(sink_->buffer);
+	}
 	if (resultCode_ < 0) {
 		// It's a naett error. Translate and handle.
 		switch (resultCode_) {