mirror of
https://github.com/hrydgard/ppsspp.git
synced 2026-10-01 14:58:14 +00:00
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.
This commit is contained in:
1 parent
da2f30ad46
commit
a15e654f11
5 files changed
+54
-2
No files matched your search
@@ -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).
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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__
|
||||
Reference in new issue
Block a user