Skip to content

Commit f57312c

Browse files
etrclaude
andcommitted
TASK-016: per-connection arena for http_request_impl
Allocate http_request_impl from a std::pmr::monotonic_buffer_resource owned by connection_state so the warm path stops touching the global heap on every request. The arena is wired in via MHD's NOTIFY_CONNECTION callback (new on STARTED, delete on CLOSED) and release()-rewound by request_completed once modded_request -- and therefore the impl's destructor -- has run. A keep-alive connection reuses the same buffer for every subsequent request. Internal pmr-aware containers (header_local/footer_local/cookies_local stay default-allocated for the test path; querystring, requestor_ip, client_cert_*, unescaped_args, path_pieces, and the auth strings move to std::pmr) propagate the arena allocator through scoped construction. files_ stays default-allocated by design: file_info owns disk-side state and decoupling its lifecycle from the arena keeps that reasoning local. The public header keeps <memory_resource> hidden: the impl deleter is a small forward-declared struct holding only a function pointer, so sizeof(unique_ptr<impl, deleter>) is two pointers regardless of where the impl was allocated. The deleter dispatches between operator-delete (heap fallback / test-request path) and destructor-only (arena path). Tests: - New unit test test/unit/http_request_arena_test.cpp covers the three acceptance facts: (a) arena_.release() rewinds the bump pointer, (b) warm-path http_request_impl construction does not touch the upstream resource (custom counting upstream verifies zero allocs), (c) two consecutive impls land at the same address across release(). - Sequential make check passes except for the pre-existing create_test_request::method_uppercase failure (unrelated to TASK-016; reproduces on the baseline). - AddressSanitizer reports no use-after-free / heap-use-after-free across any of basic/threaded/http_request_arena/http_request_pimpl/ authentication/deferred under -fsanitize=address. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 707bfbc commit f57312c

8 files changed

Lines changed: 584 additions & 62 deletions

File tree

src/create_test_request.cpp

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,27 @@
2828

2929
namespace httpserver {
3030

31+
namespace {
32+
33+
// TASK-016: heap-deleter used for the test-request impl. The live-request
34+
// constructor in http_request.cpp uses an internal-linkage delete_impl_heap
35+
// of the same shape; reproducing it here avoids exposing it across TUs.
36+
// Both implementations call operator delete, matching v1 lifetime exactly.
37+
void delete_test_impl_heap(detail::http_request_impl* p) noexcept {
38+
delete p;
39+
}
40+
41+
} // namespace
42+
3143
http_request create_test_request::build() {
3244
http_request req;
3345

3446
// Allocate an impl for this test request (connection_ stays null,
3547
// indicating the test-request path to all MHD-touching accessors).
36-
req.impl_ = std::make_unique<detail::http_request_impl>();
48+
// Heap-allocated; uses the heap deleter so destruction frees via
49+
// operator delete -- same lifetime as v1.
50+
req.impl_.reset(new detail::http_request_impl());
51+
req.impl_.get_deleter().fn = &delete_test_impl_heap;
3752

3853
req.set_method(_method);
3954
req.set_path(_path);
@@ -44,27 +59,42 @@ http_request create_test_request::build() {
4459
req.impl_->footers_local = std::move(_footers);
4560
req.impl_->cookies_local = std::move(_cookies);
4661

62+
// Test-request path: the impl was default-constructed (no arena), so
63+
// its pmr-aware members fall back to std::pmr::get_default_resource()
64+
// -- equivalent to plain heap allocation. Cross-allocator move is not
65+
// available, so we copy element-wise via .assign(ptr, len) /
66+
// emplace_back(view, alloc).
67+
auto args_alloc = req.impl_->unescaped_args.get_allocator();
4768
for (auto& [key, values] : _args) {
69+
auto it = req.impl_->unescaped_args.find(std::string_view(key));
70+
if (it == req.impl_->unescaped_args.end()) {
71+
std::pmr::vector<std::pmr::string> empty(args_alloc);
72+
auto inserted = req.impl_->unescaped_args.emplace(
73+
std::pmr::string(key.data(), key.size(), args_alloc),
74+
std::move(empty));
75+
it = inserted.first;
76+
}
4877
for (auto& value : values) {
49-
req.impl_->unescaped_args[key].push_back(std::move(value));
78+
it->second.emplace_back(value.data(), value.size());
5079
}
5180
}
5281
req.impl_->args_populated = true;
5382

5483
if (!_querystring.empty()) {
55-
req.impl_->querystring = std::move(_querystring);
84+
req.impl_->querystring.assign(_querystring.data(), _querystring.size());
5685
}
5786

5887
#ifdef HAVE_BAUTH
59-
req.impl_->username = std::move(_user);
60-
req.impl_->password = std::move(_pass);
88+
req.impl_->username.assign(_user.data(), _user.size());
89+
req.impl_->password.assign(_pass.data(), _pass.size());
6190
#endif // HAVE_BAUTH
6291

6392
#ifdef HAVE_DAUTH
64-
req.impl_->digested_user = std::move(_digested_user);
93+
req.impl_->digested_user.assign(_digested_user.data(),
94+
_digested_user.size());
6595
#endif // HAVE_DAUTH
6696

67-
req.impl_->requestor_ip = std::move(_requestor);
97+
req.impl_->requestor_ip.assign(_requestor.data(), _requestor.size());
6898
req.impl_->requestor_port_local = _requestor_port;
6999

70100
#ifdef HAVE_GNUTLS

0 commit comments

Comments
 (0)