Skip to content

Commit 12bcbb8

Browse files
etrclaude
andcommitted
Merge TASK-094: extend threadsafety_stress with per-resource add_hook CAS race
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2 parents 9b86e8f + bdb6823 commit 12bcbb8

6 files changed

Lines changed: 494 additions & 20 deletions

File tree

specs/tasks/M7-v2-cleanup/TASK-070.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,15 @@
1515
- [ ] Remove the TODO comment.
1616

1717
**Dependencies:**
18-
- Blocked by: TASK-001 (C++20 floor, Done), TASK-051 (per-route hooks, Done)
18+
- Blocked by: TASK-001 (C++20 floor, Done), TASK-051 (per-route hooks, Done), libc++ P0718R2 (Apple Clang/libc++ still lacks `std::atomic<std::shared_ptr<T>>` — recheck each LLVM release; see project memory)
1919
- Blocks: None
20+
- Spun off: TASK-094 (Done) — delivers the TSan stress harness for the per-resource CAS path ahead of the migration
2021

2122
**Acceptance Criteria:**
2223
- `grep -nE 'atomic_load|atomic_store' src/http_resource.cpp` returns no matches.
2324
- `grep -nE 'Wdeprecated-declarations' src/http_resource.cpp` returns no matches.
2425
- Stress test `threadsafety_stress` extended with hook table swap remains TSan-clean.
26+
*(Satisfied out-of-band by TASK-094 — Done. TASK-094 added Sub-test D that stress-tests the legacy CAS path under TSan. When TASK-070 eventually lands, Sub-test D is the required regression net for the migrated atomic path.)*
2527
- Typecheck passes.
2628
- Tests pass.
2729

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
### TASK-094: Extend `threadsafety_stress` with per-resource `add_hook` CAS race
2+
3+
**Milestone:** M7 - v2 Cleanup
4+
**Component:** `test/integ/threadsafety_stress.cpp`, `src/http_resource.cpp`
5+
**Estimate:** S
6+
7+
**Goal:**
8+
Spun off from TASK-070, which is blocked on libc++ implementing P0718R2 (see TASK-070 "Blocker" section). One acceptance-criterion piece of TASK-070 — *"Stress test `threadsafety_stress` extended with hook table swap remains TSan-clean"* — does **not** depend on the migration and can land today. The existing stress test (`test/integ/threadsafety_stress.cpp`, comment at lines 25–30) claims to cover the `route_table_mutex_ → resource hook_table → server-wide hook_table` lock order under TSan, but in practice it only hammers `webserver::add_hook` + `hook_handle::remove` (the M5/TASK-052 server-side surface). The per-resource lazy CAS path in `src/http_resource.cpp:93-110` (`ensure_table()` — `std::atomic_load_explicit` + `std::atomic_compare_exchange_strong_explicit` on a `std::shared_ptr<detail::resource_hook_table>` field) is never driven by the test. This task closes that gap so we (a) gain regression coverage on the current legacy implementation **today** and (b) seed a ready-made stress harness for the day TASK-070 unblocks.
9+
10+
**Action Items:**
11+
- [x] Add a new stress phase to `test/integ/threadsafety_stress.cpp` that targets the lazy CAS path in `http_resource::ensure_table()` (`src/http_resource.cpp:93-110`). The phase must repeatedly: (a) construct a fresh `http_resource` subclass whose `hook_table_` is null, (b) launch N concurrent threads (≥8) that each call `http_resource::add_hook` so they race the `atomic_compare_exchange_strong_explicit` on a null slot, (c) follow with mixed add/remove load against the now-installed table to exercise concurrent registration + dispatch.
12+
- [x] Sequence the new phase alongside the existing webserver-side hook ops so a single run exercises both lock-order tiers (`webserver::add_hook``http_resource::add_hook`).
13+
- [x] Verify the phase actually drives `ensure_table()` (e.g., assert that at least one CAS loser occurs across the run — via either a debug counter behind `HTTPSERVER_TEST_INSTRUMENTATION` or by inspecting `hook_table_raw_()` from a witness thread). Acceptable if the assertion only runs in TSan/instrumented lanes.
14+
- [x] Run the extended `threadsafety_stress` under the existing TSan lane and confirm zero warnings.
15+
- [x] Update the test header comment (lines 23–34) to accurately describe the resource-side coverage that was previously claimed but not delivered.
16+
- [x] Do **not** modify `src/http_resource.cpp` semantics. The migration stays parked in TASK-070; this task is test-only (plus the optional test-only instrumentation counter, if used).
17+
18+
**Dependencies:**
19+
- Blocked by: None (the legacy CAS path is what this task stresses; it exists today).
20+
- Blocks: None. Unblocks one acceptance criterion of TASK-070 ahead of time — when TASK-070 eventually lands, this stress phase is the regression net it requires.
21+
22+
**Acceptance Criteria:**
23+
- Extended `threadsafety_stress` runs TSan-clean under the existing CI matrix (Linux libstdc++ TSan lane is authoritative; macOS lane informational).
24+
- New phase observably drives the CAS in `ensure_table()`: either an instrumented counter shows ≥1 CAS-loser across a representative run, or a documented design rationale explains why the schedule provably hits the contended-null window.
25+
- `grep -n "resource hook_table" test/integ/threadsafety_stress.cpp` returns at least one comment line that accurately describes the new coverage (replacing the previous overclaim).
26+
- `src/http_resource.cpp` is unchanged except for at most one optional `#ifdef HTTPSERVER_TEST_INSTRUMENTATION` counter increment in `ensure_table()` if the visibility approach chosen needs it.
27+
- Typecheck passes.
28+
- All tests pass.
29+
30+
**Related Requirements:** PRD §2 modern C++ NFR; PRD §5 concurrency NFR (lock-order documentation).
31+
**Related Decisions:** DR-001 (C++20); the `route_table_mutex_ → resource hook_table → server-wide hook_table` lock order documented in `specs/architecture/05-cross-cutting.md` §5.6 (recorded by TASK-051).
32+
33+
**Status:** Done

specs/tasks/M7-v2-cleanup/_index.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# M7 — v2.0 Branch Cleanup
22

33
**Status:** Draft 1
4-
**Last updated:** 2026-06-04
4+
**Last updated:** 2026-06-07
55
**Owner:** Sebastiano Merlino
66
**Inputs:** [../v2-branch-gap-audit.md](../v2-branch-gap-audit.md)
77

@@ -58,6 +58,7 @@ TASK-093).
5858
| TASK-091 | Tighten CI script soft-degradations | MED | M | Backlog |
5959
| TASK-092 | Wire `route_table_concurrency` TSan + `stop()`-from-handler into per-PR CI | MED | S | Backlog |
6060
| TASK-093 | Tighten example caveats (auth, pipe, access-log) | LOW | S | Backlog |
61+
| TASK-094 | Extend `threadsafety_stress` with per-resource `add_hook` CAS race | MED | S | Done |
6162

6263
## Dependency notes
6364

0 commit comments

Comments
 (0)