Merge TASK-188: Publish immutable peer policies without writer starvation

Author: etr

specs/architecture/v3/TASK-188-implementation-evidence.md

@@ -51,6 +51,10 @@ route/admission/lifecycle services + semantic HTTP stream
 
 Each connection has one serialized protocol-state owner; request handlers may run concurrently on configured workers. State-changing operations from handlers return to the connection owner. Shared route tables, IP policy, hooks, and TLS snapshots publish immutable generations so readers need no global hot-path lock. This is a target responsibility split, not a direct transplant of v2's MHD callback services.
 
+Peer policy implements this split with an atomic `shared_ptr<const generation>` owning both pattern vectors, enabled state, mode and derived armed state. A writer mutex serializes read-modify-publish operations against the latest generation, preventing lost independent updates. Parsing happens before serialization. Each reader retains one owned generation through both scans; no reader-held shared mutex spans the scan. Atomic shared-pointer operations are not promised to be lock-free. The scalar `armed()` acquire load remains the unused-policy gate. Writers release-publish the generation before release-storing that gate; after passing the gate, classification acquire-loads a generation and checks its own armed state, including disabled acceptance. Overlapping calls may use an older complete generation; operations synchronized after a completed mutation observe the published update.
+
+Candidate construction leaves the previous generation intact on failure. Pattern mutators retain their existing throwing allocation behavior and typed invalid-pattern outcomes. The existing `noexcept` mode/enabled setters now allocate to copy a generation: allocation failure terminates, rather than silently dropping an update. These setters already offered no recoverable failure channel for synchronization failures. Generation lifetime is bounded by the current publication and in-flight readers; the policy introduces no reclamation queue or custom hazard-pointer scheme. TASK-188's bounded progress, independent-writer and installed listener evidence pins this publication contract.
+
 ### 3.1 Public semantic interface (illustrative, not final signatures)
 
 ```cpp

specs/architecture/v3/architecture.md

@@ -9,10 +9,10 @@
 **Workflow:** Follow `../v3-release-repair-workflow.md`; resolve TASK-188 with the worktree helper, require `base_branch=v3`, root-cause investigation and verified RED/GREEN before changes, then Groundwork validation/finalization.
 
 **Action Items:**
-- [ ] Reproduce the frozen `peer_policy/concurrent_mutation_and_reads` stall. Inspect test joining/termination and `src/detail/peer_policy.cpp` shared-mutex paths; collect bounded writer-progress observations before concluding starvation, deadlock or test lifecycle error.
-- [ ] Add a bounded concurrency regression which holds reader pressure until a writer completes, asserts consistent classifications and fails on missing mutation progress. Terminate/join every thread on failure; do not solve this by weakening the number of mutations/readers, adding sleeps, raising the release deadline or making the readers stop before writers need progress.
-- [ ] Apply the proven repair. Prefer architecture's immutable generation publication with serialized writers if shared-mutex starvation is established; preserve atomic stance/list consistency, deny/allow precedence, typed invalid patterns, armed fast path and live mutation from handlers. Clear concurrency doubts with tests before committing.
-- [ ] Check concurrent add/remove/mode/enabled publication and real public listener live policy changes; retain parser/truth-table compatibility and update architecture if the implementation publication model changes.
+- [x] Reproduce the frozen `peer_policy/concurrent_mutation_and_reads` stall. Inspect test joining/termination and `src/detail/peer_policy.cpp` shared-mutex paths; collect bounded writer-progress observations before concluding starvation, deadlock or test lifecycle error.
+- [x] Add a bounded concurrency regression which holds reader pressure until a writer completes, asserts consistent classifications and fails on missing mutation progress. Terminate/join every thread on failure; do not solve this by weakening the number of mutations/readers, adding sleeps, raising the release deadline or making the readers stop before writers need progress.
+- [x] Apply the proven repair. Prefer architecture's immutable generation publication with serialized writers if shared-mutex starvation is established; preserve atomic stance/list consistency, deny/allow precedence, typed invalid patterns, armed fast path and live mutation from handlers. Clear concurrency doubts with tests before committing.
+- [x] Check concurrent add/remove/mode/enabled publication and real public listener live policy changes; retain parser/truth-table compatibility and update architecture if the implementation publication model changes.
 
 **Acceptance Criteria:**
 - Existing `peer_policy` finishes within the unchanged 180-second release deadline under normal TLS-off/on; the bounded writer-progress regression passes under normal, ASan/UBSan and TSan.
@@ -24,4 +24,6 @@
 - Blocks: TASK-189, TASK-185
 **Related Requirements:** PRD-V3N-REQ-014, PRD-V3N-REQ-025, PRD-V3N-REQ-038.
 **Related Decisions:** DR-V3-001, DR-V3-006, DR-V3-008; architecture §3 immutable policy generations.
-**Status:** Not Started
+**Status:** Complete
+
+**Implementation evidence:** [TASK-188 implementation evidence](../../architecture/v3/TASK-188-implementation-evidence.md). Six local C++20 normal/ASan/UBSan/TSan TLS-off/on lanes and installed managed/external live-policy consumers pass; Groundwork validation passed in one iteration with eight approvals, zero fixes and zero unworked findings (run `476f15d40115b153d901c612b1b6473d`). All implementation build/debug receipts are retained beneath `build/task188-evidence/implementation-2cc58b4b/` in the primary worktree, with a SHA256 archive manifest.

specs/tasks/M14-v3-release-repairs/TASK-188.md

@@ -335,7 +335,7 @@ Nominally: **13 sequential tasks**, each S–XL. Most other tasks parallelize of
 | TASK-185 | [Run final dependency, conformance, sanitizer and performance release gates](M13-v3-release/TASK-185.md) | M13 | In Progress | TASK-180, TASK-181, TASK-182, TASK-183, TASK-184, TASK-200 |
 | TASK-186 | [Repair concurrency consumer lifetime and sanitizer-owned allocation coverage](M14-v3-release-repairs/TASK-186.md) | M14 | Complete | None |
 | TASK-187 | [Repair response-source and multipart coroutine lifetimes](M14-v3-release-repairs/TASK-187.md) | M14 | Complete | TASK-186 |
-| TASK-188 | [Eliminate peer-policy mutation starvation](M14-v3-release-repairs/TASK-188.md) | M14 | Not Started | TASK-187 |
+| TASK-188 | [Eliminate peer-policy mutation starvation](M14-v3-release-repairs/TASK-188.md) | M14 | Complete | TASK-187 |
 | TASK-189 | [Repair native HTTP/1 WebSocket handshake and flood progress](M14-v3-release-repairs/TASK-189.md) | M14 | Not Started | TASK-188 |
 | TASK-190 | [Repair HTTP/2 conformance route-effect ordering](M14-v3-release-repairs/TASK-190.md) | M14 | Not Started | TASK-189 |
 | TASK-191 | [Repair independent HTTP/3 semantic drain completion](M14-v3-release-repairs/TASK-191.md) | M14 | Not Started | TASK-190 |

specs/tasks/_index.md

@@ -19,20 +19,13 @@
      02110-1301 USA
 */
 
-// TASK-119 step 2: the peer policy storage (server/peer_policy.hpp).
-// Two pattern lists under ONE shared mutex (a classify() snapshot is
-// consistent across both; v2 used one mutex per list and could observe
-// a deny from before a mutation and an allow from after it). The
-// prefix-order insert keeps each list free of covering pairs, so
-// membership is a plain scan. armed() is an atomic recomputed under
-// the same lock at every mutation, keeping the zero-cost gate
-// consistent with the store it summarizes.
+// Readers retain one immutable generation while scanning. Writers serialize
+// read-modify-publish operations; no reader-held lock spans a pattern scan.
 
 #include <atomic>
 #include <memory>
 #include <mutex>
 #include <optional>
-#include <shared_mutex>
 #include <string>
 #include <utility>
 #include <vector>
@@ -103,9 +96,10 @@ class peer_policy_impl {
                       " spelling");
         }
         {
-            std::unique_lock<std::shared_mutex> lock(mu_);
-            insert_aware(denying ? deny_ : allow_, *parsed);
-            recompute_armed_locked();
+            std::lock_guard<std::mutex> lock(writers_);
+            auto next = std::make_shared<generation>(*current_.load(std::memory_order_acquire));
+            insert_aware(denying ? next->deny : next->allow, *parsed);
+            publish(std::move(next));
         }
         return http::outcome::okay();
     }
@@ -121,78 +115,90 @@ class peer_policy_impl {
                       " spelling");
         }
         {
-            std::unique_lock<std::shared_mutex> lock(mu_);
+            std::lock_guard<std::mutex> lock(writers_);
+            auto next = std::make_shared<generation>(*current_.load(std::memory_order_acquire));
             std::vector<net::address_pattern>& list =
-                denying ? deny_ : allow_;
+                denying ? next->deny : next->allow;
             for (std::size_t i = 0; i < list.size(); ++i) {
                 if (list[i] == *parsed) {
                     list.erase(list.begin()
                                + static_cast<std::ptrdiff_t>(i));
                     break;
                 }
             }
-            recompute_armed_locked();
+            publish(std::move(next));
         }
         return http::outcome::okay();
     }
 
     void set_mode(peer_policy_mode mode) noexcept {
-        std::unique_lock<std::shared_mutex> lock(mu_);
-        mode_ = mode;
-        recompute_armed_locked();
+        std::lock_guard<std::mutex> lock(writers_);
+        auto next = std::make_shared<generation>(*current_.load(std::memory_order_acquire));
+        next->mode = mode;
+        publish(std::move(next));
     }
 
     peer_policy_mode mode() const noexcept {
-        std::shared_lock<std::shared_mutex> lock(mu_);
-        return mode_;
+        return current_.load(std::memory_order_acquire)->mode;
     }
 
     void set_enabled(bool enabled) noexcept {
-        std::unique_lock<std::shared_mutex> lock(mu_);
-        enabled_ = enabled;
-        recompute_armed_locked();
+        std::lock_guard<std::mutex> lock(writers_);
+        auto next = std::make_shared<generation>(*current_.load(std::memory_order_acquire));
+        next->enabled = enabled;
+        publish(std::move(next));
     }
 
     bool enabled() const noexcept {
-        std::shared_lock<std::shared_mutex> lock(mu_);
-        return enabled_;
+        return current_.load(std::memory_order_acquire)->enabled;
     }
 
     bool armed() const noexcept {
         return armed_.load(std::memory_order_acquire);
     }
 
-    // The consistent snapshot decision: both lists under one shared
-    // lock, then the v2 truth table.
     peer_verdict classify(const net::peer_address& peer) {
         if (!armed()) return peer_verdict{};
-        std::shared_lock<std::shared_mutex> lock(mu_);
-        const bool denied = matches_locked(deny_, peer.address);
-        const bool allowed = matches_locked(allow_, peer.address);
-        return decide(mode_, denied, allowed);
+        const auto snapshot = current_.load(std::memory_order_acquire);
+        // A concurrent disarm may have published after the gate was observed.
+        if (!snapshot->armed) return peer_verdict{};
+        const bool denied = matches(snapshot->deny, peer.address);
+        const bool allowed = matches(snapshot->allow, peer.address);
+        return decide(snapshot->mode, denied, allowed);
     }
 
  private:
-    static bool matches_locked(const std::vector<net::address_pattern>& list,
-                               const net::address& candidate) noexcept {
+    static bool matches(const std::vector<net::address_pattern>& list,
+                        const net::address& candidate) noexcept {
         for (const net::address_pattern& entry : list) {
             if (entry.matches(candidate)) return true;
         }
         return false;
     }
 
-    void recompute_armed_locked() noexcept {
-        const bool armed = enabled_
-            && (mode_ == peer_policy_mode::reject_all || !deny_.empty()
-                || !allow_.empty());
+    struct generation {
+        std::vector<net::address_pattern> deny;
+        std::vector<net::address_pattern> allow;
+        peer_policy_mode mode = peer_policy_mode::accept_all;
+        bool enabled = true;
+        bool armed = false;
+    };
+
+    void publish(std::shared_ptr<generation> next) noexcept {
+        next->armed = next->enabled
+            && (next->mode == peer_policy_mode::reject_all || !next->deny.empty()
+                || !next->allow.empty());
+        const bool armed = next->armed;
+        // Publish ownership before releasing the scalar gate. Readers arriving
+        // during publication may use an older complete generation; readers
+        // synchronized after a mutation see its gate and generation together.
+        current_.store(std::move(next), std::memory_order_release);
         armed_.store(armed, std::memory_order_release);
     }
 
-    mutable std::shared_mutex mu_;
-    std::vector<net::address_pattern> deny_;
-    std::vector<net::address_pattern> allow_;
-    peer_policy_mode mode_ = peer_policy_mode::accept_all;
-    bool enabled_ = true;
+    std::mutex writers_;
+    std::atomic<std::shared_ptr<const generation>> current_{
+        std::make_shared<const generation>()};
     std::atomic<bool> armed_{false};
 };
 

src/detail/peer_policy.cpp

@@ -22,10 +22,10 @@
 // TASK-119 (plan D5): the v3 peer policy store -- the v3 equivalent
 // of v2's deny_ip/allow_ip sets with default_policy (PRD-V3N-REQ-014,
 // DR-V3-008). One server-wide policy: two pattern lists (deny, allow)
-// under one lock, a mode (the v2 default_policy), and an enabled
-// flag. Runtime-mutable from any thread including inside handlers:
-// every member is synchronized, and classify() takes a consistent
-// snapshot of both lists.
+// in an immutable generation with a mode (the v2 default_policy) and
+// an enabled flag. Runtime-mutable from any thread including handlers:
+// writers serialize publication; classify() retains one complete generation
+// without holding a shared mutex across the scan.
 //
 // Pattern vocabulary: net::parse_pattern (exact literals, trailing-*
 // IPv4 wildcards, CIDR over both families). Pattern spellings that
@@ -53,7 +53,7 @@
 // (not_on_allow_list) and accept_all admits it.
 //
 // Zero-cost-when-unused: armed() is one atomic load, false when the
-// policy is disabled, the mode is accept_all, and both lists are
+// policy is disabled OR the mode is accept_all and both lists are
 // empty; an engine that observes !armed() may skip classify()
 // entirely (the observable verdict is the same).
 //

src/httpserver/server/peer_policy.hpp

@@ -1745,3 +1745,9 @@ endif
 
 # Native fixture dependencies must accompany their translation units in distributions.
 noinst_HEADERS += integ/raw_http_client.hpp parity/case_wire.hpp parity/v3_dispatch_replay.hpp unit/response_source_rig.hpp
+
+check_PROGRAMS += consumer_v3_peer_policy_runtime
+consumer_v3_peer_policy_runtime_SOURCES = integ/consumer_v3_peer_policy_runtime.cpp
+consumer_v3_peer_policy_runtime_CPPFLAGS = -I$(top_srcdir)/src -I$(top_srcdir)/examples
+consumer_v3_peer_policy_runtime_LDADD = $(top_builddir)/src/libhttpserver_v3core.la
+consumer_v3_peer_policy_runtime_LDFLAGS = $(V3_TLS_LDFLAGS) $(AM_LDFLAGS)