Skip to content

fix(core): join both output drains once across the grace windows - #315

Merged
Max17190 merged 1 commit into
mainfrom
join-output-drains-once
Sep 7, 2026
Merged

Max17190 merged 1 commit into
mainfrom
join-output-drains-once

Conversation

@Max17190

@Max17190 Max17190 commented Sep 7, 2026 •

Copy link
Copy Markdown
Owner

Why

A supervised process's stdout and stderr drains were joined inside the first grace window with a fresh tokio::join! of the two JoinHandles, and joined again in the second window when the first timed out. A JoinHandle yields its output once and panics when polled after that, so a drain that finished inside the first window (the usual shape when a detached descendant keeps only one inherited pipe open) was polled again in the second, and the whole turn ended on JoinHandle polled after completion with the task abandoned mid-way.

Summary

  • join_streams pins one join future and polls it through both grace windows, so the finished side is remembered and never polled again. Grace timings, the stop cancel, the abort path, and the error text are unchanged.
  • The Option plus let-else replaces the nested match, and one closure replaces the duplicated two-step error mapping.

Test Plan

  • New a_finished_drain_is_not_polled_again_when_the_other_hangs: finishes one drain at once and holds the other until the stop token fires. Panics with the exact message on the previous code; passes now.
  • cargo test --workspace --locked green (599 core, 27 CLI); cargo +1.97.0 clippy --workspace --all-targets --locked -- -D warnings clean.

Greptile Summary

This PR fixes output-drain supervision by polling one pinned join future across both grace windows, preventing a completed drain task from being polled again. The targeted regression scenario now returns both captured streams after cancellation instead of crashing.

Confidence Score: 5/5

Safe to merge: the changed drain-supervision path preserves timeout and cancellation behavior while preventing the completed-task polling crash.

No outstanding issues were found. The focused scenario that previously crashed after one drain completed now completes and returns both stream results.

Files Needing Attention: None.

T-Rex T-Rex Logs

What T-Rex did

  • Executed source/script: `trex-artifacts/join-streams-grace-repro.sh`.
  • Exact after command: `cargo test -p open-max-core --lib execution::tests::a_finished_drain_is_not_polled_again_when_the_other_hangs -- --exact --nocapture`; exit 0, one test passed.
  • Exact before command: `cargo test -p open-max-core --lib execution::join_streams_parent_repro::a_finished_drain_is_not_polled_again_when_the_other_hangs -- --exact --nocapture`; exit 101, with `JoinHandle polled after completion`.
  • Relevant code: `crates/core/src/execution.rs:659-693`; regression test: `crates/core/src/execution.rs:948-969`.

View all artifacts

T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "fix(core): join both output drains once ..." | Re-trigger Greptile

Context used:

A supervised process's stdout and stderr drains were joined inside the
first grace window with a fresh tokio::join! of the two JoinHandles, and
joined again in the second window when the first timed out. A JoinHandle
yields its output once and panics when polled after that, so a drain that
finished inside the first window (the usual shape when a detached
descendant keeps only one inherited pipe open) was polled again in the
second and the whole turn ended on "JoinHandle polled after completion".

One join future is now pinned and polled through both windows, so the
finished side is remembered and never polled again. Grace timings, the
stop cancel, the abort path, and the error text are unchanged.

The new test finishes one drain at once and holds the other until the
stop token fires; it panics on the previous code.
@Max17190
Max17190 merged commit 91db019 into main Sep 7, 2026
11 checks passed
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