perf: drop per-request weak_ptr lock() in gated hook-fire helpers

The three tail-end hook-fire helpers each locked the request's
resource_weak_ BEFORE checking their any_hooks_ gate, so every matched
request paid ~8 atomic RMWs on the resource's shared control block (3x
weak_ptr::lock() CAS + 3x shared_ptr destruction, plus the weak assign
/destroy) even with zero hooks registered -- the common case, and worse
under thread-pool load where all workers hammer the same cache line.
Master's dispatch used raw http_resource* with no refcount traffic; this
also contradicted TASK-051's plan ("one pointer null-check per phase").

fire_after_handler_gated and fire_response_sent_gated both fire within
finalize_answer's scope, where the owning shared_ptr (hrm) is still
alive, so they now take the resolved http_resource* directly and read
hook_table_raw_() from it -- no lock(). The resource is threaded through
materialize_and_queue_response for the response_sent site (nullptr on the
skip-handler / 404 paths, which both helpers already tolerate).

fire_request_completed_gated genuinely fires from the MHD completion
callback, after that shared_ptr is gone, so it keeps the weak_ptr lock()
-- but gated behind a new modded_request::route_has_hook_table_ snapshot
(taken in finalize_answer). When no per-route hook table existed at
dispatch and no server-wide request_completed hook is registered, the
lock is skipped entirely; when the snapshot is set it still locks and
runs the precise any_hooks(request_completed) check, preserving the
unregistration-skip contract.

Net on the zero-per-route-hook path: from ~8 control-block atomics per
matched request down to the 2 (resource_weak_ assign + destroy) that the
handler_exception path independently requires. No behavior change beyond
the same same-request-registration race the relaxed any_hooks_ gates
already tolerate. Full suite green (109/109).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NpysYDDJac63yz2mZKKiDf

Author: etr

src/detail/webserver_finalize.cpp

@@ -135,10 +135,13 @@ bool webserver_impl::fire_before_handler_gated(
 // Per §4.10: after_handler fires only when a handler conceptually ran.
 // The pre-handler short-circuit branch (mr->skip_handler) is handled
 // upstream in finalize_answer and never reaches this site.
-void webserver_impl::fire_after_handler_gated(detail::modded_request* mr) {
+void webserver_impl::fire_after_handler_gated(detail::modded_request* mr,
+                                              http_resource* resource) {
     const bool server_gate = has_hooks_for(hook_phase::after_handler);
-    std::shared_ptr<http_resource> res;
-    auto* rtable = per_route_table(mr, res);
+    // resource is borrowed from finalize_answer's live shared_ptr; no
+    // weak_ptr lock() needed (this fires within finalize_answer's scope).
+    // nullptr on the 404 path -- no per-route table then.
+    auto* rtable = resource != nullptr ? resource->hook_table_raw_() : nullptr;
     const bool route_gate = rtable != nullptr &&
         rtable->any_hooks(hook_phase::after_handler);
 
@@ -175,10 +178,14 @@ void webserver_impl::fire_after_handler_gated(detail::modded_request* mr) {
 // webserver_impl::request_completed). bytes_queued is the logical body
 // size from http_response::body_->size(); for deferred/pipe bodies this
 // is 0 and consumers should fall back to the Content-Length header.
-void webserver_impl::fire_response_sent_gated(detail::modded_request* mr) {
+void webserver_impl::fire_response_sent_gated(detail::modded_request* mr,
+                                              http_resource* resource) {
     const bool server_gate = has_hooks_for(hook_phase::response_sent);
-    std::shared_ptr<http_resource> res;
-    auto* rtable = per_route_table(mr, res);
+    // resource is borrowed from finalize_answer's live shared_ptr (this
+    // fires from materialize_and_queue_response, still within that scope),
+    // so read the per-route table directly instead of locking the
+    // weak_ptr. nullptr on the skip_handler / 404 paths.
+    auto* rtable = resource != nullptr ? resource->hook_table_raw_() : nullptr;
     const bool route_gate = rtable != nullptr &&
         rtable->any_hooks(hook_phase::response_sent);
 
@@ -226,12 +233,24 @@ void webserver_impl::fire_request_completed_gated(
         enum MHD_RequestTerminationCode toe) {
     const bool server_gate = has_hooks_for(hook_phase::request_completed);
 
-    // TASK-051: per-route gate. per_route_table() returns null if the
-    // resource was unregistered between dispatch and completion (per the
-    // action-item contract: skip the per-route chain in that case).
-    // res keeps the resource alive while rtable is in use.
+    // TASK-051: per-route gate. Unlike after_handler/response_sent, this
+    // fires from the MHD completion callback -- finalize_answer's owning
+    // shared_ptr is gone, so we must lock() the weak_ptr to keep the
+    // resource alive while rtable is in use. per_route_table() returns
+    // null if the resource was unregistered between dispatch and
+    // completion (action-item contract: skip the per-route chain).
+    //
+    // But skip the lock() entirely on the common path: mr->route_has_hook_table_
+    // is a snapshot (taken in finalize_answer) of whether the resource
+    // carried any per-route hook table. When it's false and no server-wide
+    // request_completed hook is registered, no control-block atomics are
+    // touched. When it's true we still lock() and run the precise
+    // any_hooks(request_completed) check, preserving the skip contract.
     std::shared_ptr<http_resource> res;
-    auto* rtable = per_route_table(mr, res);
+    resource_hook_table* rtable = nullptr;
+    if (server_gate || mr->route_has_hook_table_) {
+        rtable = per_route_table(mr, res);
+    }
     const bool per_route_present = rtable != nullptr &&
         rtable->any_hooks(hook_phase::request_completed);
 

src/detail/webserver_request.cpp

@@ -270,7 +270,8 @@ MHD_Result webserver_impl::finalize_answer(MHD_Connection* connection,
     // NOTE: after_handler is NOT fired on this path (no handler ran);
     // response_sent fires unconditionally in materialize_and_queue_response.
     if (mr->skip_handler) {
-        return materialize_and_queue_response(connection, mr);
+        // No resource was resolved on this path -- pass nullptr.
+        return materialize_and_queue_response(connection, mr, nullptr);
     }
 
     // TASK-023: hold a shared_ptr copy across dispatch. If a concurrent
@@ -289,6 +290,13 @@ MHD_Result webserver_impl::finalize_answer(MHD_Connection* connection,
     // the resource directly.
     if (found) {
         mr->resource_weak_ = hrm;
+        // Snapshot whether this resource carries a per-route hook table,
+        // so fire_request_completed_gated (which fires after this
+        // shared_ptr is gone) can gate its weak_ptr lock() on the common
+        // zero-per-route-hook path. Same acquire barrier as
+        // fire_before_handler_gated relies on (hrm came from
+        // resolve_resource_for_request under the route-table shared_lock).
+        mr->route_has_hook_table_ = (hrm->hook_table_raw_() != nullptr);
     }
 
     fire_route_resolved_gated(this, mr, found, hrm);
@@ -305,7 +313,7 @@ MHD_Result webserver_impl::finalize_answer(MHD_Connection* connection,
         // NOTE: after_handler was NOT fired on this path (before_handler
         // short-circuit); response_sent fires unconditionally in
         // materialize_and_queue_response below.
-        return materialize_and_queue_response(connection, mr);
+        return materialize_and_queue_response(connection, mr, hrm.get());
     }
 
     if (found) {
@@ -323,9 +331,11 @@ MHD_Result webserver_impl::finalize_answer(MHD_Connection* connection,
     // design choice: we fire because the dispatch site has produced a
     // response and the contract is "fires between response readiness
     // and queue", and that gives users a uniform observation point.
-    fire_after_handler_gated(mr);
+    // hrm is null on the 404 path (found == false); fire_after_handler_gated
+    // and materialize_and_queue_response both tolerate a null resource.
+    fire_after_handler_gated(mr, hrm.get());
 
-    return materialize_and_queue_response(connection, mr);
+    return materialize_and_queue_response(connection, mr, hrm.get());
 }
 
 MHD_Result webserver_impl::complete_request(MHD_Connection* connection, struct detail::modded_request* mr, const char* version, const char* method) {

src/detail/webserver_response_queue.cpp

@@ -231,7 +231,8 @@ int webserver_impl::queue_response_dispatching_kind(
 }
 
 MHD_Result webserver_impl::materialize_and_queue_response(MHD_Connection* connection,
-                                                          detail::modded_request* mr) {
+                                                          detail::modded_request* mr,
+                                                          http_resource* resource) {
     struct MHD_Response* raw_response = get_raw_response_with_fallback(mr);
     if (raw_response == nullptr) {
         // Belt-and-suspenders: even get_raw_response_with_fallback's
@@ -263,7 +264,7 @@ MHD_Result webserver_impl::materialize_and_queue_response(MHD_Connection* connec
     // queue, so destroying the MHD_Response below does not affect the
     // queued bytes -- only the http_response in mr->response matters
     // for the hook ctx pointer.
-    fire_response_sent_gated(mr);
+    fire_response_sent_gated(mr, resource);
     // MHD reference-counting note: for MHD_create_response_from_callback
     // responses (deferred/streaming), MHD increments its own internal
     // refcount during MHD_queue_response, so this MHD_destroy_response only

src/httpserver/detail/modded_request.hpp

@@ -120,17 +120,31 @@ struct modded_request {
 
     // TASK-051: weak_ptr to the resource that handled this request.
     // Populated in finalize_answer when resolve_resource_for_request
-    // returns a non-null hrm. Used by fire_response_sent_gated and
-    // fire_request_completed_gated to fire the per-route phase chain
-    // after the server-wide one. If the resource was unregistered
-    // between dispatch and completion, lock() returns null and the
-    // per-route chain is skipped (the action-item contract). The
-    // weak_ptr also keeps a control-block reference into the resource
-    // alive until ~modded_request, so the resource cannot be destroyed
-    // mid-firing -- the hot-path firing helpers lock() into a local
-    // shared_ptr before iterating.
+    // returns a non-null hrm. Used by fire_request_completed_gated (which
+    // fires from the MHD completion callback, after finalize_answer's
+    // owning shared_ptr is gone) and the handler_exception path. If the
+    // resource was unregistered between dispatch and completion, lock()
+    // returns null and the per-route chain is skipped (the action-item
+    // contract).
+    //
+    // The two in-scope firing helpers (fire_after_handler_gated /
+    // fire_response_sent_gated) do NOT lock() this weak_ptr: they run
+    // while finalize_answer's owning shared_ptr is still alive, so the
+    // resolved resource is passed to them directly. Locking there cost a
+    // control-block CAS per matched request even with zero hooks -- the
+    // gate is checked first now instead.
     std::weak_ptr<http_resource> resource_weak_{};
 
+    // Snapshot, taken in finalize_answer, of whether the resolved
+    // resource carried a per-route hook table at dispatch time. Lets
+    // fire_request_completed_gated skip the weak_ptr lock() (and its
+    // control-block atomics) on the overwhelmingly common zero-per-route-
+    // hook path: when this is false and no server-wide request_completed
+    // hook is registered, no lock is taken at all. The precise
+    // any_hooks(request_completed) check still happens after locking when
+    // this is true, preserving the unregistration-skip contract.
+    bool route_has_hook_table_ = false;
+
     modded_request() = default;
 
     modded_request(const modded_request& b) = delete;

src/httpserver/detail/webserver_impl_dispatch.hpp

@@ -224,8 +224,21 @@ void fire_request_completed(
 // TASK-050: gated-fire helpers, members so the http_response friendship
 // applies (response_sent_ctx::bytes_queued reads response.body_->size()).
 // Definitions live in src/detail/webserver_finalize.cpp.
-void fire_after_handler_gated(modded_request* mr);
-void fire_response_sent_gated(modded_request* mr);
+//
+// @p resource is the resolved resource borrowed from finalize_answer's
+// owning shared_ptr (nullptr on the 404 / short-circuit paths where no
+// resource was resolved). Passing it avoids a per-request weak_ptr
+// lock() -- and its control-block atomics -- on the zero-hook hot path;
+// the resource stays alive for the whole finalize_answer scope, which
+// both of these fire sites run within.
+void fire_after_handler_gated(modded_request* mr,
+                              ::httpserver::http_resource* resource);
+void fire_response_sent_gated(modded_request* mr,
+                              ::httpserver::http_resource* resource);
+// request_completed fires from the MHD completion callback, after
+// finalize_answer's shared_ptr is gone, so it keeps the weak_ptr lock()
+// -- but gated behind mr->route_has_hook_table_ so the lock is skipped
+// entirely on the common zero-per-route-hook path.
 void fire_request_completed_gated(modded_request* mr,
                                   enum MHD_RequestTerminationCode toe);
 
@@ -330,8 +343,14 @@ std::string serialize_allow_methods(method_set allowed) const;
 // mr->response, decorate it, queue it on the connection. Handles
 // the belt-and-suspenders fallback when get_raw_response_with_fallback
 // itself fails to produce a response.
+//
+// @p resource is the resolved resource (nullptr when none was resolved),
+// forwarded to fire_response_sent_gated so it can reach the per-route
+// hook table without a weak_ptr lock(). Borrowed for the duration of the
+// call from finalize_answer's owning shared_ptr.
 MHD_Result materialize_and_queue_response(MHD_Connection* connection,
-                                          modded_request* mr);
+                                          modded_request* mr,
+                                          ::httpserver::http_resource* resource);
 
 // TASK-062: kind-dispatched MHD queue entry-point.  For
 // body_kind::digest_challenge, branches into the auth-required