app-server: harden command/exec drain and lifecycle races#13696
app-server: harden command/exec drain and lifecycle races#13696euroelessar wants to merge 1 commit intomainfrom
Conversation
euroelessar
force-pushed
the
ruslan/exec-stream
branch
from
4473b52 to
fce9180
Compare
euroelessar
force-pushed
the
ruslan/exec-stream-harden
branch
2 times, most recently
from
e01940e to
e41718f
Compare
euroelessar
force-pushed
the
ruslan/exec-stream
branch
from
fce9180 to
e61a5d0
Compare
euroelessar
force-pushed
the
ruslan/exec-stream-harden
branch
from
e41718f to
1de71cd
Compare
euroelessar
force-pushed
the
ruslan/exec-stream
branch
2 times, most recently
from
e2cc0a2 to
0628035
Compare
Close the remaining post-exit and control-path races by draining tail output with a quiet window plus a bounded final ready pass, and by returning consistent not-running errors after a process has already exited. Add a dedicated helper binary and focused tests so the lifecycle behavior is reviewable on its own, and reject Windows disableTimeout requests that the buffered fallback cannot honor.
euroelessar
force-pushed
the
ruslan/exec-stream-harden
branch
from
1de71cd to
58a5bd9
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58a5bd9c55
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if drained_any { | ||
| if let Some((quiet_deadline, _)) = post_exit_deadlines.as_mut() { | ||
| *quiet_deadline = Instant::now() + post_exit_quiet_period; | ||
| } | ||
| continue; |
There was a problem hiding this comment.
collect_process_output resets the quiet deadline and continues whenever any chunk is drained. Once hard_deadline has elapsed, ready_drain_limit only caps a single inner try_recv pass; the outer loop immediately starts another pass, so continuous grandchild output can keep this task alive forever. run_command then waits indefinitely on stdout/stderr join handles, delaying response/session cleanup.
Useful? React with 👍 / 👎.
1 participant
Close the remaining post-exit and control-path races by draining tail output with a quiet window plus a bounded final ready pass, and by returning consistent not-running errors after a process has already exited.
Logic is primarily complicated in order to handle a case where child process did spawn some grand-children processes which inherit its stdout pipe (and therefore something may still continue writing there).
Add a dedicated helper binary and focused tests so the lifecycle behavior is reviewable on its own, and reject Windows disableTimeout requests that the buffered fallback cannot honor.