From 28962036ef5945ef02691a58ae2be8e693d2e471 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 5 Sep 2026 11:53:32 -0600 Subject: [PATCH 1/6] naett: Fix two leaks and the response buffer's overflow naettFree frees the method and url it strdup'd but never the user agent, which stringSetter allocates exactly the same way. We set a user agent on every request, so that leaked on every one of them, on all platforms. On Linux, headerCallback strndup's each header line and only hands it to the header list when it finds a colon - the status line and the blank line that ends the block don't have one, so it leaked those every response, and again per hop when following redirects. defaultBodyWriter grew its capacity by doubling an int until the new data fit. Both the length and the resulting capacity come from the response, so that's signed overflow on a large one, and a negative capacity then reaches realloc as a huge size_t. It also assigned the realloc result straight over the old pointer, so a failed allocation lost the buffer and the memcpy below went through NULL. Grow in int64_t, cap at INT_MAX, and report failure by returning short - which is what every caller already checks for. --- ext/naett-lib/README-ppsspp.md | 10 +++++++++ ext/naett-lib/src/naett_core.c | 38 +++++++++++++++++++++++++-------- ext/naett-lib/src/naett_linux.c | 8 +++++++ 3 files changed, 47 insertions(+), 9 deletions(-) diff --git a/ext/naett-lib/README-ppsspp.md b/ext/naett-lib/README-ppsspp.md index 114b630b61..2ea0e361a2 100644 --- a/ext/naett-lib/README-ppsspp.md +++ b/ext/naett-lib/README-ppsspp.md @@ -33,3 +33,13 @@ Keep this list up to date - it's what makes it possible to move to a newer upstr - `src/naett_linux.c`: `curl_easy_setopt` is varargs and takes a `long` for these options; upstream passed `int` literals and `int` variables, which is UB on LP64 (and what curl's own typecheck macros warn about). They're `1L`/`(long)` now. +- `src/naett_core.c`: `naettFree` never freed `options.userAgent`, though it's `strdup`'d by + the same setter as `method`. Leaked once per request for every caller that sets a user agent, + which we do on all of them. +- `src/naett_core.c`: `defaultBodyWriter` doubled an `int` capacity until it fit, which is signed + overflow on a large response, and used the `realloc` result without checking it - losing the + old pointer and then `memcpy`ing through NULL. Grows in `int64_t` against `INT_MAX` and + reports failure by returning short, which every caller already treats as an error. +- `src/naett_linux.c`: `headerCallback` only handed its `strndup` to the header list when the + line had a colon, and leaked it otherwise. curl passes the status line and the blank line that + ends the header block, so that leaked at least twice per response. diff --git a/ext/naett-lib/src/naett_core.c b/ext/naett-lib/src/naett_core.c index 0e9ee529e7..bc6e2e24e1 100644 --- a/ext/naett-lib/src/naett_core.c +++ b/ext/naett-lib/src/naett_core.c @@ -1,6 +1,8 @@ #include "naett_internal.h" #include #include +#include +#include #include #include #include @@ -81,18 +83,34 @@ static int defaultBodyReader(void* dest, int bufferSize, void* userData) { return bytesToRead; } +// PPSSPP: `bytes` and the resulting capacity come off the network, so the growth has to be +// done in a type that can't wrap into a negative capacity, and a failed realloc has to leave +// the buffer we already have intact. Returning short is how this reports failure - every +// caller compares the result against what it asked to write and errors the request out. static int defaultBodyWriter(const void* source, int bytes, void* userData) { Buffer* buffer = (Buffer*) userData; - int newCapacity = buffer->capacity; - if (newCapacity == 0) { - newCapacity = bytes; + if (bytes <= 0) { + return 0; } - while (newCapacity - buffer->size < bytes) { - newCapacity *= 2; - } - if (newCapacity != buffer->capacity) { - buffer->data = realloc(buffer->data, newCapacity); - buffer->capacity = newCapacity; + if (buffer->capacity - buffer->size < bytes) { + int64_t newCapacity = buffer->capacity > 0 ? buffer->capacity : bytes; + while (newCapacity - buffer->size < bytes) { + newCapacity *= 2; + if (newCapacity > INT_MAX) { + newCapacity = INT_MAX; + break; + } + } + if (newCapacity - buffer->size < bytes) { + // Doesn't fit in the int sizes this struct uses. + return 0; + } + void* newData = realloc(buffer->data, (size_t)newCapacity); + if (newData == NULL) { + return 0; + } + buffer->data = newData; + buffer->capacity = (int)newCapacity; } char* dest = ((char*)buffer->data) + buffer->size; memcpy(dest, source, bytes); @@ -388,6 +406,8 @@ void naettFree(naettReq* request) { KVLink* node = req->options.headers; freeKVList(node); free((void*)req->options.method); + // PPSSPP: userAgent is strdup'd by stringSetter like method is, and was never freed. + free((void*)req->options.userAgent); free((void*)req->url); free(request); } diff --git a/ext/naett-lib/src/naett_linux.c b/ext/naett-lib/src/naett_linux.c index 165abb1960..3cdbac25f6 100644 --- a/ext/naett-lib/src/naett_linux.c +++ b/ext/naett-lib/src/naett_linux.c @@ -171,6 +171,9 @@ static size_t headerCallback(char* buffer, size_t size, size_t nitems, void* use size_t headerSize = size * nitems; char* headerName = strndup(buffer, headerSize); + if (headerName == NULL) { + return headerSize; + } char* split = strchr(headerName, ':'); if (split) { *split = 0; @@ -195,6 +198,11 @@ static size_t headerCallback(char* buffer, size_t size, size_t nitems, void* use node->key = headerName; node->value = headerValue; res->headers = node; + } else { + // PPSSPP: no colon, so the list never takes ownership of this copy. curl hands us the + // status line and the blank line that terminates the header block, neither of which has + // one - so upstream leaked at least twice per response, more with redirects. + free(headerName); } return headerSize; From cb2de6317c72f642929d5679686f6777f8edec15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 5 Sep 2026 11:54:49 -0600 Subject: [PATCH 2/6] naett: Fix the Apple backend's unregistered class and stack-sized headers createDelegate built its NSURLSessionDataDelegate with objc_allocateClassPair and then sent it +alloc without ever calling objc_registerClassPair. The runtime requires registration before a class pair can be used; everything up to then is still being assembled. Register it, after the methods and the ivar go on. The response header arrays were VLAs sized from the count the server sent, so a response with enough headers walked the stack off the end, and one with no headers at all declared zero-length VLAs, which is undefined by itself. Heap now, and skipped when there's nothing to read. The NSURLSession was also kept in the response without retaining it, while the autorelease pool it came from is drained on the way out of the function. It only survived because a session keeps itself alive while it has tasks running. Retain it, release it when the response closes. Finally, addMethod/addIvar signalled failure with assert alone, which is compiled out in release - a delegate missing its methods would just never receive data and the request would hang with nothing logged. --- ext/naett-lib/README-ppsspp.md | 11 ++++++++ ext/naett-lib/src/naett_objc.h | 18 ++++++++++--- ext/naett-lib/src/naett_osx.c | 47 ++++++++++++++++++++++++++-------- 3 files changed, 61 insertions(+), 15 deletions(-) diff --git a/ext/naett-lib/README-ppsspp.md b/ext/naett-lib/README-ppsspp.md index 2ea0e361a2..bc60037561 100644 --- a/ext/naett-lib/README-ppsspp.md +++ b/ext/naett-lib/README-ppsspp.md @@ -43,3 +43,14 @@ Keep this list up to date - it's what makes it possible to move to a newer upstr - `src/naett_linux.c`: `headerCallback` only handed its `strndup` to the header list when the line had a colon, and leaked it otherwise. curl passes the status line and the blank line that ends the header block, so that leaked at least twice per response. +- `src/naett_osx.c`: the delegate class was built with `objc_allocateClassPair` and then used + without ever calling `objc_registerClassPair`, which the runtime requires before the class can + be instantiated. Registered now, after the methods and ivar are added. +- `src/naett_osx.c`: the response header arrays were VLAs sized from the server's header count - + unbounded stack use from network data, and a zero-length VLA when a response had no headers. + They're heap allocations now, skipped entirely when there are none. +- `src/naett_osx.c`: the `NSURLSession` was stored in the response without a `retain`, though the + autorelease pool it came from is drained before returning. Retained, and released in + `naettPlatformCloseResponse`. +- `src/naett_objc.h`: `addMethod`/`addIvar` reported failure with `assert` only, so in release a + delegate could silently come up without its methods. They print as well now. diff --git a/ext/naett-lib/src/naett_objc.h b/ext/naett-lib/src/naett_objc.h index da95a02a52..813651c9d9 100644 --- a/ext/naett-lib/src/naett_objc.h +++ b/ext/naett-lib/src/naett_objc.h @@ -4,6 +4,7 @@ #if defined(__IOS__) || defined (__MACOS__) #include #include +#include #include #include @@ -42,11 +43,20 @@ // Check here to get the signature right: // https://nshipster.com/type-encodings/ // https://ko9.org/posts/encode-types/ -#define addMethod(CLASS, NAME, IMPL, SIGNATURE) \ - if (!class_addMethod(CLASS, sel(NAME), (IMP) (IMPL), (SIGNATURE))) assert(false) +// PPSSPP: these used to report failure with assert() alone, which is compiled out in release - +// so a delegate that failed to pick up its methods would just never receive data, and the +// request would hang with no clue why. Still asserts in debug, says so either way. +#define addMethod(CLASS, NAME, IMPL, SIGNATURE) \ + if (!class_addMethod(CLASS, sel(NAME), (IMP)(IMPL), (SIGNATURE))) { \ + fprintf(stderr, "naett: failed to add method %s\n", (NAME)); \ + assert(false); \ + } -#define addIvar(CLASS, NAME, SIZE, SIGNATURE) \ - if (!class_addIvar(CLASS, NAME, SIZE, rint(log2(SIZE)), SIGNATURE)) assert(false) +#define addIvar(CLASS, NAME, SIZE, SIGNATURE) \ + if (!class_addIvar(CLASS, NAME, SIZE, rint(log2(SIZE)), SIGNATURE)) { \ + fprintf(stderr, "naett: failed to add ivar %s\n", (NAME)); \ + assert(false); \ + } #define objc_alloc(CLASS) objc_msgSend_id(class(CLASS), sel("alloc")) #define autorelease(OBJ) objc_msgSend_void(OBJ, sel("autorelease")) diff --git a/ext/naett-lib/src/naett_osx.c b/ext/naett-lib/src/naett_osx.c index 68c70992f9..24eb7c8ae3 100644 --- a/ext/naett-lib/src/naett_osx.c +++ b/ext/naett-lib/src/naett_osx.c @@ -105,18 +105,31 @@ void didReceiveData(id self, SEL _sel, id session, id dataTask, id data) { id allHeaders = objc_msgSend_t(id)(response, sel("allHeaderFields")); NSUInteger headerCount = objc_msgSend_t(NSUInteger)(allHeaders, sel("count")); - id headerNames[headerCount]; - id headerValues[headerCount]; - - objc_msgSend_t(NSInteger, id*, id*, NSUInteger)( - allHeaders, sel("getObjects:andKeys:count:"), headerValues, headerNames, headerCount); + // PPSSPP: this was a pair of VLAs sized straight from the response, so a server with + // enough headers ran the stack out - and a response with none at all is a zero-length + // VLA, which is undefined on its own. Heap, and nothing to do when there are none. KVLink* firstHeader = NULL; - for (int i = 0; i < headerCount; i++) { - naettAlloc(KVLink, node); - node->key = strdup(objc_msgSend_t(const char*)(headerNames[i], sel("UTF8String"))); - node->value = strdup(objc_msgSend_t(const char*)(headerValues[i], sel("UTF8String"))); - node->next = firstHeader; - firstHeader = node; + if (headerCount > 0) { + id* headerNames = (id*)calloc(headerCount, sizeof(id)); + id* headerValues = (id*)calloc(headerCount, sizeof(id)); + if (headerNames != NULL && headerValues != NULL) { + objc_msgSend_t(NSInteger, id*, id*, NSUInteger)( + allHeaders, sel("getObjects:andKeys:count:"), headerValues, headerNames, headerCount); + for (NSUInteger i = 0; i < headerCount; i++) { + const char* key = objc_msgSend_t(const char*)(headerNames[i], sel("UTF8String")); + const char* value = objc_msgSend_t(const char*)(headerValues[i], sel("UTF8String")); + if (key == NULL || value == NULL) { + continue; + } + naettAlloc(KVLink, node); + node->key = strdup(key); + node->value = strdup(value); + node->next = firstHeader; + firstHeader = node; + } + } + free(headerNames); + free(headerValues); } res->headers = firstHeader; @@ -156,6 +169,10 @@ static id createDelegate(void) { addMethod(TaskDelegateClass, "URLSession:dataTask:didReceiveData:", didReceiveData, "v@:@@@"); addMethod(TaskDelegateClass, "URLSession:task:didCompleteWithError:", didComplete, "v@:@@@"); addIvar(TaskDelegateClass, "response", sizeof(void*), "^v"); + // PPSSPP: a class from objc_allocateClassPair isn't usable until it's registered, and + // upstream never did - it went straight to +alloc. Methods, protocols and ivars all have + // to be added before this call, which is why it comes last. + objc_registerClassPair(TaskDelegateClass); } id delegate = objc_msgSend_id((id)TaskDelegateClass, sel("alloc")); @@ -179,6 +196,10 @@ void naettPlatformMakeRequest(InternalResponse* res) { delegate, nil); + // PPSSPP: sessionWithConfiguration: hands back an autoreleased session, and the pool below + // is drained before we return. It survived on NSURLSession keeping itself alive while tasks + // run, but we hold the pointer until naettClose, so hold a reference to go with it. + retain(session); res->session = session; id task = objc_msgSend_t(id, id)(session, sel("dataTaskWithRequest:"), req->urlRequest); @@ -193,7 +214,11 @@ void naettPlatformFreeRequest(InternalRequest* req) { } void naettPlatformCloseResponse(InternalResponse* res) { + if (res->session == nil) { + return; + } objc_msgSend_void(res->session, sel("invalidateAndCancel")); + release(res->session); res->session = nil; } From 0e9397b8b46b16b178a65954a3850fc89e1131a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 5 Sep 2026 11:55:59 -0600 Subject: [PATCH 3/6] naett: Harden the WinHTTP backend's header and read paths WinHttpQueryHeaders only writes the size it needs when it fails with ERROR_INSUFFICIENT_BUFFER. Any other failure left bufSize at zero, so we allocated nothing and unpackHeaders walked wcslen over it looking for the double-null that terminates the list. Check the size, check the second query, and allocate zeroed with room for a terminator. winToUTF8, winFromUTF8 and wcsndup can all return NULL - on a failed conversion or a failed allocation - and not one caller checked. packHeaders is the one that mattered: its result goes straight into headers[0]. res->bytesLeft is a size_t counting down from what WinHTTP announced. If a read ever returned more than that, it wrapped to an enormous count and the loop kept reading. --- ext/naett-lib/README-ppsspp.md | 8 +++++ ext/naett-lib/src/naett_win.c | 59 ++++++++++++++++++++++++++++------ 2 files changed, 57 insertions(+), 10 deletions(-) diff --git a/ext/naett-lib/README-ppsspp.md b/ext/naett-lib/README-ppsspp.md index bc60037561..b40fff649f 100644 --- a/ext/naett-lib/README-ppsspp.md +++ b/ext/naett-lib/README-ppsspp.md @@ -54,3 +54,11 @@ Keep this list up to date - it's what makes it possible to move to a newer upstr `naettPlatformCloseResponse`. - `src/naett_objc.h`: `addMethod`/`addIvar` reported failure with `assert` only, so in release a delegate could silently come up without its methods. They print as well now. +- `src/naett_win.c`: the header sizing call only reports a size when it fails with + `ERROR_INSUFFICIENT_BUFFER`; on any other failure the size stayed zero and `unpackHeaders` ran + `wcslen` over a `malloc(0)` block. Checked, and the second query's result is checked too. +- `src/naett_win.c`: `winToUTF8`/`winFromUTF8`/`wcsndup` could all return NULL and every caller + used the result unchecked - `packHeaders` most visibly, whose result is indexed as + `headers[0]`. All checked now. +- `src/naett_win.c`: `res->bytesLeft` is unsigned, so a read longer than the announced count + wrapped it into an enormous value and kept the read loop running. diff --git a/ext/naett-lib/src/naett_win.c b/ext/naett-lib/src/naett_win.c index b75c6874a8..57e1f7858a 100644 --- a/ext/naett-lib/src/naett_win.c +++ b/ext/naett-lib/src/naett_win.c @@ -14,7 +14,13 @@ void naettPlatformInit(naettInitData initData) { static char* winToUTF8(LPWSTR source) { int length = WideCharToMultiByte(CP_UTF8, 0, source, -1, NULL, 0, NULL, NULL); + if (length <= 0) { + return NULL; + } char* chars = (char*)malloc(length); + if (chars == NULL) { + return NULL; + } int result = WideCharToMultiByte(CP_UTF8, 0, source, -1, chars, length, NULL, NULL); if (!result) { free(chars); @@ -25,7 +31,13 @@ static char* winToUTF8(LPWSTR source) { static LPWSTR winFromUTF8(const char* source) { int length = MultiByteToWideChar(CP_UTF8, 0, source, -1, NULL, 0); + if (length <= 0) { + return NULL; + } LPWSTR chars = (LPWSTR)malloc(length * sizeof(WCHAR)); + if (chars == NULL) { + return NULL; + } int result = MultiByteToWideChar(CP_UTF8, 0, source, -1, chars, length); if (!result) { free(chars); @@ -43,6 +55,9 @@ static LPWSTR winFromUTF8(const char* source) { static LPWSTR wcsndup(LPCWSTR str, size_t len) { LPWSTR result = calloc(1, sizeof(WCHAR) * (len + 1)); + if (result == NULL) { + return NULL; + } wcsncpy(result, str, len); return result; } @@ -69,6 +84,10 @@ static void unpackHeaders(InternalResponse* res, LPWSTR packed) { KVLink* firstHeader = NULL; while ((len = wcslen(packed)) != 0) { char* header = winToUTF8(packed); + if (header == NULL) { + packed += len + 1; + continue; + } char* split = strchr(header, ':'); if (split) { *split = 0; @@ -94,6 +113,9 @@ callback(HINTERNET request, DWORD_PTR context, DWORD status, LPVOID statusInform switch (status) { case WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE: { + // PPSSPP: the sizing call only fills in bufSize when it fails with + // ERROR_INSUFFICIENT_BUFFER. Any other failure left it at zero, and unpackHeaders + // then ran wcslen over a malloc(0) block. DWORD bufSize = 0; WinHttpQueryHeaders(request, WINHTTP_QUERY_RAW_HEADERS, @@ -101,15 +123,18 @@ callback(HINTERNET request, DWORD_PTR context, DWORD status, LPVOID statusInform NULL, &bufSize, WINHTTP_NO_HEADER_INDEX); - LPWSTR buffer = (LPWSTR)malloc(bufSize); - WinHttpQueryHeaders(request, - WINHTTP_QUERY_RAW_HEADERS, - WINHTTP_HEADER_NAME_BY_INDEX, - buffer, - &bufSize, - WINHTTP_NO_HEADER_INDEX); - unpackHeaders(res, buffer); - free(buffer); + if (bufSize >= sizeof(WCHAR)) { + LPWSTR buffer = (LPWSTR)calloc(1, bufSize + sizeof(WCHAR)); + if (buffer != NULL && WinHttpQueryHeaders(request, + WINHTTP_QUERY_RAW_HEADERS, + WINHTTP_HEADER_NAME_BY_INDEX, + buffer, + &bufSize, + WINHTTP_NO_HEADER_INDEX)) { + unpackHeaders(res, buffer); + } + free(buffer); + } const char* contentLength = naettGetHeader((naettRes*)res, "Content-Length"); if (!contentLength || sscanf(contentLength, "%d", &res->contentLength) != 1) { @@ -157,7 +182,9 @@ callback(HINTERNET request, DWORD_PTR context, DWORD status, LPVOID statusInform res->complete = 1; } res->totalBytesRead += (int)bytesRead; - res->bytesLeft -= bytesRead; + // PPSSPP: bytesLeft is unsigned, so a read longer than announced used to wrap it + // into an enormous count and keep the read loop going. + res->bytesLeft = bytesRead >= res->bytesLeft ? 0 : res->bytesLeft - bytesRead; if (res->bytesLeft > 0) { size_t bytesToRead = min(res->bytesLeft, sizeof(res->buffer)); if (!WinHttpReadData(request, res->buffer, (DWORD)bytesToRead, NULL)) { @@ -212,6 +239,9 @@ callback(HINTERNET request, DWORD_PTR context, DWORD status, LPVOID statusInform int naettPlatformInitRequest(InternalRequest* req) { LPWSTR url = winFromUTF8(req->url); + if (url == NULL) { + return 0; + } URL_COMPONENTS components; ZeroMemory(&components, sizeof(components)); @@ -230,6 +260,9 @@ int naettPlatformInitRequest(InternalRequest* req) { req->host = wcsndup(components.lpszHostName, components.dwHostNameLength); req->resource = wcsndup(components.lpszUrlPath, components.dwUrlPathLength + components.dwExtraInfoLength); free(url); + if (req->host == NULL || req->resource == NULL) { + return 0; + } LPWSTR uaBuf = winFromUTF8(req->options.userAgent ? req->options.userAgent : NAETT_UA); req->session = WinHttpOpen(uaBuf, @@ -269,6 +302,12 @@ int naettPlatformInitRequest(InternalRequest* req) { } LPCWSTR headers = packHeaders(req); + if (headers == NULL) { + // PPSSPP: only happens if a header didn't survive the UTF-8 conversion, but upstream + // indexed straight into it. + naettPlatformFreeRequest(req); + return 0; + } if (headers[0] != 0) { if (!WinHttpAddRequestHeaders( req->request, headers, -1, WINHTTP_ADDREQ_FLAG_ADD | WINHTTP_ADDREQ_FLAG_REPLACE)) { From 60919a2ba9d4c9f23a07cb104a9a289f87b9ea59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 5 Sep 2026 12:02:22 -0600 Subject: [PATCH 4/6] naett: Fix the curl worker's pipe read, queueing and handle cleanup The worker reads a queued CURL* out of a pipe eight bytes at a time, tracking how much it has so far - but it always read into the start of the buffer rather than at that offset. A short read would have resumed mid-pointer and eventually handed curl_multi_add_handle a spliced pointer. It never bit because a write that size to a pipe is atomic, so reads are all-or-nothing, but the code is written as though it isn't. The write that queues the request was unchecked. If it ever failed, the request was never run and never completed, and the caller sits in naettComplete forever. It reports the failure now, and retries on EINTR. curl wants an easy handle out of its multi before cleanup; that needs curl_multi_remove_handle, which meant adding it - and curl_multi_cleanup, for the init failure paths that leaked the multi handle and the pipe - to the dlopen table. workerRunning is written by the worker and read by the request path, so it's an atomic rather than a plain int. And the read/write callbacks passed the body callbacks' int return straight back to curl, which takes a size_t: a negative came through as an enormous count rather than the error it was. --- ext/naett-lib/README-ppsspp.md | 15 +++++++++ ext/naett-lib/src/naett_curl.c | 2 ++ ext/naett-lib/src/naett_curl.h | 4 +++ ext/naett-lib/src/naett_linux.c | 55 ++++++++++++++++++++++++++++----- 4 files changed, 69 insertions(+), 7 deletions(-) diff --git a/ext/naett-lib/README-ppsspp.md b/ext/naett-lib/README-ppsspp.md index b40fff649f..29cd6c3b9a 100644 --- a/ext/naett-lib/README-ppsspp.md +++ b/ext/naett-lib/README-ppsspp.md @@ -62,3 +62,18 @@ Keep this list up to date - it's what makes it possible to move to a newer upstr `headers[0]`. All checked now. - `src/naett_win.c`: `res->bytesLeft` is unsigned, so a read longer than the announced count wrapped it into an enormous value and kept the read loop running. +- `src/naett_linux.c`: the worker read the queued `CURL*` out of the pipe into the start of its + buffer while tracking a fill position, so a short read would have resumed mid-pointer and + handed curl a mangled handle. Only ever safe because a write that size to a pipe is atomic. +- `src/naett_linux.c`: the easy handle is removed from the multi before being cleaned up, which + is what curl asks for. `curl_multi_remove_handle` and `curl_multi_cleanup` were added to the + dlopen table in `naett_curl.h`/`naett_curl.c` for this. +- `src/naett_linux.c`: `workerRunning` is written by the worker and read by the request path, so + it's an `atomic_int` now rather than a plain `int`. +- `src/naett_linux.c`: the `write` that hands a request to the worker was unchecked - a failed + one meant a request that never ran and never completed, so the caller polled `naettComplete` + forever. Also retries on `EINTR`, and `curl_easy_init` failure is handled. +- `src/naett_linux.c`: the read and write callbacks returned the body callbacks' `int` straight + to curl, which reads it as a `size_t` - a negative arrived as an enormous count instead of an + error. +- `src/naett_linux.c`: the multi handle and the pipe leaked when init failed partway. diff --git a/ext/naett-lib/src/naett_curl.c b/ext/naett-lib/src/naett_curl.c index 4a9a7617aa..da798fc53c 100644 --- a/ext/naett-lib/src/naett_curl.c +++ b/ext/naett-lib/src/naett_curl.c @@ -37,7 +37,9 @@ static int LoadSymbols(void *lib) { LOAD(easy_getinfo, "curl_easy_getinfo") LOAD(easy_cleanup, "curl_easy_cleanup") LOAD(multi_init, "curl_multi_init") + LOAD(multi_cleanup, "curl_multi_cleanup") LOAD(multi_add_handle, "curl_multi_add_handle") + LOAD(multi_remove_handle, "curl_multi_remove_handle") LOAD(multi_perform, "curl_multi_perform") LOAD(multi_info_read, "curl_multi_info_read") LOAD(multi_wait, "curl_multi_wait") diff --git a/ext/naett-lib/src/naett_curl.h b/ext/naett-lib/src/naett_curl.h index b474027525..656b5f713f 100644 --- a/ext/naett-lib/src/naett_curl.h +++ b/ext/naett-lib/src/naett_curl.h @@ -46,7 +46,9 @@ typedef struct { void (*easy_cleanup)(CURL *handle); CURLM *(*multi_init)(void); + CURLMcode (*multi_cleanup)(CURLM *multi); CURLMcode (*multi_add_handle)(CURLM *multi, CURL *easy); + CURLMcode (*multi_remove_handle)(CURLM *multi, CURL *easy); CURLMcode (*multi_perform)(CURLM *multi, int *runningHandles); CURLMsg *(*multi_info_read)(CURLM *multi, int *msgsInQueue); CURLMcode (*multi_wait)(CURLM *multi, struct curl_waitfd extraFDs[], unsigned int extraNFDs, int timeoutMS, @@ -64,7 +66,9 @@ extern NaettCurl g_curl; #define curl_easy_getinfo g_curl.easy_getinfo #define curl_easy_cleanup g_curl.easy_cleanup #define curl_multi_init g_curl.multi_init +#define curl_multi_cleanup g_curl.multi_cleanup #define curl_multi_add_handle g_curl.multi_add_handle +#define curl_multi_remove_handle g_curl.multi_remove_handle #define curl_multi_perform g_curl.multi_perform #define curl_multi_info_read g_curl.multi_info_read #define curl_multi_wait g_curl.multi_wait diff --git a/ext/naett-lib/src/naett_linux.c b/ext/naett-lib/src/naett_linux.c index 3cdbac25f6..ad505137e6 100644 --- a/ext/naett-lib/src/naett_linux.c +++ b/ext/naett-lib/src/naett_linux.c @@ -14,6 +14,8 @@ #include #include #include +#include +#include static pthread_t workerThread; static int handleReadFD = 0; @@ -21,8 +23,9 @@ static int handleWriteFD = 0; // PPSSPP: upstream had a panic() here that called exit(1). Taking the whole application // down because a download failed isn't acceptable for us, so failures now leave the -// backend disabled and requests complete with naettGenericError instead. -static int workerRunning = 0; +// backend disabled and requests complete with naettGenericError instead. Atomic because +// the worker writes it while the request path reads it. +static atomic_int workerRunning = 0; static void fail(const char* message) { fprintf(stderr, "naett: %s\n", message); @@ -62,6 +65,8 @@ static void* curlWorker(void* data) { curl_easy_getinfo(handle, CURLINFO_RESPONSE_CODE, &responseCode); res->code = (int)responseCode; res->complete = 1; + // PPSSPP: curl wants the easy handle out of the multi before it's cleaned up. + curl_multi_remove_handle(mc, handle); curl_easy_cleanup(handle); } @@ -73,7 +78,10 @@ static void* curlWorker(void* data) { usleep(100 * 1000); } - int bytesRead = read(handleReadFD, newHandle.buf, sizeof(newHandle.buf) - newHandlePos); + // PPSSPP: upstream always read into the start of the buffer while tracking a position, + // so a short read would have restarted mid-pointer and handed curl a mangled handle. A + // write of this size to a pipe is atomic, which is the only reason it never bit. + int bytesRead = read(handleReadFD, newHandle.buf + newHandlePos, sizeof(newHandle.buf) - newHandlePos); if (bytesRead > 0) { newHandlePos += bytesRead; } @@ -99,6 +107,7 @@ void naettPlatformInit(naettInitData initData) { int fds[2]; if (pipe(fds) != 0) { fail("failed to open pipe"); + curl_multi_cleanup(mc); return; } handleReadFD = fds[0]; @@ -113,25 +122,38 @@ void naettPlatformInit(naettInitData initData) { workerRunning = 1; if (pthread_create(&workerThread, &attr, curlWorker, mc) != 0) { fail("failed to start the HTTP worker thread"); + // PPSSPP: nothing owns any of this now that there's no worker. + close(handleReadFD); + close(handleWriteFD); + handleReadFD = handleWriteFD = -1; + curl_multi_cleanup(mc); } + pthread_attr_destroy(&attr); } int naettPlatformInitRequest(InternalRequest* req) { return 1; } +// PPSSPP: the body callbacks return int, and curl reads the result as a size_t - so a negative +// return used to come through as an enormous count rather than as the error it is. Returning +// something other than what curl asked for aborts the transfer, which is what we want. static size_t readCallback(char* buffer, size_t size, size_t numItems, void* userData) { InternalResponse* res = (InternalResponse*)userData; InternalRequest* req = res->request; - return req->options.bodyReader(buffer, size * numItems, req->options.bodyReaderData); + int bytesRead = req->options.bodyReader(buffer, (int)(size * numItems), req->options.bodyReaderData); + return bytesRead > 0 ? (size_t)bytesRead : 0; } static size_t writeCallback(char* ptr, size_t size, size_t numItems, void* userData) { InternalResponse* res = (InternalResponse*)userData; InternalRequest* req = res->request; - size_t bytesWritten = req->options.bodyWriter(ptr, size * numItems, req->options.bodyWriterData); + int bytesWritten = req->options.bodyWriter(ptr, (int)(size * numItems), req->options.bodyWriterData); + if (bytesWritten <= 0) { + return 0; + } res->totalBytesRead += bytesWritten; - return bytesWritten; + return (size_t)bytesWritten; } #define METHOD(A, B, C) (((A) << 16) | ((B) << 8) | (C)) @@ -220,6 +242,11 @@ void naettPlatformMakeRequest(InternalResponse* res) { } CURL* c = curl_easy_init(); + if (c == NULL) { + res->code = naettGenericError; + res->complete = 1; + return; + } curl_easy_setopt(c, CURLOPT_URL, req->url); curl_easy_setopt(c, CURLOPT_CONNECTTIMEOUT_MS, (long)req->options.timeoutMS); @@ -263,7 +290,21 @@ void naettPlatformMakeRequest(InternalResponse* res) { curl_easy_setopt(c, CURLOPT_PRIVATE, res); - write(handleWriteFD, &c, sizeof(c)); + // PPSSPP: this is the only thing that hands the request to the worker. Upstream ignored the + // result, so a failed write meant a request that never ran and never completed - the caller + // then waits on naettComplete forever. + ssize_t written = write(handleWriteFD, &c, sizeof(c)); + while (written < 0 && errno == EINTR) { + written = write(handleWriteFD, &c, sizeof(c)); + } + if (written != (ssize_t)sizeof(c)) { + fprintf(stderr, "naett: couldn't queue a request for the HTTP worker\n"); + curl_slist_free_all(headerList); + res->headerList = NULL; + curl_easy_cleanup(c); + res->code = naettGenericError; + res->complete = 1; + } } void naettPlatformFreeRequest(InternalRequest* req) { From da2f30ad46372c63787e8396c97a268994cd1875 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 5 Sep 2026 12:04:31 -0600 Subject: [PATCH 5/6] naett: Stop leaving threads attached to the JVM, and check JNI results getEnv called AttachCurrentThread and nothing detached. processRequest gets away with it by detaching its own thread at the end, but naettPlatformInitRequest and naettPlatformFreeRequest run on whatever thread the caller is using - and a thread that exits while still attached takes the process down on Android, with "Native thread exiting without having called DetachCurrentThread". They now attach only when the thread isn't already, and detach on the way out. pthread_create's result was ignored. Without a worker nothing ever sets res->complete, so the caller polls naettComplete forever. getOutputStream throws for anything from a refused connection onwards, and the calls after it ran with that exception still pending, which most of JNI doesn't allow. GetMethodID also returns NULL for a method it can't find, and calling with a NULL jmethodID aborts the VM - so the call helpers check. Same for a header whose value list is empty, which handed GetStringUTFChars a null. Checked with the NDK's clang; the other backends were syntax-checked the same way, against stub headers. --- ext/naett-lib/README-ppsspp.md | 11 ++++ ext/naett-lib/src/naett_android.c | 101 +++++++++++++++++++++++++++--- 2 files changed, 103 insertions(+), 9 deletions(-) diff --git a/ext/naett-lib/README-ppsspp.md b/ext/naett-lib/README-ppsspp.md index 29cd6c3b9a..4178b678a4 100644 --- a/ext/naett-lib/README-ppsspp.md +++ b/ext/naett-lib/README-ppsspp.md @@ -77,3 +77,14 @@ Keep this list up to date - it's what makes it possible to move to a newer upstr to curl, which reads it as a `size_t` - a negative arrived as an enormous count instead of an error. - `src/naett_linux.c`: the multi handle and the pipe leaked when init failed partway. +- `src/naett_android.c`: `getEnv` called `AttachCurrentThread` and nothing ever detached. + `processRequest` detaches its own thread, but `naettPlatformInitRequest`/`FreeRequest` run on + the caller's, and a thread that exits while attached is fatal on Android. They attach only if + the thread wasn't already, and detach when they're done. +- `src/naett_android.c`: `pthread_create`'s result was ignored - with no worker, nothing sets + `complete` and the caller polls `naettComplete` forever. +- `src/naett_android.c`: `getOutputStream` can throw, and the calls after it ran with the + exception still pending, which isn't allowed for most of JNI. Checked now. +- `src/naett_android.c`: `GetMethodID` returns NULL for a method it can't find, and calling with + a NULL `jmethodID` aborts the VM; the header loop could also hand `GetStringUTFChars` a null + value for a header with no entries. diff --git a/ext/naett-lib/src/naett_android.c b/ext/naett-lib/src/naett_android.c index 30d35550dd..4b79fa6012 100644 --- a/ext/naett-lib/src/naett_android.c +++ b/ext/naett-lib/src/naett_android.c @@ -31,13 +31,36 @@ static JavaVM* getVM() { return globalVM; } -static JNIEnv* getEnv() { +// PPSSPP: AttachCurrentThread on an already-attached thread is a no-op that still hands back the +// env, so report whether we're the ones who attached it. A thread that exits while attached is +// fatal on Android ("Native thread exiting without having called DetachCurrentThread"), and +// naettPlatformInitRequest/FreeRequest run on whichever thread the caller uses. +static JNIEnv* getEnvAttached(int* attached) { JavaVM* vm = getVM(); - JNIEnv* env; - (*vm)->AttachCurrentThread(vm, &env, NULL); + JNIEnv* env = NULL; + *attached = 0; + if ((*vm)->GetEnv(vm, (void**)&env, JNI_VERSION_1_6) == JNI_OK) { + return env; + } + if ((*vm)->AttachCurrentThread(vm, &env, NULL) != JNI_OK) { + return NULL; + } + *attached = 1; return env; } +static void detachIfAttached(int attached) { + if (attached) { + JavaVM* vm = getVM(); + (*vm)->DetachCurrentThread(vm); + } +} + +static JNIEnv* getEnv() { + int attached = 0; + return getEnvAttached(&attached); +} + static int catch (JNIEnv* env) { int thrown = (*env)->ExceptionCheck(env); if (thrown) { @@ -47,14 +70,23 @@ static int catch (JNIEnv* env) { } static jmethodID getMethod(JNIEnv* env, jobject instance, const char* method, const char* sig) { + if (instance == NULL) { + return NULL; + } jclass clazz = (*env)->GetObjectClass(env, instance); jmethodID id = (*env)->GetMethodID(env, clazz, method, sig); (*env)->DeleteLocalRef(env, clazz); return id; } +// PPSSPP: GetMethodID leaves an exception pending and returns NULL when it can't find the +// method, and calling with a NULL jmethodID aborts the VM. Bail out instead - the caller's +// catch() picks up the pending exception. static jobject call(JNIEnv* env, jobject instance, const char* method, const char* sig, ...) { jmethodID methodID = getMethod(env, instance, method, sig); + if (methodID == NULL) { + return NULL; + } va_list args; va_start(args, sig); jobject result = (*env)->CallObjectMethodV(env, instance, methodID, args); @@ -64,6 +96,9 @@ static jobject call(JNIEnv* env, jobject instance, const char* method, const cha static void voidCall(JNIEnv* env, jobject instance, const char* method, const char* sig, ...) { jmethodID methodID = getMethod(env, instance, method, sig); + if (methodID == NULL) { + return; + } va_list args; va_start(args, sig); (*env)->CallVoidMethodV(env, instance, methodID, args); @@ -72,6 +107,9 @@ static void voidCall(JNIEnv* env, jobject instance, const char* method, const ch static jint intCall(JNIEnv* env, jobject instance, const char* method, const char* sig, ...) { jmethodID methodID = getMethod(env, instance, method, sig); + if (methodID == NULL) { + return 0; + } va_list args; va_start(args, sig); jint result = (*env)->CallIntMethodV(env, instance, methodID, args); @@ -84,7 +122,11 @@ void naettPlatformInit(naettInitData initData) { } int naettPlatformInitRequest(InternalRequest* req) { - JNIEnv* env = getEnv(); + int attached = 0; + JNIEnv* env = getEnvAttached(&attached); + if (env == NULL) { + return 0; + } (*env)->PushLocalFrame(env, 10); jclass URL = (*env)->FindClass(env, "java/net/URL"); jmethodID newURL = (*env)->GetMethodID(env, URL, "", "(Ljava/lang/String;)V"); @@ -92,10 +134,12 @@ int naettPlatformInitRequest(InternalRequest* req) { jobject url = (*env)->NewObject(env, URL, newURL, urlString); if (catch (env)) { (*env)->PopLocalFrame(env, NULL); + detachIfAttached(attached); return 0; } req->urlObject = (*env)->NewGlobalRef(env, url); (*env)->PopLocalFrame(env, NULL); + detachIfAttached(attached); return 1; } @@ -137,6 +181,12 @@ static void* processRequest(void* data) { strcmp(req->options.method, "PATCH") == 0 || strcmp(req->options.method, "DELETE") == 0) { voidCall(env, connection, "setDoOutput", "(Z)V", 1); outputStream = call(env, connection, "getOutputStream", "()Ljava/io/OutputStream;"); + // PPSSPP: most JNI calls are not safe to make with an exception pending, and this one + // throws for anything from a refused connection to a protocol the server won't take. + if (catch (env)) { + res->code = naettConnectionError; + goto finally; + } } jobject methodString = (*env)->NewStringUTF(env, req->options.method); voidCall(env, connection, "setRequestMethod", "(Ljava/lang/String;)V", methodString); @@ -182,11 +232,25 @@ static void* processRequest(void* data) { if (name == NULL) { continue; } - const char* nameString = (*env)->GetStringUTFChars(env, name, NULL); - jobject values = call(env, headerMap, "get", "(Ljava/lang/Object;)Ljava/lang/Object;", name); jstring value = call(env, values, "get", "(I)Ljava/lang/Object;", 0); + // PPSSPP: a header with no values gives a null here, and GetStringUTFChars on it aborts. + if (value == NULL) { + (*env)->ExceptionClear(env); + (*env)->DeleteLocalRef(env, name); + (*env)->DeleteLocalRef(env, values); + continue; + } + const char* nameString = (*env)->GetStringUTFChars(env, name, NULL); const char* valueString = (*env)->GetStringUTFChars(env, value, NULL); + if (nameString == NULL || valueString == NULL) { + if (nameString) (*env)->ReleaseStringUTFChars(env, name, nameString); + if (valueString) (*env)->ReleaseStringUTFChars(env, value, valueString); + (*env)->DeleteLocalRef(env, name); + (*env)->DeleteLocalRef(env, value); + (*env)->DeleteLocalRef(env, values); + continue; + } naettAlloc(KVLink, node); node->key = strdup(nameString); @@ -255,8 +319,17 @@ static void startWorkerThread(InternalResponse* res) { pthread_attr_t attr; pthread_attr_init(&attr); pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_JOINABLE); - pthread_create(&res->workerThread, &attr, processRequest, res); - pthread_setname_np(res->workerThread, "naett worker thread"); + // PPSSPP: upstream ignored this. With no thread, nothing ever sets res->complete and the + // caller polls naettComplete forever. + if (pthread_create(&res->workerThread, &attr, processRequest, res) != 0) { + LOGE("Failed to start the request worker thread"); + res->workerThread = 0; + res->code = naettGenericError; + res->complete = 1; + } else { + pthread_setname_np(res->workerThread, "naett worker thread"); + } + pthread_attr_destroy(&attr); } void naettPlatformMakeRequest(InternalResponse* res) { @@ -264,8 +337,18 @@ void naettPlatformMakeRequest(InternalResponse* res) { } void naettPlatformFreeRequest(InternalRequest* req) { - JNIEnv* env = getEnv(); + if (req->urlObject == NULL) { + // Init failed before it got that far. + return; + } + int attached = 0; + JNIEnv* env = getEnvAttached(&attached); + if (env == NULL) { + return; + } (*env)->DeleteGlobalRef(env, req->urlObject); + req->urlObject = NULL; + detachIfAttached(attached); } void naettPlatformCloseResponse(InternalResponse* res) { From a15e654f11032c991b9b9708c19a2909cd9c333c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 5 Sep 2026 12:07:03 -0600 Subject: [PATCH 6/6] naett: Don't hand a closing response to backends that can't see its request naettClose cleared res->request and then asked the backend to close the response - but the request is what a backend needs to do that. On Windows it owns the WinHTTP handles, so the close had nothing to work with, which is part of why it did nothing at all. Backend first, then clear. With that in place, the Windows close unhooks the status callback and shuts the request handle, so a completion raised later can't write through the response after it's freed. It stops short of a full cancel: a callback already running on another thread isn't waited for, which needs the HANDLE_CLOSING handshake. On Apple, invalidateAndCancel returns before the session has finished with its delegate, so clear the delegate's pointer back to the response first and have didReceiveData check it, the way didCompleteWithError already did. Android is still the only backend that genuinely cancels and waits, so naett.h now says a response should be complete before it's closed rather than leaving that to be discovered. --- ext/naett-lib/README-ppsspp.md | 12 ++++++++++++ ext/naett-lib/naett.h | 6 ++++++ ext/naett-lib/src/naett_core.c | 4 +++- ext/naett-lib/src/naett_osx.c | 20 +++++++++++++++++++- ext/naett-lib/src/naett_win.c | 14 ++++++++++++++ 5 files changed, 54 insertions(+), 2 deletions(-) diff --git a/ext/naett-lib/README-ppsspp.md b/ext/naett-lib/README-ppsspp.md index 4178b678a4..32bd5fa89b 100644 --- a/ext/naett-lib/README-ppsspp.md +++ b/ext/naett-lib/README-ppsspp.md @@ -88,3 +88,15 @@ Keep this list up to date - it's what makes it possible to move to a newer upstr - `src/naett_android.c`: `GetMethodID` returns NULL for a method it can't find, and calling with a NULL `jmethodID` aborts the VM; the header loop could also hand `GetStringUTFChars` a null value for a header with no entries. +- `src/naett_core.c`: `naettClose` cleared `res->request` before calling the backend's close, + which is the one thing the backend needs - the WinHTTP handles hang off the request. The + backend goes first now. +- `naett.h`: documented that a response should be complete before it's closed. Only the Android + backend really cancels and waits. +- `src/naett_win.c`: `naettPlatformCloseResponse` was empty, so the status callback kept the + freed response as its context. Unhooks the callback and closes the request handle. Not a full + cancel - a callback already running isn't waited for, which would need the + `WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING` handshake. +- `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). diff --git a/ext/naett-lib/naett.h b/ext/naett-lib/naett.h index b79130db3b..d49614d0b1 100644 --- a/ext/naett-lib/naett.h +++ b/ext/naett-lib/naett.h @@ -131,6 +131,12 @@ naettReq* naettGetRequest(naettRes* response); /** * @brief Closes a response object. + * + * PPSSPP: the response should be complete (see `naettComplete`) before closing it. Only the + * Android backend actually cancels an in-flight request and waits for its worker; the others + * stop what they can and return, so a transfer already inside a callback on another thread can + * still be running as this returns. Closing an incomplete response is therefore not safe in + * general - wait for it, or let it leak, which is what we do on shutdown. */ void naettClose(naettRes* response); diff --git a/ext/naett-lib/src/naett_core.c b/ext/naett-lib/src/naett_core.c index bc6e2e24e1..71f5cdaf7a 100644 --- a/ext/naett-lib/src/naett_core.c +++ b/ext/naett-lib/src/naett_core.c @@ -416,8 +416,10 @@ void naettClose(naettRes* response) { assert(response != NULL); InternalResponse* res = (InternalResponse*)response; - res->request = NULL; + // PPSSPP: the backends need the request to shut a response down - it owns the handles on + // Windows, for one - so clear it after they've had their turn, not before. naettPlatformCloseResponse(res); + res->request = NULL; KVLink* node = res->headers; freeKVList(node); free(res->body.data); diff --git a/ext/naett-lib/src/naett_osx.c b/ext/naett-lib/src/naett_osx.c index 24eb7c8ae3..1692b0c09e 100644 --- a/ext/naett-lib/src/naett_osx.c +++ b/ext/naett-lib/src/naett_osx.c @@ -98,6 +98,12 @@ void didReceiveData(id self, SEL _sel, id session, id dataTask, id data) { id p = pool(); 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) { + release(p); + return; + } if (res->headers == NULL) { id response = objc_msgSend_t(id)(dataTask, sel("response")); @@ -139,10 +145,15 @@ void didReceiveData(id self, SEL _sel, id session, id dataTask, id data) { } } + if (res->request == NULL) { + release(p); + return; + } + 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, length, res->request->options.bodyWriterData); + res->request->options.bodyWriter(bytes, (int)length, res->request->options.bodyWriterData); res->totalBytesRead += (int)length; release(p); @@ -217,6 +228,13 @@ void naettPlatformCloseResponse(InternalResponse* res) { if (res->session == nil) { return; } + // PPSSPP: invalidateAndCancel returns before the session is done with its delegate, so clear + // the back pointer the delegate holds - a callback that lands afterwards then sees NULL + // rather than a freed response. + id delegate = objc_msgSend_t(id)(res->session, sel("delegate")); + if (delegate != nil) { + object_setInstanceVariable(delegate, "response", NULL); + } objc_msgSend_void(res->session, sel("invalidateAndCancel")); release(res->session); res->session = nil; diff --git a/ext/naett-lib/src/naett_win.c b/ext/naett-lib/src/naett_win.c index 57e1f7858a..c80b157150 100644 --- a/ext/naett-lib/src/naett_win.c +++ b/ext/naett-lib/src/naett_win.c @@ -365,6 +365,20 @@ void naettPlatformFreeRequest(InternalRequest* req) { } void naettPlatformCloseResponse(InternalResponse* res) { + // PPSSPP: this used to be empty. The status callback carries the response as its context, so + // once it's freed any further completion writes through a dangling pointer. Unhook the + // callback and close the request handle, which stops new ones being raised. + // + // This is not a full cancel: a callback already running on another thread isn't waited for, + // which would need the WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING handshake. Closing a response + // that hasn't completed still isn't supported here - see naettClose in naett.h. + InternalRequest* req = res->request; + if (req == NULL || req->request == NULL) { + return; + } + WinHttpSetStatusCallback(req->request, NULL, WINHTTP_CALLBACK_FLAG_ALL_NOTIFICATIONS, 0); + WinHttpCloseHandle(req->request); + req->request = NULL; } #endif // __WINDOWS__