Repository navigation
Conversation
finish_exit/2 served parked stdout/stderr readers from the pipe but not the select-driven :consume drain. When the exit status arrived before the stderr readiness event, stderr_tail/1 right after await_exit/1, and run/2 with stderr: :capture, missed the child's last writes: about 1 run in 2,000 on an ubuntu-latest runner under concurrent load. The regression test runs 2,000 concurrent children through both entry points; it failed on ubuntu-latest before this change and passes on every CI job after it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In
:consumestderr mode, the tail can miss the child's last writes when the exit status arrives before the stderr readiness event.finish_exit/2serves parked stdout/stderr readers from the pipe (the comment there explains why: the exit status can arrive over the UDS while output is still sitting in the pipe), but the:consumedrain is only driven by select events, so nothing drains it at exit. If the UDS exit message wins the race,stderr_tail/1called right afterawait_exit/1returns a short or empty tail. That includesrun/2withstderr: :capture, whose comment says the tail is complete once the process has exited.stderr_tail_test.exsworks around it withwait_until_drained/2("avoids racing the select-driven consumer against the OS process exit notification").Fix: also call
maybe_consume_stderr/1infinish_exit/2. The child's write ends are closed by then, so the drain reads the remaining bytes and stops at:eof(or:eagainif a grandchild still holds the pipe open). The:consume_stderr_morehandler already treats a drain after exit as a no-op