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.
This commit is contained in:
Henrik Rydgård committed 2026-09-05 13:57:31 -06:00
1 parent 618bdc2d72
commit 1041c976c6
2 files changed
+16 -15

No files matched your search

+8 -10
View File
@@ -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<bool> 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<std::shared_ptr<NaettBodySink>> *g_abandonedSinks = new std::vector<std::shared_ptr<NaettBodySink>>();
static std::vector<std::unique_ptr<NaettBodySink>> *g_abandonedSinks = new std::vector<std::unique_ptr<NaettBodySink>>();
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<NaettBodySink>();
sink_ = std::make_unique<NaettBodySink>();
// 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) {
+8 -5
View File
@@ -1,7 +1,8 @@
#pragma once
#include <thread>
#include <memory>
#include <string_view>
#include <thread>
#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<struct NaettBodySink> 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<NaettBodySink> sink_;
// Naett state
naettReq *req_ = nullptr;