mirror of
https://github.com/hrydgard/ppsspp.git
synced 2026-11-13 09:39:33 +01:00
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.
This commit is contained in:
1 parent
b120f0004f
commit
bc095c5e31
5 files changed
+69
-3
No files matched your search
@@ -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<NaettBodySink>();
|
||||
// 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;
|
||||
|
||||
Reference in new issue
Block a user