TASK-032 review cleanup: address 5 majors + 18 minors from unworked review

Major fixes (all 5 addressed):
- Replaced bare catch(...) on unregister_path with catch(invalid_argument) +
  catch(exception) with stderr logging — surface unexpected exceptions.
- Added 5 ms inter-request sleep to curl client loop — rate-limits to
  ~200 req/s under TSan, keeping wall-clock within CI budget.
- Added null check after curl_easy_init() — skip thread gracefully on
  resource exhaustion rather than crash.
- Added early return after fork() failure (LT_CHECK(child >= 0)) —
  prevents waitpid(-1,...) from reaping unrelated processes.
- TASK-032.md status already Done; assertions already individual (TASK-052).

Minor fixes (cosmetic + docs):
- Added kDynSlots, kIpRange, kExitCodeStopReturned, kExitCodeCurlCompleted
  named constants; replaced all hex magic literals (0x7, 0xff, 42, 43).
- Reduced CURLOPT_TIMEOUT_MS in stop-from-handler child to 3000L
  (< parent's 5 s window) so timeout ordering is consistent.
- Renamed deadline → child_deadline in stop_from_handler test.
- Replaced std::string(run) with std::string_view(run) for env-var check.
- Added upper bound (v <= 3600) to stress_seconds() (CWE-1284).
- Added comment documenting why register_prefix is intentionally excluded.
- Added @see stop() cross-reference to ~webserver() Doxygen.
- Updated specs/architecture/09-testing.md §9 item 6 to use v2 API names.
- Corrected DR-008.md Verification: exit codes are regression sentinels,
  not positive observations; WIFSIGNALED paths are the positive observations.
- Added ~webserver() note to DR-008.md Verification section.
- Added PRD-CON-REQ-001/002 EARS concurrency requirements to product_specs.md.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: etr

specs/architecture/09-testing.md

@@ -7,6 +7,6 @@ The architecture itself does not prescribe test frameworks (out of architecture
 3. **Move semantics on `http_response`** (DR-5): sanitizer-clean tests for inline↔inline, inline↔heap, heap↔inline, heap↔heap on both move-construct and move-assign.
 4. **SBO size invariant** (DR-5): `static_assert(sizeof(detail::deferred_body) <= http_response::body_buf_size, ...)` at the end of `detail/body.hpp`. Compile-time guarantee.
 5. **Routing semantics preservation** (DR-7): the v1 routing-test corpus runs against v2.0 unchanged. Any regression is treated as a release-blocker.
-6. **Thread-safety contract** (DR-8): a stress test exercises `register_resource` / `block_ip` from within handlers, verifies no deadlock except for the documented `stop()` case.
+6. **Thread-safety contract** (DR-8): a stress test exercises `register_path` / `unregister_path` / `block_ip` / `unblock_ip` from within handlers, verifies no deadlock except for the documented `stop()` case. An opt-in negative test (`stop_from_handler_deadlocks_as_documented`, enabled via `HTTPSERVER_RUN_STOP_FROM_HANDLER=1`) confirms the deadlock contract by forking a child that calls `stop()` from inside a handler.
 
 ---

specs/architecture/11-decisions/DR-008.md

@@ -22,6 +22,6 @@
 **Verification (TASK-032):**
 - `test/integ/threadsafety_stress.cpp` — stress test binary `threadsafety_stress` runs 16 concurrent curl clients for 60 seconds (override via `HTTPSERVER_STRESS_SECONDS=N`), each request randomly invoking `register_path`, `unregister_path`, `block_ip`, and `unblock_ip` against the running `webserver` from inside a handler thread. A duplicate-registration from a handler throws `std::invalid_argument` rather than causing a data race.
 - CI coverage: the `build-type: tsan` matrix entry in `.github/workflows/verify-build.yml` compiles with `-fsanitize=thread` and runs `make check`, which automatically picks up `threadsafety_stress` as a registered `check_PROGRAMS` entry — no separate workflow wiring is needed.
-- Opt-in negative test `stop_from_handler_deadlocks_as_documented` (enabled via `HTTPSERVER_RUN_STOP_FROM_HANDLER=1`) forks a child process that calls `stop()` from inside a handler. A non-zero child exit (libmicrohttpd self-join abort, exit code 42) or a 5-second parent timeout (exit code 43) both count as positive observation of the documented deadlock contract. These exit codes serve as regression sentinels.
+- Opt-in negative test `stop_from_handler_deadlocks_as_documented` (enabled via `HTTPSERVER_RUN_STOP_FROM_HANDLER=1`) forks a child process that calls `stop()` from inside a handler. Positive observations of the contract are signal-terminated (libmicrohttpd self-join abort, `WIFSIGNALED`) or SIGKILL-from-parent after a 5-second timeout (silent deadlock, also `WIFSIGNALED`). Two sentinel exit codes mark regressions: the unreachable-after-`stop()` line inside the child (`stop()` returned from a handler — regression) and the post-curl line (`stop()` did not block at all — regression). A zero child exit means `stop()` returned cleanly, which is also a regression against DR-008. `~webserver()` carries the same threading constraint as `stop()` but is not separately exercised because destructor invocation from a running handler is inherently process-unsafe; the `stop()` test is considered representative.
 
 ---

specs/product_specs.md

@@ -36,6 +36,9 @@
 - **Hot-path performance:** Per-request getters shall not allocate or copy containers; they return `const&` or `string_view`.
 - **Naming:** All public method names shall be snake_case; one canonical verb per concept.
 - **Documentation:** v2.0 ships with a rewritten `README` and an updated examples set. A short `RELEASE_NOTES.md` summarizes the API changes for users porting from v1; it is informational, not a compatibility commitment.
+- **Concurrency (DR-008):**
+  - `PRD-CON-REQ-001` When a caller invokes any `webserver` public method from a handler thread (except `stop()` or `~webserver()`), the system shall complete the call without deadlock or data race.
+  - `PRD-CON-REQ-002` When a caller invokes `stop()` or `~webserver()` from inside a handler thread, the system shall either deadlock (join waits indefinitely) or abort (libmicrohttpd self-join detection); it shall not return successfully.
 
 ---
 

src/httpserver/webserver.hpp

@@ -162,6 +162,8 @@ class webserver {
       * Destroy the webserver from the thread that constructed it.
       *
       * See specs/architecture/11-decisions/DR-008.md and §5.1.
+      *
+      * @see stop() for the threading constraints that apply equally here.
      **/
      ~webserver();
      // PIMPL-owned: copy/move would slice the backing impl object.

test/integ/threadsafety_stress.cpp

@@ -81,6 +81,16 @@ namespace ht = httpserver;
 
 namespace {
 
+// Named constants — replaces hex magic literals throughout.
+// kDynSlots: number of competing dynamic path slots (slot = i & (kDynSlots-1)).
+// kIpRange: number of distinct test IPs (ip suffix = i & (kIpRange-1)).
+// kExitCodeStopReturned: child exit when stop() returns from a handler (regression).
+// kExitCodeCurlCompleted: child exit when curl completes without abort (regression).
+constexpr int kDynSlots = 8;
+constexpr int kIpRange  = 256;
+constexpr int kExitCodeStopReturned  = 42;  // stop() returned — should not happen
+constexpr int kExitCodeCurlCompleted = 43;  // curl completed — stop() did not block
+
 // Discard libcurl response bodies — we only care about completing the
 // round-trip, not the body content.
 size_t discard_write(char* /*ptr*/, size_t size, size_t nmemb,
@@ -206,15 +216,19 @@ ht::http_response driver_body(const ht::http_request& req,
         // exercise.
     }
 
-    const int slot = i & 0x7;
+    const int slot = i & (kDynSlots - 1);
     const std::string dyn_path =
         "/dyn/" + std::to_string(slot);
     const std::string ip =
-        "198.51.100." + std::to_string(i & 0xff);
+        "198.51.100." + std::to_string(i & (kIpRange - 1));
 
     // Six ops total: 0..3 are the TASK-032 route-table / ban-list
     // mutators; 4..5 are TASK-052's hook bus churn. `op % 6` keeps
     // the distribution roughly uniform.
+    // NOTE: register_prefix / unregister_prefix are intentionally not
+    // exercised here because they share the same lock path as
+    // register_path / unregister_path (register_impl_ with family=true
+    // vs false); the existing cases already cover the mutex contention.
     switch (op % 6) {
         case 0:
             try {
@@ -231,7 +245,13 @@ ht::http_response driver_body(const ht::http_request& req,
                 ws->unregister_path(dyn_path);
                 counters->unregister_ok.fetch_add(
                     1, std::memory_order_relaxed);
-            } catch (...) {
+            } catch (const std::invalid_argument&) {
+                // Path not registered yet — expected race, not a bug.
+            } catch (const std::exception& e) {
+                // Surface unexpected exceptions so they are visible in
+                // test logs (e.g. bad_alloc, logic_error).
+                std::cerr << "[stress] unexpected unregister_path exception: "
+                          << e.what() << '\n';
             }
             break;
         case 2:
@@ -290,11 +310,12 @@ ht::http_response driver_body(const ht::http_request& req,
 
 // Stress duration: default 60 s (acceptance criterion), overridable
 // via HTTPSERVER_STRESS_SECONDS for fast local iteration.
+// Capped at 3600 s to prevent runaway in CI (CWE-1284).
 int stress_seconds() {
     if (const char* s = std::getenv("HTTPSERVER_STRESS_SECONDS")) {
         try {
             int v = std::stoi(s);
-            if (v > 0) return v;
+            if (v > 0 && v <= 3600) return v;
         } catch (...) {
         }
     }
@@ -349,6 +370,7 @@ LT_BEGIN_AUTO_TEST(threadsafety_stress_suite,
             // Per-thread curl handle: curl_easy_* is per-handle
             // thread-safe; each thread owns its handle.
             CURL* curl = curl_easy_init();
+            if (!curl) return;  // resource exhaustion — skip this thread
             curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, discard_write);
             // 198.51.100.0/24 is TEST-NET-2 (RFC 5737) — block/unblock
             // ops on these IPs cannot blacklist the loopback driver
@@ -361,12 +383,17 @@ LT_BEGIN_AUTO_TEST(threadsafety_stress_suite,
             while (!stop.load(std::memory_order_relaxed)) {
                 // Six ops: 0..3 route-table/ban; 4..5 hook bus (TASK-052).
                 const int op = static_cast<int>(rng() % 6u);
-                const int i = static_cast<int>(rng() & 0xff);
+                const int i = static_cast<int>(rng() & (kIpRange - 1));
                 const std::string url =
                     base + "?op=" + std::to_string(op) +
                     "&i=" + std::to_string(i);
                 curl_easy_setopt(curl, CURLOPT_URL, url.c_str());
                 curl_easy_perform(curl);
+                // 5 ms inter-request sleep to rate-limit each thread to
+                // ~200 req/s. Under TSan (5–20× slower) this keeps total
+                // lock pressure and shadow-memory churn within the CI
+                // wall-clock budget without reducing lock-path coverage.
+                std::this_thread::sleep_for(std::chrono::milliseconds(5));
             }
             curl_easy_cleanup(curl);
         });
@@ -415,7 +442,7 @@ LT_BEGIN_AUTO_TEST(threadsafety_stress_suite,
     // and is opt-in because reproducing the deadlock requires _Exit()
     // to escape the wedged process.
     const char* run = std::getenv("HTTPSERVER_RUN_STOP_FROM_HANDLER");
-    if (run == nullptr || std::string(run) != "1") {
+    if (run == nullptr || std::string_view(run) != "1") {
         std::cout << "[SKIP] stop_from_handler_deadlocks_as_documented"
                      " — set HTTPSERVER_RUN_STOP_FROM_HANDLER=1 to run\n";
         return;
@@ -442,6 +469,7 @@ LT_BEGIN_AUTO_TEST(threadsafety_stress_suite,
     // a handler, contradicting the documented contract.
     const pid_t child = fork();
     LT_CHECK(child >= 0);
+    if (child < 0) return;  // fork failed; waitpid(-1,...) would reap unrelated processes
     if (child == 0) {
         // Child: trigger the contract violation. We do not care about
         // the child's stdout — silence it so the test log stays
@@ -459,9 +487,9 @@ LT_BEGIN_AUTO_TEST(threadsafety_stress_suite,
             // thread → DR-008 unsafe path.
             ws.stop();
             // Below is unreachable. If reached, the contract is
-            // broken — exit with a distinctive code (42) so the
-            // parent can flag the regression.
-            std::_Exit(42);
+            // broken — exit with a distinctive code so the parent
+            // can flag the regression.
+            std::_Exit(kExitCodeStopReturned);
             return ht::http_response::string("unreachable");
         });
 
@@ -473,26 +501,28 @@ LT_BEGIN_AUTO_TEST(threadsafety_stress_suite,
         CURL* curl = curl_easy_init();
         curl_easy_setopt(curl, CURLOPT_URL, url.c_str());
         curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, discard_write);
-        curl_easy_setopt(curl, CURLOPT_TIMEOUT_MS, 10000L);
+        // 3 s < parent's 5 s deadline: curl's window expires before the
+        // parent SIGKILLs the child, keeping the two timeouts ordered.
+        curl_easy_setopt(curl, CURLOPT_TIMEOUT_MS, 3000L);
         curl_easy_perform(curl);
         curl_easy_cleanup(curl);
 
         // If we ever reach here, stop()-from-handler did NOT abort
-        // or deadlock as documented → regression. Exit 43 so the
-        // parent flags it distinctly from the unreachable-after-
-        // stop() case above.
-        std::_Exit(43);
+        // or deadlock as documented → regression. Use a distinct
+        // sentinel so the parent can flag it separately from the
+        // unreachable-after-stop() case above.
+        std::_Exit(kExitCodeCurlCompleted);
     }
 
     // Parent: bounded wait on the child. SIGKILL it after 5 s if it
     // is still running (the silent-deadlock branch of DR-008). Any
     // outcome OTHER than a zero exit is a positive observation of
-    // the contract; a zero exit (or codes 42/43) is a regression.
+    // the contract; a zero exit (or sentinel codes) is a regression.
     int status = 0;
-    auto deadline =
+    auto child_deadline =
         std::chrono::steady_clock::now() + std::chrono::seconds(5);
     bool reaped = false;
-    while (std::chrono::steady_clock::now() < deadline) {
+    while (std::chrono::steady_clock::now() < child_deadline) {
         pid_t r = ::waitpid(child, &status, WNOHANG);
         if (r == child) {
             reaped = true;
@@ -513,9 +543,12 @@ LT_BEGIN_AUTO_TEST(threadsafety_stress_suite,
                   << " (MHD self-join detection)\n";
     } else if (WIFEXITED(status)) {
         const int code = WEXITSTATUS(status);
-        // Codes 42/43 mark regressions seeded inside the child.
-        // Zero exit means stop() returned cleanly from a handler.
-        LT_CHECK(code != 0 && code != 42 && code != 43);
+        // kExitCodeStopReturned / kExitCodeCurlCompleted mark
+        // regressions seeded inside the child. Zero exit means
+        // stop() returned cleanly from a handler.
+        LT_CHECK(code != 0 &&
+                 code != kExitCodeStopReturned &&
+                 code != kExitCodeCurlCompleted);
         std::cout << "[OK] stop_from_handler — child exited with "
                      "code " << code
                   << " (non-zero exit on contract violation)\n";