From 96634d6c066a57197be9c3ba942ac33dae063d6c Mon Sep 17 00:00:00 2001 From: Ryan Lee Date: Fri, 14 Aug 2026 19:44:41 +0000 Subject: [PATCH 1/2] fix(server): bound the authority-call timeout by dropping the cancelled call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `remote_authority_call` raced the inner host call against a cooperative cancel token and a deadline, then ran `let _ = call.await;` on every non-success arm — re-awaiting the future it had just cancelled. When the inner future was parked on an await that never observes the token (a statement-store connect, an unacked subscribe), the timeout branch parked with it and the call returned nothing: no success, no timeout error. The dispatcher only sends a response frame once the handler resolves, so an `account_get` request was left neither answered nor refused. Drop the `select!` loser instead, matching the precedent in `bulletin_rpc::submit_preimage`. The cancel signal is still raised first, so cancel-aware inner futures and other holders of the shared token keep their cooperative path; the drop is the hard stop behind it. Restores the host-side `warn!` for an abandoned call, which used to come from `submit_remote_message`'s unwind and is skipped by the drop. Two pre-existing assertions that a timed-out SSO request unsubscribes two statement streams are removed: they only held because the re-await let the timed-out call keep working past its deadline, subscribing a second time and then tearing both down. Under the bound only one subscribe is ever sent. `sign_raw_cancellation_unsubscribes_sso_subscriptions`, which stages both subscriptions before firing the token, still covers unsubscribe-on-drop. Tests observed failing against the unfixed code (each on the `wait_until` hang assertion) before the fix landed, and again on a deliberate revert. Closes #405 --- rust/crates/truapi-server/src/runtime.rs | 205 ++++++++++++++++++++--- 1 file changed, 181 insertions(+), 24 deletions(-) diff --git a/rust/crates/truapi-server/src/runtime.rs b/rust/crates/truapi-server/src/runtime.rs index 8b983d61..f9e88192 100644 --- a/rust/crates/truapi-server/src/runtime.rs +++ b/rust/crates/truapi-server/src/runtime.rs @@ -273,35 +273,27 @@ where pin_mut!(timeout); futures::select! { result = call => result, - reason = cancelled => { - let error = authority_cancellation_error(cx, reason); - let _ = call.await; - Err(error.into()) - }, + reason = cancelled => Err(authority_cancellation_error(cx, reason).into()), () = timeout => { let reason = CancellationReason::TimedOut { timeout: timeout_duration, }; cx.cancel().cancel_with_reason(reason.clone()); - let error = authority_cancellation_error(cx, reason); - let _ = call.await; - Err(error.into()) + Err(authority_cancellation_error(cx, reason).into()) } } } else { futures::select! { result = call => result, - reason = cancelled => { - let error = authority_cancellation_error(cx, reason); - let _ = call.await; - Err(error.into()) - }, + reason = cancelled => Err(authority_cancellation_error(cx, reason).into()), } } } fn authority_cancellation_error(cx: &CallContext, reason: CancellationReason) -> AuthorityError { - AuthorityError::Cancelled(AuthorityCancelError::new(cx.request_id(), reason)) + let error = AuthorityError::Cancelled(AuthorityCancelError::new(cx.request_id(), reason)); + warn!(request_id = %cx.request_id(), %error, "authority call abandoned"); + error } /// Product-scoped adapter that exposes a long-lived host runtime through the @@ -2892,6 +2884,181 @@ mod tests { )); } + struct NeverReadyCall { + dropped: Arc, + } + + impl Future for NeverReadyCall { + type Output = Result<(), AuthorityError>; + + fn poll( + self: std::pin::Pin<&mut Self>, + _cx: &mut std::task::Context<'_>, + ) -> std::task::Poll { + std::task::Poll::Pending + } + } + + impl Drop for NeverReadyCall { + fn drop(&mut self) { + self.dropped.store(true, Ordering::SeqCst); + } + } + + const BOUNDED_RETURN_CEILING: std::time::Duration = std::time::Duration::from_millis(500); + + fn assert_bounded_remote_authority_call( + request_id: &str, + timeout: Option, + after_spawn: impl FnOnce(&truapi::CancellationToken), + expected_reason: CancellationReason, + message: &'static str, + ) { + let token = truapi::CancellationToken::default(); + let mut cx = CallContext::with_parts(request_id.to_string(), token.clone()); + if let Some(timeout) = timeout { + cx.set_timeout(timeout); + } + let dropped = Arc::new(std::sync::atomic::AtomicBool::new(false)); + let call = NeverReadyCall { + dropped: dropped.clone(), + }; + let started = std::time::Instant::now(); + let handle = std::thread::spawn(move || { + futures::executor::block_on(remote_authority_call::<(), AuthorityError, _>(&cx, call)) + .expect_err("a cancelled authority call never succeeds") + }); + after_spawn(&token); + wait_until(|| handle.is_finished(), message); + let elapsed = started.elapsed(); + + match handle.join().expect("authority call thread panicked") { + AuthorityError::Cancelled(err) => assert_eq!( + err.to_string(), + AuthorityCancelError::new(request_id, expected_reason.clone()).to_string() + ), + other => panic!("expected a cancellation, got {other:?}"), + } + assert_eq!( + token.reason(), + Some(expected_reason), + "the abandoned call did not record its cancellation reason on the shared token" + ); + assert!( + dropped.load(Ordering::SeqCst), + "the abandoned inner call was not dropped before the caller resumed" + ); + assert!( + elapsed < BOUNDED_RETURN_CEILING, + "{message}: returned only after {elapsed:?}" + ); + } + + #[test] + fn remote_authority_call_times_out_when_the_inner_call_ignores_cancellation() { + assert_bounded_remote_authority_call( + "rac-timeout", + Some(std::time::Duration::from_millis(1)), + |_| {}, + CancellationReason::TimedOut { + timeout: std::time::Duration::from_millis(1), + }, + "remote_authority_call never returned after its deadline elapsed", + ); + } + + #[test] + fn remote_authority_call_returns_on_explicit_cancellation_under_a_deadline() { + assert_bounded_remote_authority_call( + "rac-cancel", + Some(std::time::Duration::from_secs(30)), + |token| token.cancel(), + CancellationReason::Cancelled, + "remote_authority_call never returned after explicit cancellation", + ); + } + + #[test] + fn remote_authority_call_returns_on_cancellation_without_a_deadline() { + assert_bounded_remote_authority_call( + "rac-no-deadline", + None, + |token| token.cancel(), + CancellationReason::Cancelled, + "remote_authority_call never returned after cancellation without a deadline", + ); + } + + #[test] + fn remote_authority_call_returns_on_an_already_expired_budget() { + assert_bounded_remote_authority_call( + "rac-expired", + Some(std::time::Duration::ZERO), + |_| {}, + CancellationReason::TimedOut { + timeout: std::time::Duration::ZERO, + }, + "remote_authority_call never returned on an already-expired budget", + ); + } + + #[test] + fn remote_authority_call_passes_through_inner_success() { + let mut cx = CallContext::with_request_id("rac-ok".to_string()); + cx.set_timeout(std::time::Duration::from_secs(30)); + + let value = futures::executor::block_on(remote_authority_call::( + &cx, + std::future::ready(Ok(7u8)), + )) + .expect("an inner success passes through unchanged"); + + assert_eq!(value, 7); + } + + #[test] + fn get_account_answers_the_caller_when_the_statement_store_never_connects() { + let platform = Arc::new(StubPlatform { + chain_connect_pending: true, + ..Default::default() + }); + let host = ProductRuntimeHost::new( + platform.clone(), + runtime_config("myapp.dot"), + test_spawner(), + ); + host.test_session_state().set_session(sso_session_info()); + let mut cx = CallContext::with_request_id("account-get-stalled".to_string()); + cx.set_timeout(std::time::Duration::from_millis(1)); + let request = HostAccountGetRequest::V1(v01::HostAccountGetRequest { + product_account_id: account_id("myapp.dot", 0), + }); + + let handle = std::thread::spawn(move || { + futures::executor::block_on(host.get_account(&cx, request)) + .expect_err("a stalled authority call cannot produce an account") + }); + wait_until( + || handle.is_finished(), + "get_account never answered while the statement-store connect stayed pending", + ); + + match handle.join().expect("get_account thread panicked") { + CallError::Domain(HostAccountGetError::V1(v01::HostAccountGetError::Unknown { + reason, + })) => assert_eq!( + reason, + "Account authority request timed out after 1ms for account-get-stalled" + ), + other => panic!("expected an authority timeout, got {other:?}"), + } + + assert!( + platform.pending_connect_dropped.load(Ordering::SeqCst), + "the stalled statement-store connect future was not dropped" + ); + } + #[test] fn get_account_rejects_invalid_product_identifier() { let host = @@ -4012,11 +4179,6 @@ mod tests { ), other => panic!("expected SSO response timeout, got {other:?}"), } - - wait_until( - || recorded_rpc_method_count(&platform.sent_rpc, "statement_unsubscribeStatement") == 2, - "timed-out SSO request did not unsubscribe statement streams", - ); } #[test] @@ -4788,11 +4950,6 @@ mod tests { ), other => panic!("expected resource-allocation timeout, got {other:?}"), } - - wait_until( - || recorded_rpc_method_count(&platform.sent_rpc, "statement_unsubscribeStatement") == 2, - "timed-out resource allocation did not unsubscribe statement streams", - ); } #[test] From 45607c2589a94768b83814cdbac1bedd6a90054e Mon Sep 17 00:00:00 2001 From: Ryan Lee Date: Fri, 14 Aug 2026 20:04:48 +0000 Subject: [PATCH 2/2] docs(learnings): capture the cooperative-cancellation bound from #405 Seeds docs/solutions/ with the transferable rule behind the #405 fix: a select! that cancels a loser and then awaits it has not bounded anything. Includes a recurrence scan confirming the fixed site was the only instance of the shape in rust/**. Seeds CONCEPTS.md with the account-authority and statement-store vocabulary the learning depends on. --- CONCEPTS.md | 56 ++++++ ...angs-when-cancelled-future-is-reawaited.md | 189 ++++++++++++++++++ 2 files changed, 245 insertions(+) create mode 100644 CONCEPTS.md create mode 100644 docs/solutions/runtime-errors/timeout-hangs-when-cancelled-future-is-reawaited.md diff --git a/CONCEPTS.md b/CONCEPTS.md new file mode 100644 index 00000000..85bf6325 --- /dev/null +++ b/CONCEPTS.md @@ -0,0 +1,56 @@ +# Concepts + +Shared domain vocabulary for this project — entities, named processes, and status concepts with project-specific meaning. Seeded with core domain vocabulary, then accretes as ce-compound and ce-compound-refresh process learnings; direct edits are fine. Glossary only, not a spec or catch-all. + +Seeded from the account-authority and statement-store area. Other areas of the system are not yet covered. + +## Relationships + +A Product Authority is fulfilled by exactly one role per connection — either a Pairing Host or a Signing Host — and a product cannot tell which from the calls it makes. Every Authority Call runs under a Call Context, which owns the Cancellation Token and the deadline that bound it. Writing to the Statement Store requires an Allowance, which the authority obtains on the product's behalf; on a Pairing Host that obtaining travels the SSO Channel, on a Signing Host it happens locally. + +## Authority + +### Authority Call +A request the product-facing runtime makes to the account authority on a product's behalf — fetching an account, allocating resources, obtaining allowance material, or signing a payload. + +An Authority Call is bounded by its Call Context: when the deadline elapses or the token is cancelled, the caller raises the cancellation reason and then abandons the in-flight work rather than waiting for it. Abandoned work is dropped, never awaited to completion — the deadline is only real because the caller stops, not because the callee agrees to. Work already dispatched to a remote peer may still complete on the peer's side after the caller has walked away. + +### Product Authority +The account-owning side of a product connection: the party that holds or reaches the account material a product asks for, and that answers Authority Calls. Distinct from the product itself, which never holds account material and can only ask. + +Two roles fulfil it, and they differ in *where* the account material lives rather than in what the product may request. + +### Pairing Host +A Product Authority whose account material lives with a remote, paired party. It answers an Authority Call by carrying a request over the SSO Channel and waiting for the remote side to approve and respond, so its calls can block on a human or on a network peer. + +### Signing Host +A Product Authority whose account material is held locally. It answers an Authority Call directly — signing, or registering an Allowance on chain — without a remote approval round trip, so its calls block on chain progress rather than on a peer. + +## Request context + +### Call Context +The ambient per-request state threaded through a call: which request it belongs to, the Cancellation Token that can stop it, and the optional deadline it must finish within. + +A Call Context is shared, not owned by any single call — clones observe the same cancellation. A context with no deadline is still cancellable; a context whose deadline has already elapsed cancels at the first opportunity rather than being treated as unbounded. + +### Cancellation Token +The shared signal that a call should stop, carrying the reason it was stopped — an explicit cancellation, or an elapsed deadline. + +Cancellation here is cooperative: the token records a reason and wakes whoever is watching, but work parked on something that never checks the token never learns of it. A token therefore requests termination and cannot guarantee it; a caller that needs a guarantee must stop waiting and drop the work itself. The reason is set once and stays readable afterwards, so latecomers can still learn why a call ended. + +## Statements and allowances + +### Statement Store +The shared store a product submits statements to and subscribes to by topic. It is the transport underneath cross-party exchanges as well as a destination in its own right — an SSO conversation is carried as statements in this store rather than over a private channel. + +### Allowance +The permission material that lets a product write to a store, obtained through the Product Authority rather than held by the product. + +Allowances are scoped to a time period and occupy a limited number of slots within it, so obtaining one is a claim against a contended resource, not a local computation: a slot is scanned for, claimed, and registered on chain. Because a period can be full, taking a slot may revoke another holder's — which makes duplicate or abandoned registration attempts costly rather than merely wasteful. + +## Single sign-on + +### SSO Channel +The request-and-response conversation between a host and its remote paired party, carried as statements through the Statement Store rather than a direct connection. + +Each request carries an opaque message id that its response must match, so a response to a request the host has already abandoned is ignored rather than mismatched onto a later one. Both sides' statement streams are subscribed for the life of a request and released when it ends. diff --git a/docs/solutions/runtime-errors/timeout-hangs-when-cancelled-future-is-reawaited.md b/docs/solutions/runtime-errors/timeout-hangs-when-cancelled-future-is-reawaited.md new file mode 100644 index 00000000..d877202f --- /dev/null +++ b/docs/solutions/runtime-errors/timeout-hangs-when-cancelled-future-is-reawaited.md @@ -0,0 +1,189 @@ +--- +title: A deadline that re-awaits the future it just cancelled is not a deadline +date: 2026-08-14 +category: runtime-errors +module: truapi-server +problem_type: runtime_error +component: service_object +symptoms: + - "A wire request is neither answered nor refused — the caller sees no success and no timeout error, only silence" + - "The stall needs no error condition: a host connect or subscribe that simply never resolves is enough" + - "The handler future never resolves, so the dispatcher never sends a response frame" + - "Tests asserting post-deadline cleanup start failing once the deadline is actually enforced" +root_cause: async_timing +resolution_type: code_fix +severity: high +related_components: + - authentication + - testing_framework +tags: + - cancellation + - cooperative-cancellation + - timeout + - async-rust + - futures-select + - drop + - deadline +--- + +# A deadline that re-awaits the future it just cancelled is not a deadline + +**The rule:** cooperative cancellation is a *request*, not a guarantee. A future that +is parked on an await which never polls the cancellation token will never observe it, +and nothing in the token machinery can force it to. The only hard stop a caller owns +is dropping the future. So a `select!` that cancels a loser and then awaits it has not +bounded anything — it has just moved the hang one line down. + +## Problem + +`remote_authority_call` (`rust/crates/truapi-server/src/runtime.rs:262`) races a host +authority call against a cooperative `CancellationToken` and an optional deadline. Every +non-success arm ended with `let _ = call.await;` — re-awaiting the future it had just +cancelled — before returning its error. When the inner future was parked on an await +that never observes the token, the timeout arm parked with it and the outer call returned +nothing at all. + +## Symptoms + +- A logged-in user's `account_get` never comes back. `Dispatcher::dispatch` sends a + response frame only once the handler resolves, so the request sat neither answered nor + refused — the failure is silence on the wire, not an error. +- No fault is required to trigger it. A statement-store `connect` that stays pending + (`rust/crates/truapi-server/src/test_support.rs:1355-1357` models exactly this with + `futures::future::pending::<()>().await`) or a subscribe whose ack never arrives + (`rust/crates/truapi-server/src/runtime/pairing_host/sso_channel.rs`, where + `submit_remote_message` parks on `wait_for_sso_remote_response`) is sufficient. +- The deadline appears to be configured and is in fact inert. Every one of the 18 + `remote_authority_call` sites sits under a non-`None` timeout, so the timeout branch is + always the one that fires — and it was the branch that hung. +- **The clearest symptom only appears after the fix:** two assertions that a timed-out + request unsubscribes two statement streams began failing. See *Prevention*. + +## What Didn't Work + +- **Make the three parking awaits cancel-aware and keep the re-await.** This treats the + awaits that happen to park today. The next await added inside any authority call that + does not poll the token reintroduces the identical hang, and the re-await is precisely + what keeps the abandoned future alive. The hazard is open-ended; the patch is not. +- **Detach the inner future onto a spawner with a reaper.** This trades a hang for a leak: + a never-resolving connect keeps its stack, its connection, and any lock it holds, now + with no one waiting on it. It is also structurally awkward — + `remote_authority_call` is a free `fn(cx: &CallContext, call: F)` with no spawner in + scope, so threading one through would touch all 18 call sites to buy that leak. +- **Arguing that the drop is safe because no orphan can hold a lock.** An earlier draft of + the plan asserted this. It is false, and code review falsified it: on the SigningHost, + `allocate_statement_store_allowance` takes `registration_lock` + (`rust/crates/truapi-server/src/runtime/signing_host/sso_responder.rs:949`) and holds it + across an on-chain submission, reachable from `remote_authority_call` at + `rust/crates/truapi-server/src/runtime/statement_store.rs:344`. The drop *is* still the + right fix — guards release on drop — but the reason is that `Drop` runs, not that + orphans hold nothing. What the drop does not do is restore the invariant the guard was + protecting, and that is a genuine residual to design for separately, not a detail to + wave away. + +## Solution + +Delete the re-await from every non-success arm so each returns its error directly, +dropping the `select!` loser. Keep `cancel_with_reason` ahead of the drop. + +Before: + +```rust +() = timeout => { + let reason = CancellationReason::TimedOut { timeout: timeout_duration }; + cx.cancel().cancel_with_reason(reason.clone()); + let _ = call.await; // parks here forever + Err(authority_cancellation_error(cx, reason).into()) +} +``` + +After (`rust/crates/truapi-server/src/runtime.rs:277-283`): + +```rust +() = timeout => { + let reason = CancellationReason::TimedOut { timeout: timeout_duration }; + cx.cancel().cancel_with_reason(reason.clone()); + Err(authority_cancellation_error(cx, reason).into()) +} +``` + +The same deletion applies to the two cancellation arms. The shape already had a precedent +in the same crate: `submit_preimage` returns on cancel and on timeout without re-awaiting +its loser (`rust/crates/truapi-server/src/runtime/bulletin_rpc.rs:293`, `:313-315`). + +The drop also skips the unwind that used to emit the host-side log for an abandoned call, +so that diagnostic moved onto `authority_cancellation_error` itself. + +## Why This Works + +`cancel_with_reason` records a reason and wakes registered wakers +(`rust/crates/truapi/src/lib.rs:289`); `CancellationFuture::poll` stays `Pending` until a +reason is set (`rust/crates/truapi/src/lib.rs:327-330`). Nothing in that machinery reaches +into a future that is parked elsewhere. If the inner future never polls the token, it never +learns anything happened — which is the whole meaning of *cooperative*. + +The caller, by contrast, owns its own control flow unconditionally. Returning from the +`select!` drops the losing future, and the drop is not negotiable: it runs `Drop` all the +way down the tree, releasing subscriptions, connection handles, and lock guards. + +Keeping `cancel_with_reason` **before** the drop is not redundant. The token is shared, so +raising it first still gives cancel-aware inner futures and every other holder of the same +token their cooperative path, and leaves the reason observable via `cx.cancel().reason()` +(`rust/crates/truapi/src/lib.rs:304`). The layering is the point: cooperation first, drop +as the guaranteed stop behind it. + +## Prevention + +- **Write the test against a future that *cannot* cooperate.** A future whose `poll` + always returns `Poll::Pending` and whose `Drop` sets an `AtomicBool` + (`rust/crates/truapi-server/src/runtime.rs:2887`) is the only shape that can tell a real + bound from a cooperative one. Drive it on a spawned thread and assert four things + together (`rust/crates/truapi-server/src/runtime.rs:2910`): the error came back, the + shared token recorded the reason, the inner future was dropped before the caller + resumed, and the wall-clock elapsed time is under a ceiling. A test that asserts only + "an error came back" passes just as happily against a re-await bounded by a grace period. +- **Assert the cancel signal, not just the return.** Deleting + `cancel_with_reason` leaves every timing assertion green — the drop alone still returns + on time. Asserting `token.reason()` is what makes the cooperative half of the contract + load-bearing. +- **Revert-check before shipping.** Restore the bug and watch the new tests fail for the + *stated* reason. Both gates here were confirmed that way: re-adding the re-await hung + the bounded-return tests, and removing `cancel_with_reason` failed the reason assertion + with `left: None, right: Some(TimedOut { timeout: 1ms })`. +- **When a fix makes an old assertion fail, ask what the assertion was measuring.** Two + assertions that a timed-out request unsubscribes *two* statement streams broke here. A + throwaway probe showed why: under a real bound only **one** subscribe is ever sent, + because the call is abandoned before it reaches the second. Those assertions had only + ever passed because the re-await let a timed-out call keep working past its deadline — + they were asserting the bug's side effect. They were deleted, not loosened. The + unsubscribe-on-abandonment contract still has a home in + `sign_raw_cancellation_unsubscribes_sso_subscriptions`, which stages both subscriptions + *before* firing the token. +- **Do not measure a bound against a suite you have not measured first.** The + `truapi-server` suite has pre-existing wall-clock flakiness under back-to-back load, + reproduced on the clean tree: `submit_preimage_recovers_inconsistent_inclusion_via_recheck` + failed once in four clean runs with `Timeout { phase: Connect }`. A replacement test + proposed in review was written, measured to flake one run in three, and removed rather + than shipped. + +## Recurrence scan + +A scan of `rust/**` for the same shape found the fixed site to be the only instance. Every +other `futures::select!` with a cancellation or timeout arm returns from that arm without +re-awaiting the loser — `bulletin_rpc.rs:293`/`:313-315`/`:559`/`:641`, `identity.rs`, +`sso_pairing.rs`, `sso_remote.rs`, `host_rpc_client.rs`. Remaining `let _ = .await;` +occurrences in the crate are shutdown pumps or deliberately joined handles, not cancelled +`select!` losers. + +Deadlines are centralized: they enter through the `remote_authority_context_*` family +(`rust/crates/truapi-server/src/runtime.rs:237-246` sets the default; `:248-260` sets an +absolute one) and are enforced only by `remote_authority_call`, so this one function is +the whole enforcement surface for the authority path. + +## Related Issues + +- Issue #405 — the originating report. +- `docs/plans/2026-08-14-1832-fix-authority-call-timeout-bounded-return-plan.md` — the + implementation plan, including the rejected alternatives and the corrected lock claim. +- `docs/residual-review-findings/ryan-405.md` — code-review findings, including the + `registration_lock` truncation and two non-`Drop` compensation windows left open.