fix(review 005 Unit 1): concurrent serving loops + stub-exercising gates (G-01, G-02)

- Split dispatch() into dispatch_start() (sync prefix) + spawned
  invocation: both single-stream loops (serve_single_stream and the
  accept-side run_loop_single_stream) spawn Once invocations, Sub
  pumps, and sink response writers; only the Pub sink start stays
  inline (chunk_tx must register before the next call.published).
  Inline dispatch deadlocked same-connection nested composition: the
  read loop awaited the parent handler, which awaited a nested call
  whose response only the same read loop could resolve (resolved only
  via the 30s sweeper). Spawned handles tracked + aborted at loop exit;
  in_flight_sinks behind an Arc<parking_lot::Mutex> with guards dropped
  before awaits.
- run_loop_single_stream gains the pending-resolution arms
  (RESPONDED/COMPLETED/ERROR): the accept side previously served only
  and had no loop resolving its own outbound pendings in single-stream
  mode — the latent accept-side imported-op composition hazard is
  mechanized shut.
- Write-failure in the spawned Once path warns instead of closing the
  loop (matches the Sink arm; dying transport still surfaces via
  ConnectionClosed on the next read).
- G-02 gate: hub_handler_composes_peer_announced_op_via_nested_composition
  — announce -> consumer calls hub/compose -> hub's serving loop
  wire-dispatches it -> handler composes via ctx.env -> forwarding
  stub's nested call crosses back to the consumer. The F-05 gate
  bypassed this path entirely.
- Interleaved-directions gate: outbound_call_resolves_while_inbound_
  subscription_is_being_served — consumer serves a live Sub while a
  wire-dispatched hub handler issues an outbound call on the same
  connection.
- Both gates verified load-bearing: run against the pre-fix loop each
  reproduces the G-01 hang (no progress, bounded-timeout failure);
  post-fix both resolve in <0.2s, no sweeper evictions.

Verification: cargo test 583 / --all-features 600, clippy
(all-targets, all-features, wasm32) clean, fmt clean, doc clean.

Refs docs/reviews/005-...md (G-01, G-02; Units 2-3 open).
This commit is contained in:
glm-5.3-flash committed 2026-09-04 09:36:08 +00:00
1 parent 435ae9da2f
commit 1cbb7c6536
3 files changed
+846 -157

No files matched your search

@@ -2,7 +2,8 @@
## Status
Verified, open for remediation.
Unit 1 (G-01, G-02) remediated and verified; Units 2–3 open for
remediation. See Remediation log.
## Scope
@@ -88,6 +89,9 @@ Same scale as review 004:
## G-01 [major] — Same-connection nested composition deadlocks the serving loop; the forwarding stub's nested call resolves only via the 30s sweeper
**Status: REMEDIATED (Unit 1)** — see Remediation log; both gates
added and verified load-bearing against the pre-fix loop.
**ADR drift:** ADR-022 amendment (2026-09-03) §`op/register`: the
announced op is *"invocable via nested composition (`env.invoke`)"*;
the amendment's whole point is that the hub's handlers compose
@@ -184,6 +188,9 @@ a Sub is being served) passes without sweeper intervention.
## G-02 [major] — The F-05 acceptance gate bypasses the forwarding stub it claims to prove
**Status: REMEDIATED (Unit 1)** — the stub-exercising gate and the
interleaved-directions gate are added; see Remediation log.
**Verified:** YES. `op_register_announce_then_hub_call_routes_back_to_consumer`
(`src/channels/client.rs:1412-1543`) asserts the announce resolves and
the overlay holds the op — then calls
@@ -368,7 +375,8 @@ either way.
Sequenced by dependency. All units are alkcall work; Unit 4 (alkhttp
wiring) stays downstream and should **not** start before Unit 1 —
alkhttp's serving consumers would compose over the same connection
and hit G-01 immediately.
and hit G-01 immediately. (Unit 1 landed — see Remediation log;
Units 2–3 remain.)
## Unit 1 — Concurrent serving loop + a stub-exercising gate (G-01, G-02)
@@ -407,6 +415,140 @@ and hit G-01 immediately.
---
# Remediation log
## Unit 1 — Concurrent serving loop + stub-exercising gates (G-01, G-02) — LANDED
**Fix shape.** `dispatch()` was split into a synchronous start half and
an awaited invocation:
- `Dispatcher::dispatch_start`
(`src/protocol/dispatch.rs`) runs the sync prefix (identity
resolution, root context, op-type branch) and returns a
`StartedDispatch`: `Once` (the invocation as a boxed future), `Stream`
(the `ResponseStream`, returned synchronously — `invoke_streaming`
is a sync call), or `Sink` (started **inline**, unchanged). `dispatch()`
is now a thin wrapper (`Once` = await the boxed future) and keeps its
shape for `dispatch_requested`/`handle_stream`/the gateway.
- The Sink start stays inline **by design**: its `chunk_tx` must be in
`in_flight_sinks` before the next `call.published` frame can be
routed; the loop insert cannot race the feed. Pub handlers that
block forever inside their sink future are a separate (pre-existing,
unreported) shape — the read loop no longer waits on any handler
except for the few instructions of the sink start itself.
- Both single-stream loops' `EVENT_REQUESTED` arms spawn the Once
invocation (`spawn_once_dispatch`), the Sub pump
(`spawn_stream_pump`), and the sink's response writer
(`spawn_sink_response_writer`); handles are tracked in a
`spawned` list and aborted at loop exit (teardown: sink-map clear →
spawn aborts → `fail_all` → sweeper abort). `in_flight_sinks` moved
behind `Arc<parking_lot::Mutex>` because the ABORTED/PUBLISHED/ERROR
arms must not hold the guard across the `chunk_tx.send().await`
(non-`Send` guard across an await — the reason the pre-fix loop
couldn't just be `tokio::spawn`ed piecemeal). Guard-dropping
(`let entry = ...remove()` before the await) keeps lock discipline:
no lock is held across an await anywhere in the loops.
- **`run_loop_single_stream` (the accept side) got the
pending-resolution arms** (`EVENT_RESPONDED`/`COMPLETED`/`ERROR` →
the outbound pending map). Previously it served only — an accept-side
nested-composing handler (e.g. a `from_call` imported-op stub riding
the same connection) had no loop resolving its response frames at
all. The G-01 accept-side latent hazard is mechanized shut, not just
unblocked: both single-stream loops are now the same shape
(dispatch-spawn + pending-resolution), the full-duplex loop
`serve_single_stream` composes them and is unchanged in its arm
semantics (frame-arm equivalence preserved per the non-findings
audit).
- Write-failure semantics changed from `break` (close the loop) to
`warn` (keep reading) in the spawned Once path, matching the Sink
arm: a dying transport surfaces on the next read as
`ConnectionClosed`; a transient frame-write failure no longer tears
down every in-flight request on the connection.
**Gates (G-02, productized from the removed probe):**
- `hub_handler_composes_peer_announced_op_via_nested_composition`
(`src/channels/client.rs`): announce → consumer calls `hub/compose`
→ the hub's serving loop wire-dispatches it → the handler resolves
`consumer/exec` via `ctx.env` nested composition → the forwarding
stub's nested `call.requested` crosses back to the consumer. This is
the stub path the F-05 gate bypassed. Two wiring facts surfaced (both
recorded as spec-consistent, both were silent before): (a) the hub's
channel-0 `Connection` must carry the peer identity —
`compose_root_env` attaches the connection overlay keyed by
`identity.id` (ADR-030 §5), and a deployment that skips identity
resolution silently gets no peer overlay (the test sets it via the
`AuthContext` the adapter passes to `install_channel_zero`);
(b) composition reachability is declared on the composing handler's
registration (`scoped_env: ScopedPeerEnv::new(["consumer/exec"])`) —
the empty `ScopedPeerEnv` is deny-by-default in
`PeerCompositeEnv::invoke_with_policy`, so a wire-dispatched handler
with no `scoped_env` composes nothing (the reachability gate, not the
overlay, was what the probe's first draft tripped over).
- `outbound_call_resolves_while_inbound_subscription_is_being_served`
(the optional interleaved-directions phase on the F-04 shape): the
consumer subscribes to the hub (Sub served by the consumer's loop),
then calls `hub/interleave` over the wire — its handler issues an
outbound hub→consumer call on the same connection while the Sub is
live — and asserts the call resolves and the Sub is still live
after. Both gates bounded at 5s (vs. the 30s sweeper), so a
regression fails fast, not via sweeper eviction.
**Load-bearing verification (both gates, empirical):** each gate was
run against the pre-fix loop (`git stash push
src/protocol/dispatch.rs`) and reproduced the G-01 hang:
```
hub_handler_composes…: panicked "nested composition through the
forwarding stub timed out (G-01 shape): Elapsed(())" — 5.01s, no progress
outbound_call_resolves…: panicked "interleaved outbound call starved
while Sub served (G-01 shape): Elapsed(())" — 5.06s, no progress
```
With the fix, both resolve (0.01s / 0.11s wall including connection
setup). No sweeper evictions (bounded timeouts ≪ 30s).
**Consequences audited against the fix:**
- Same-connection nested composition of peer-announced ops works for
wire-dispatched parents (the flagship ADR-022 amendment flow) — the
gate proves it end-to-end.
- The accept-side imported-op composition hazard (G-01's second
consequence, latent pre-`f84d214`) is mechanized shut by the
pending-resolution arms in `run_loop_single_stream` — not merely
unblocked. There is no dedicated gate for this shape yet (the
accept-side import + nested-compose e2e would be a further gate;
noted as residual work, not a regression).
- Head-of-line blocking is gone: spawned arms proceed concurrently;
the loop never awaits a handler.
- Frame ordering: per-request ordering is preserved by the
`SharedFrameWriter` (each `write_frame` is atomic under the mutex);
responses for two requests may now interleave *at frame
granularity*, which single-stream mode always permitted (both
directions' frames are multiplexed by design; correlation is by id).
**Verification (post-fix):**
```
cargo test → 583 passed, 0 failed
cargo test --all-features → 600 passed, 0 failed
cargo clippy --all-targets -- -D warnings → clean
cargo clippy --all-features --all-targets -- -D warnings → clean
cargo fmt --check → clean (fmt applied)
cargo clippy --target wasm32-unknown-unknown -- -D warnings → clean
cargo doc --no-deps → clean
```
**Residual (not blocking Unit 1):** the sink-start-inline shape means a
`Pub` op whose registration itself blocks (ACL/schema compile are sync
and fast; `resolve_sink_handler` runs the handler's ACL path) still
holds the loop briefly — bounded by registry work, not handler work. A
malicious `AccessControl::check` implementation could stall the loop;
that is a deployment-provided trait object, the same trust boundary as
`IdentityProvider`, and unchanged from the pre-existing shape.
---
## Verification log (this pass)
- All gates in Baseline verification reproduced at tree `f84d214`