Repository navigation
Blocking cleanup hidden inside Drop #12
Description
Timing: Follow-up after trial/3 merges into main; defer implementation until then.
Review baseline: Local trial/3 at a094418baf6388e3eeca23821df481a7f0a8751c. This branch is not yet on the remote. Source locations below refer to that snapshot and should be rechecked after merge.
Problem
TtyInput::drop synchronously joins its native input worker. Dropping a pending async prompt during cancellation or unwind can therefore block the executor thread performing the drop.
This is a documented compromise, not a demonstrated deadlock: the normal worker loop uses a 20-ms polling/backpressure interval, but competing Crossterm calls and OS stalls are explicitly outside that bound. The existing code deliberately avoids detaching a live stdin reader and attempts to restore terminal ownership.
Evidence
tools/cli/src/tty.rs:247-254: explicit async close awaits completion before joining.tools/cli/src/tty.rs:273-281: synchronous worker join.tools/cli/src/tty.rs:289-296: Drop signals stop and then joins synchronously.
Acceptance criteria
- Evaluate a cleanup design that minimizes executor blocking on cancellation/unwind while preserving exclusive stdin ownership and raw-mode restoration.
- Keep normal explicit close asynchronous and idempotent.
- Do not silently detach a competing terminal reader or claim that arbitrary OS stalls have a strict time bound.
- Verify idle, backpressured, cancellation, repeated-close, worker-failure, and restoration-failure behavior; document any unavoidable blocking fallback.
Code examples and suggested direction
A possible ownership direction is a prestarted cleanup/reaping owner:
// Design sketch only: cleanup is a proposed new owned component.
impl Drop for TtyInput {
fn drop(&mut self) {
self.signal_stop();
if let Some(worker) = self.worker.take() {
self.cleanup.retire(worker);
}
}
}retire would have to transfer ownership without waiting for worker completion, guarantee eventual joining, and define queue/shutdown failure handling. This snippet is not a complete fix. The terminal lease must remain held until the worker actually restores raw mode.
Activity
cblanquera commentedon Oct 26, 2026
Suggested investigation and fix notes
Deferred until
trial/3is merged intomain. Recheck the merged source and tests first. The snippets are examples/proposals, not an approved patch or a change to frozen contracts.close(), and Drop across success, cancellation, panic, and restoration failure. Usetty_contracts,tty_prompt_contracts, and their PTY probe helpers as the baseline.TtySession; dropping the join handle alone detaches the reader and is not sufficient.spawn_blockingfrom Drop is always available or abortable.Suggested feature checks:
cargo test -p stackpress-cli --test tty_contracts --test tty_prompt_contracts --locked cargo check -p stackpress-cli --no-default-features --lockedImplementation workflow: After the merge, follow
.agents/workflows/rust-programming.md: prove the affected behavior, complete the three test-verified review rounds, and record relevant formatting/lints/docs, feature/platform scope, and applicable development coverage. Preserve unrelated changes.cblanquera commentedon Oct 26, 2026
Fixed on
trial/3in 407b4d6 + df443b7 (merged as ff7a251, task 00010 lane 2; not yet onmain, so leaving open).TtyInputdrop no longer blocks the executor on a synchronous worker join in the normal path; explicit close stays async and idempotent, and a competing stdin reader is never detached.Coordinator-verified on the combined code: fmt, Clippy -D warnings, Rustdoc -D warnings,
cargo +1.96.0 check, 466 workspace tests and 857 doctests pass (467 tests on mergedtrial/3). Coverage: server 91.61%, CLI 93.15%, transformer 93.62%.Fix owner review issues #10, #3, #8, #9, #13, #12 and host parts of #4,