From a31ce78ff4272a1da5c3e95650502b93dbe65c7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 5 Sep 2026 13:14:55 -0600 Subject: [PATCH 1/5] naett: Honour the body writer's return on Apple and Android A writer that takes less than it was given is failing the request. Windows and Linux already treat it that way, and it's how the default writer reports that it couldn't grow its buffer - but the Apple and Android backends threw the result away, so a short write silently truncated the body and still reported success. That also means it's how a caller stops a transfer in progress, which is what the next commit uses, so document what the return value means while we're here. Apple cancels the data task as well, or the chunks just keep coming. --- ext/naett-lib/README-ppsspp.md | 7 +++++++ ext/naett-lib/naett.h | 5 +++++ ext/naett-lib/src/naett_android.c | 9 ++++++++- ext/naett-lib/src/naett_osx.c | 13 ++++++++++++- 4 files changed, 32 insertions(+), 2 deletions(-) diff --git a/ext/naett-lib/README-ppsspp.md b/ext/naett-lib/README-ppsspp.md index 32bd5fa89b..5f56e0f1a6 100644 --- a/ext/naett-lib/README-ppsspp.md +++ b/ext/naett-lib/README-ppsspp.md @@ -100,3 +100,10 @@ Keep this list up to date - it's what makes it possible to move to a newer upstr - `src/naett_osx.c`: `invalidateAndCancel` returns before the session lets go of its delegate, so the delegate's back pointer to the response is cleared first, and `didReceiveData` checks it (as `didCompleteWithError` already did). +- `src/naett_osx.c` / `src/naett_android.c`: both threw away the body writer's return value. + Windows and Linux already treat a short write as a failed request - it's how the default + writer reports it couldn't grow, and how we cancel a transfer - so on those two a short write + silently truncated the body and still looked like a success. Apple also cancels the task, or + the data just keeps arriving. +- `naett.h`: documented what the body writer's return value means, since three of the four + backends' behaviour depends on it. diff --git a/ext/naett-lib/naett.h b/ext/naett-lib/naett.h index d49614d0b1..67716a0626 100644 --- a/ext/naett-lib/naett.h +++ b/ext/naett-lib/naett.h @@ -41,6 +41,11 @@ naettOption* naettBody(const char* body, int size); // Sets a request body reader. naettOption* naettBodyReader(naettReadFunc reader, void* userData); // Sets a response body writer. +// +// PPSSPP: the writer is handed each chunk as it arrives, on whichever thread the backend runs +// its transfer on, and must return the number of bytes it took. Returning anything else fails +// the request with naettReadError and stops the transfer - which is both how the default writer +// reports that it couldn't grow its buffer, and how a caller cancels one in progress. naettOption* naettBodyWriter(naettWriteFunc writer, void* userData); // Sets connection timeout in milliseconds. naettOption* naettTimeout(int milliSeconds); diff --git a/ext/naett-lib/src/naett_android.c b/ext/naett-lib/src/naett_android.c index 4b79fa6012..3d433200db 100644 --- a/ext/naett-lib/src/naett_android.c +++ b/ext/naett-lib/src/naett_android.c @@ -297,7 +297,14 @@ static void* processRequest(void* data) { break; } else if (bytesRead > 0) { (*env)->GetByteArrayRegion(env, buffer, 0, bytesRead, (jbyte*) byteBuffer); - req->options.bodyWriter(byteBuffer, bytesRead, req->options.bodyWriterData); + // PPSSPP: a short write from the body writer fails the request - it's how a caller + // aborts, and how the default writer reports that it couldn't grow. Upstream ignored + // it, so the body was silently truncated and still reported as a success. + int written = req->options.bodyWriter(byteBuffer, bytesRead, req->options.bodyWriterData); + if (written != bytesRead) { + res->code = naettReadError; + goto finally; + } res->totalBytesRead += bytesRead; } } while (!res->closeRequested); diff --git a/ext/naett-lib/src/naett_osx.c b/ext/naett-lib/src/naett_osx.c index 1692b0c09e..4befc79e20 100644 --- a/ext/naett-lib/src/naett_osx.c +++ b/ext/naett-lib/src/naett_osx.c @@ -153,7 +153,18 @@ void didReceiveData(id self, SEL _sel, id session, id dataTask, id data) { const void* bytes = objc_msgSend_t(const void*)(data, sel("bytes")); NSUInteger length = objc_msgSend_t(NSUInteger)(data, sel("length")); - res->request->options.bodyWriter(bytes, (int)length, res->request->options.bodyWriterData); + // PPSSPP: a writer that doesn't take everything is failing the request - that's how a caller + // aborts a transfer, and how the default writer reports that it couldn't grow. Upstream threw + // the result away here, so a short write silently truncated the body and still looked like a + // success. Cancel the task too, or the data just keeps coming. + int written = res->request->options.bodyWriter(bytes, (int)length, res->request->options.bodyWriterData); + if (written != (int)length) { + res->code = naettReadError; + res->complete = 1; + objc_msgSend_void(dataTask, sel("cancel")); + release(p); + return; + } res->totalBytesRead += (int)length; release(p); From b120f0004f97205e4f2c389a07e459bbbe626e45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 5 Sep 2026 13:15:12 -0600 Subject: [PATCH 2/5] 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 | 79 +++++++++++++++++++++++++++++---- Common/Net/HTTPNaettRequest.h | 11 +++++ Common/Net/HTTPRequest.h | 4 +- 3 files changed, 85 insertions(+), 9 deletions(-) diff --git a/Common/Net/HTTPNaettRequest.cpp b/Common/Net/HTTPNaettRequest.cpp index f84accc98e..5d61427bd0 100644 --- a/Common/Net/HTTPNaettRequest.cpp +++ b/Common/Net/HTTPNaettRequest.cpp @@ -1,6 +1,9 @@ #ifndef HTTPS_NOT_AVAILABLE +#include #include +#include +#include #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 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> *g_abandonedSinks = new std::vector>(); + +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(); + 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_) { diff --git a/Common/Net/HTTPNaettRequest.h b/Common/Net/HTTPNaettRequest.h index 9d1a4d95b0..7be0527e90 100644 --- a/Common/Net/HTTPNaettRequest.h +++ b/Common/Net/HTTPNaettRequest.h @@ -24,12 +24,23 @@ public: bool Done() override; bool Failed() const override { return failed_; } + // Cancelling has to reach naett, or it only changes the code we report once the transfer ends + // on its own. See the .cpp. + void Cancel() override; + private: + static int WriteBodyThunk(const void *source, int bytes, void *userData); + std::string postData_; std::string postMime_; bool completed_ = false; bool failed_ = false; + // Where the response body lands. Deliberately not a member of this object: naett writes into + // it from its own transfer thread, and that can outlive us if we're torn down before the + // request finishes. See NaettBodySink in the .cpp. + std::shared_ptr sink_; + // Naett state naettReq *req_ = nullptr; naettRes *res_ = nullptr; diff --git a/Common/Net/HTTPRequest.h b/Common/Net/HTTPRequest.h index 29991b49ac..0cd8db3288 100644 --- a/Common/Net/HTTPRequest.h +++ b/Common/Net/HTTPRequest.h @@ -75,7 +75,9 @@ public: flags_ |= flag; } - void Cancel() { cancelled_ = true; } + // Virtual so a backend can act on it. The HTTPS one has to tell naett, which is what actually + // stops a transfer in progress - see HTTPSRequest::Cancel. + virtual void Cancel() { cancelled_ = true; } bool IsCancelled() const { return cancelled_; } // If not downloading to a file, access this to get the result. From bc095c5e311a370bf7152a8589eb9e24b4c1e253 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 5 Sep 2026 13:22:44 -0600 Subject: [PATCH 3/5] naett: Stop the transfer when the body writer fails it Found reviewing the commits before this one. On Windows a short write marked the request complete and then queued another read regardless, which is worse than it sounds: the caller is free to close a response the moment it sees complete, so WinHTTP carried on writing into memory that was being freed. It also meant cancelling didn't actually stop anything there. Stop at that point, and ignore anything raised after the request is complete. The Apple callbacks do the same check, so the cancellation we ask for after a failed write doesn't overwrite the error that caused it. naettMake only asserted that its request was non-NULL, and the request constructors do return NULL when a platform can't set one up - an unparseable URL is enough on Windows. Asserts are compiled out in release, so that was a null dereference. It returns NULL now, and HTTPSRequest checks for it and fails the request instead of handing it on. Also: a request cancelled between construction and Start() didn't carry that into the sink, and -1000 - our own cancellation code, not naett's - was logging "Unhandled naett error" on a perfectly normal cancel. --- Common/Net/HTTPNaettRequest.cpp | 34 +++++++++++++++++++++++++++++++-- ext/naett-lib/README-ppsspp.md | 9 +++++++++ ext/naett-lib/src/naett_core.c | 6 ++++++ ext/naett-lib/src/naett_osx.c | 8 +++++++- ext/naett-lib/src/naett_win.c | 15 +++++++++++++++ 5 files changed, 69 insertions(+), 3 deletions(-) diff --git a/Common/Net/HTTPNaettRequest.cpp b/Common/Net/HTTPNaettRequest.cpp index 5d61427bd0..250b01fa8e 100644 --- a/Common/Net/HTTPNaettRequest.cpp +++ b/Common/Net/HTTPNaettRequest.cpp @@ -89,11 +89,35 @@ void HTTPSRequest::Start() { // Our own writer, so that Cancel() can actually stop a transfer rather than just relabelling // it once it finishes. sink_ = std::make_shared(); + // In case someone managed to cancel us between construction and here. + sink_->cancelled = cancelled_; options.push_back(naettBodyWriter(&HTTPSRequest::WriteBodyThunk, sink_.get())); const naettOption **opts = (const naettOption **)options.data(); req_ = naettRequestWithOptions(url_.c_str(), (int)options.size(), opts); + if (!req_) { + // naett couldn't set the request up - a URL it can't parse, most likely. Fail it here + // rather than handing a null to naettMake. + ERROR_LOG(Log::HTTP, "Couldn't create a request for '%s'", url_.c_str()); + resultCode_ = naettGenericError; + failed_ = true; + completed_ = true; + sink_.reset(); + progress_.Update(0, 0, true); + return; + } res_ = naettMake(req_); + if (!res_) { + ERROR_LOG(Log::HTTP, "Couldn't start a request for '%s'", url_.c_str()); + naettFree(req_); + req_ = nullptr; + resultCode_ = naettGenericError; + failed_ = true; + completed_ = true; + sink_.reset(); + progress_.Update(0, 0, true); + return; + } progress_.Update(0, 0, false); } @@ -129,8 +153,10 @@ void HTTPSRequest::Join() { bool HTTPSRequest::Done() { if (completed_) return true; - - _dbg_assert_(res_ != nullptr); + if (!res_) { + // Never started, or already let go of. Nothing left to wait for. + return true; + } if (!naettComplete(res_)) { int total = 0; @@ -164,6 +190,10 @@ bool HTTPSRequest::Done() { case naettGenericError: // -5 ERROR_LOG(Log::HTTP, "Generic error"); break; + case -1000: + // Ours, not naett's - see above. Cancelling is a normal thing to do, not an error. + INFO_LOG(Log::HTTP, "Request to '%s' cancelled", url_.c_str()); + break; default: ERROR_LOG(Log::HTTP, "Unhandled naett error %d", resultCode_); break; diff --git a/ext/naett-lib/README-ppsspp.md b/ext/naett-lib/README-ppsspp.md index 5f56e0f1a6..b0612eb6fb 100644 --- a/ext/naett-lib/README-ppsspp.md +++ b/ext/naett-lib/README-ppsspp.md @@ -107,3 +107,12 @@ Keep this list up to date - it's what makes it possible to move to a newer upstr the data just keeps arriving. - `naett.h`: documented what the body writer's return value means, since three of the four backends' behaviour depends on it. +- `src/naett_win.c`: a short write from the body writer marked the request complete and then + queued another read anyway, so the transfer carried on and WinHTTP kept writing into a + response the caller was by then free to close. It stops there now, and the callback returns + early for anything raised after the request is complete. +- `src/naett_core.c`: `naettMake` only asserted that the request wasn't NULL, and the request + constructors return NULL when the platform can't set one up - a URL it can't parse, say. In a + release build that walked into a null dereference. Returns NULL instead. +- `src/naett_osx.c`: the delegate callbacks ignore a response that's already complete, so a + cancellation we asked for doesn't overwrite the error that caused it. diff --git a/ext/naett-lib/src/naett_core.c b/ext/naett-lib/src/naett_core.c index 71f5cdaf7a..b6f4efc8c3 100644 --- a/ext/naett-lib/src/naett_core.c +++ b/ext/naett-lib/src/naett_core.c @@ -310,6 +310,12 @@ naettReq* naettRequestWithOptions(const char* url, int numOptions, const naettOp naettRes* naettMake(naettReq* request) { assert(initialized); assert(request != NULL); + // PPSSPP: naettRequest* returns NULL when the platform can't set the request up - a URL it + // can't parse, most likely - and the assert above is compiled out in release, so this used to + // walk straight into a null dereference. + if (request == NULL) { + return NULL; + } InternalRequest* req = (InternalRequest*)request; naettAlloc(InternalResponse, res); diff --git a/ext/naett-lib/src/naett_osx.c b/ext/naett-lib/src/naett_osx.c index 4befc79e20..a583d09057 100644 --- a/ext/naett-lib/src/naett_osx.c +++ b/ext/naett-lib/src/naett_osx.c @@ -100,7 +100,8 @@ void didReceiveData(id self, SEL _sel, id session, id dataTask, id data) { object_getInstanceVariable(self, "response", (void**)&res); // PPSSPP: didComplete already checked this; this one didn't, and the delegate outlives the // response when a session is invalidated. - if (res == NULL) { + if (res == NULL || res->complete) { + // PPSSPP: once complete, the caller may already be closing this - don't touch it further. release(p); return; } @@ -174,6 +175,11 @@ static void didComplete(id self, SEL _sel, id session, id dataTask, id error) { InternalResponse* res = NULL; object_getInstanceVariable(self, "response", (void**)&res); if (res != NULL) { + // PPSSPP: if we already failed the request - a writer that wouldn't take the data, say - + // that's the reason we want reported, not the cancellation it caused. + if (res->complete) { + return; + } if (error != nil) { res->code = naettConnectionError; } diff --git a/ext/naett-lib/src/naett_win.c b/ext/naett-lib/src/naett_win.c index c80b157150..84c38340b6 100644 --- a/ext/naett-lib/src/naett_win.c +++ b/ext/naett-lib/src/naett_win.c @@ -111,6 +111,13 @@ static void CALLBACK callback(HINTERNET request, DWORD_PTR context, DWORD status, LPVOID statusInformation, DWORD statusInfoLength) { InternalResponse* res = (InternalResponse*)context; + // PPSSPP: once we've reported the request as complete the caller is free to close it, so + // don't start any more I/O against it - anything still in flight would be writing into a + // response that's being freed. + if (res->complete) { + return; + } + switch (status) { case WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE: { // PPSSPP: the sizing call only fills in bufSize when it fails with @@ -177,9 +184,17 @@ callback(HINTERNET request, DWORD_PTR context, DWORD status, LPVOID statusInform size_t bytesRead = statusInfoLength; InternalRequest* req = res->request; + if (req == NULL) { + return; + } if (req->options.bodyWriter(res->buffer, (int)bytesRead, req->options.bodyWriterData) != bytesRead) { + // PPSSPP: the writer failed the request - it couldn't grow, or the caller is + // cancelling. Upstream marked it complete and then queued another read anyway, + // which both kept the transfer running and left WinHTTP writing into a response + // the caller was by then free to close. res->code = naettReadError; res->complete = 1; + break; } res->totalBytesRead += (int)bytesRead; // PPSSPP: bytesLeft is unsigned, so a read longer than announced used to wrap it From 618bdc2d729e04503577bbad58e0135a0d5fb4c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 5 Sep 2026 13:31:16 -0600 Subject: [PATCH 4/5] naett: Let naettInit be called more than once It asserted that it was the first call, so net::Init - which is documented as safe to call repeatedly, and is - needed a bool of its own purely to guard this one line. That's bookkeeping the library may as well do itself, and the WSA half of the same function already says as much: it doesn't track anything because WSAStartup counts its own references. So a repeat call does nothing, and net::Init loses g_naettInitialized. Asking HTTPSAvailable() twice costs nothing - on Linux it's a cached dlopen result and everywhere else it's a constant. --- Common/Net/Resolve.cpp | 18 ++++++++---------- ext/naett-lib/README-ppsspp.md | 3 +++ ext/naett-lib/naett.h | 4 ++++ ext/naett-lib/src/naett_core.c | 7 ++++++- 4 files changed, 21 insertions(+), 11 deletions(-) diff --git a/Common/Net/Resolve.cpp b/Common/Net/Resolve.cpp index bc52b912ac..e43192ee9d 100644 --- a/Common/Net/Resolve.cpp +++ b/Common/Net/Resolve.cpp @@ -31,7 +31,6 @@ extern JavaVM *gJvm; namespace net { -static bool g_naettInitialized; static bool g_wsaInitialized; void Init() { @@ -44,19 +43,18 @@ void Init() { g_wsaInitialized = true; } #endif - if (!g_naettInitialized) { + // naett ignores repeat calls the same way WSAStartup does, so there's nothing to track here + // either. HTTPSAvailable is cheap to ask twice - on Linux it's a cached dlopen result. #ifndef HTTPS_NOT_AVAILABLE #if PPSSPP_PLATFORM(ANDROID) - _assert_(gJvm != nullptr); - naettInit(gJvm); + _assert_(gJvm != nullptr); + naettInit(gJvm); #else - if (HTTPSAvailable()) { - naettInit(NULL); - } -#endif -#endif - g_naettInitialized = true; + if (HTTPSAvailable()) { + naettInit(NULL); } +#endif +#endif } bool HTTPSAvailable() { diff --git a/ext/naett-lib/README-ppsspp.md b/ext/naett-lib/README-ppsspp.md index b0612eb6fb..008b7262fb 100644 --- a/ext/naett-lib/README-ppsspp.md +++ b/ext/naett-lib/README-ppsspp.md @@ -116,3 +116,6 @@ Keep this list up to date - it's what makes it possible to move to a newer upstr release build that walked into a null dereference. Returns NULL instead. - `src/naett_osx.c`: the delegate callbacks ignore a response that's already complete, so a cancellation we asked for doesn't overwrite the error that caused it. +- `src/naett_core.c` / `naett.h`: `naettInit` asserted it was only ever called once, so a caller + that can be entered more than again needed a flag purely to guard it. A repeat call is a no-op + now, the way `WSAStartup` behaves, and `net::Init` dropped its `g_naettInitialized`. diff --git a/ext/naett-lib/naett.h b/ext/naett-lib/naett.h index 67716a0626..7d6102671d 100644 --- a/ext/naett-lib/naett.h +++ b/ext/naett-lib/naett.h @@ -17,6 +17,10 @@ typedef void* naettInitData; /** * @brief Global init method. * Call to initialize the library. + * + * PPSSPP: calling this more than once is fine - everything after the first call does nothing, + * including the platform setup, so the init data from the first call is the one that sticks. + * Not thread safe; call it during startup, before anything else can reach the library. */ void naettInit(naettInitData initThing); diff --git a/ext/naett-lib/src/naett_core.c b/ext/naett-lib/src/naett_core.c index b6f4efc8c3..7140beaebd 100644 --- a/ext/naett-lib/src/naett_core.c +++ b/ext/naett-lib/src/naett_core.c @@ -137,7 +137,12 @@ static void applyOptionParams(InternalRequest* req, InternalOption* option) { // Public API void naettInit(naettInitData initData) { - assert(!initialized); + // PPSSPP: upstream asserted that this was the first call. Callers that can be entered more + // than once then need a flag of their own purely to guard this, which is bookkeeping the + // library may as well do itself - so a repeat call is a no-op instead. + if (initialized) { + return; + } naettPlatformInit(initData); initialized = 1; } From 1041c976c6996a52664cb43ef81eff907650860f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 5 Sep 2026 13:57:31 -0600 Subject: [PATCH 5/5] http: Say what the sink's ownership actually is unique_ptr, not shared_ptr. Nothing ever shares it: naett holds a raw pointer and takes no part in the counting, and the abandonment path hands the sink over and lets go in the same breath. What's really going on is a single owner that moves, which is what unique_ptr says and shared_ptr left you to work out from reading all the uses. naett never frees the pointer it's given for the writer - it only reads bodyWriterData and hands it back to the callback - so deleting the sink is ours to do. The things naett does allocate, the request and response, stay raw pointers with explicit naettFree/naettClose. The length field was just buffer.size() written down twice. --- Common/Net/HTTPNaettRequest.cpp | 18 ++++++++---------- Common/Net/HTTPNaettRequest.h | 13 ++++++++----- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/Common/Net/HTTPNaettRequest.cpp b/Common/Net/HTTPNaettRequest.cpp index 250b01fa8e..4bbb2bd8ed 100644 --- a/Common/Net/HTTPNaettRequest.cpp +++ b/Common/Net/HTTPNaettRequest.cpp @@ -26,11 +26,11 @@ HTTPSRequest::~HTTPSRequest() { // 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. +// writes into is owned separately from the request, and that ownership can move - if we go away +// first, the sink is handed off rather than destroyed, and a late chunk lands somewhere that +// still exists instead of in a destroyed object. struct NaettBodySink { Buffer buffer; - int length = 0; // Written by us, read by the transfer thread. std::atomic cancelled{false}; }; @@ -40,7 +40,7 @@ struct NaettBodySink { // 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> *g_abandonedSinks = new std::vector>(); +static std::vector> *g_abandonedSinks = new std::vector>(); int HTTPSRequest::WriteBodyThunk(const void *source, int bytes, void *userData) { NaettBodySink *sink = (NaettBodySink *)userData; @@ -54,7 +54,6 @@ int HTTPSRequest::WriteBodyThunk(const void *source, int bytes, void *userData) } char *dest = sink->buffer.Append((size_t)bytes); memcpy(dest, source, bytes); - sink->length += bytes; return bytes; } @@ -88,7 +87,7 @@ void HTTPSRequest::Start() { 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(); + sink_ = std::make_unique(); // In case someone managed to cancel us between construction and here. sink_->cancelled = cancelled_; options.push_back(naettBodyWriter(&HTTPSRequest::WriteBodyThunk, sink_.get())); @@ -142,8 +141,7 @@ void HTTPSRequest::Join() { 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(); + g_abandonedSinks->push_back(std::move(sink_)); } res_ = nullptr; req_ = nullptr; @@ -168,8 +166,8 @@ 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_); // 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) { + const int bodyLength = sink_ ? (int)sink_->buffer.size() : 0; + if (bodyLength > 0) { buffer_.Append(sink_->buffer); } if (resultCode_ < 0) { diff --git a/Common/Net/HTTPNaettRequest.h b/Common/Net/HTTPNaettRequest.h index 7be0527e90..5c731678a5 100644 --- a/Common/Net/HTTPNaettRequest.h +++ b/Common/Net/HTTPNaettRequest.h @@ -1,7 +1,8 @@ #pragma once -#include +#include #include +#include #include "Common/Net/HTTPRequest.h" @@ -11,6 +12,8 @@ namespace http { +struct NaettBodySink; + // Really an asynchronous request. class HTTPSRequest : public Request { public: @@ -36,10 +39,10 @@ private: bool completed_ = false; bool failed_ = false; - // Where the response body lands. Deliberately not a member of this object: naett writes into - // it from its own transfer thread, and that can outlive us if we're torn down before the - // request finishes. See NaettBodySink in the .cpp. - std::shared_ptr sink_; + // Where the response body lands. Deliberately not part of this object: naett writes into it + // from its own transfer thread, and that can outlive us if we're torn down before the request + // finishes, so ownership has to be able to move elsewhere. See NaettBodySink in the .cpp. + std::unique_ptr sink_; // Naett state naettReq *req_ = nullptr;