Repository navigation
Conversation
update --cli's smoke test spawns the freshly downloaded binary with Command::output(). On Linux, fork() duplicates every file descriptor open anywhere in the process, so an unrelated Command::spawn() running concurrently elsewhere in the same process can fork a child that inherits a transient write handle on the just-written binary and hold it open until that child execs or exits. Until then, exec of the binary fails with ETXTBSY (os error 26) even though the smoke test's own write handle closed well before the exec was attempted. Retry the spawn up to 10 times with a 100ms delay between attempts (about 1 second total) when the failure is ExecutableFileBusy, giving the other process time to exec or exit and release its inherited handle. The retry loop is extracted into its own function with an injectable spawn/sleep seam, so a unit test can drive it deterministically with a fake exec that fails N times then succeeds, and a separate case that exhausts all retries, without depending on reproducing the real race.
12 of 14 tasks
Contributor
Author
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.
Description
CLI:
cli_self_update.rs:update --cli's smoke test now retries the exec of the freshly downloaded binary onExecutableFileBusy(os error 26), instead of failing the update outright.On Linux,
fork()duplicates every file descriptor open anywhere in the process. If another thread forks while the downloaded binary's write handle is still open, the child inherits that handle and holds it until it execs or exits. Until then, exec of the binary fails with "Text file busy", even though the smoke test's own write handle closed well before the exec was attempted. This is a known class of race in the Rust standard library; see rust-lang/rust#114554.The fix retries the spawn up to 10 times, 100ms apart (about 1 second total), but only when the failure is
ExecutableFileBusyor raw os error 26. The retry loop lives in its own function with an injectable spawn/sleep seam, so a unit test can drive it deterministically with a fake exec that fails a set number of times then succeeds, without depending on reproducing the real race.1.0.3->1.0.5with a correspondingCHANGELOG.mdentry.Fixes # (issue)
Type of Change
cli/) change — RustTesting
cli/changed:cd cli && make testpasses (Rust + Python + conformance suites)Full
cargo testpasses locally: 1725 tests. Ran the full suite 15 times in a row at 64 threads to rule out flakiness from the timing-sensitive retry logic; all 15 runs were green.Checklist
cli/changes,cd cli && make lintpasses (cargo clippy -- -D warnings+cargo fmt --check)Agents Affected
None.
Notes for Reviewers
This only affects the self-update path on Linux, where it's possible for a concurrent fork elsewhere in the process to transiently hold the downloaded binary's file descriptor open. The retry window (about 1 second) is a judgment call: long enough to clear the race in practice, short enough that a genuinely broken binary still fails the update promptly.