Skip to content

fix: keep script shutdown independent of stdout writes - #301

Merged
swananan merged 1 commit into
mainfrom
fix/cancellable-script-output
Sep 6, 2026
Merged

fix: keep script shutdown independent of stdout writes#301
swananan merged 1 commit into
mainfrom
fix/cancellable-script-output

Conversation

@swananan

@swananan swananan commented Sep 6, 2026

Copy link
Copy Markdown
Owner

When a stdout consumer stops reading, script output can block the CLI event loop and prevent signal handling. A closed consumer or another write failure could also leave tracing running after output had failed.

Send rendered output through a bounded queue to a dedicated thread owning a duplicate stdout descriptor. Signals remain observable while waiting for queue capacity. Writer completion stops tracing: a broken pipe exits normally, while other I/O failures return an error. The writer owns no session or probe resources and uses neither Rust's global stdout lock nor Tokio's blocking pool.

On graceful shutdown, release the session and allow accepted output up to 100 ms to drain. A deadline expiry or an interrupted producer batch reports abandoned output. That best-effort stderr notice uses its own bounded writer and 100 ms deadline, so redirecting stderr into the blocked stdout pipe cannot introduce a shutdown wait. An outstanding OS write is not forcibly cancelled and its dedicated thread is not joined.

Rebased onto main at 9b0c982, with the review fixes amended into one commit, 9314b7d; four files changed.

Validation:

  • Formatting and Clippy with CI warning settings passed.
  • All 958 non-e2e tests passed; the local and GitHub test-name lists match.
  • All 353 standard e2e tests passed locally and in GitHub on the amended head, including five real-pipe lifecycle cases: SIGTERM, SIGINT, merged stdout/stderr, immediate consumer close, and close after queue backpressure.
  • The healthy-consumer drain regression passes with the fix and fails when the original immediate-drop behavior is restored in an isolated temporary copy.
  • Real CLI comparisons cover /dev/full and a consumer resuming during shutdown. The final CLI returns ENOSPC as an error, drains queued data, and reports interrupted output.
  • Independent sub-agent review completed with no unresolved confirmed findings.

All nine applicable GitHub checks passed on 9314b7d, including standard e2e, three full container topologies, and host-PID smoke. Local container-topology runs were omitted because this change does not alter container behavior.

@swananan
swananan marked this pull request as ready for review September 6, 2026 04:59
@swananan
swananan force-pushed the fix/report-partial-script-failures branch from 7b5d007 to f418dcf Compare September 6, 2026 11:30
Base automatically changed from fix/report-partial-script-failures to main September 6, 2026 12:21
@swananan
swananan force-pushed the fix/cancellable-script-output branch from d366401 to 9314b7d Compare September 6, 2026 12:40
@swananan
swananan merged commit f36a86c into main Sep 6, 2026
9 checks passed
@swananan
swananan deleted the fix/cancellable-script-output branch September 6, 2026 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant