version-management: tolerate a Windows delete-pending race in license repair - #793
Open
pullfrog[bot] wants to merge 1 commit into
Open
version-management: tolerate a Windows delete-pending race in license repair#793pullfrog[bot] wants to merge 1 commit into
pullfrog[bot] wants to merge 1 commit into
Conversation
… repair atomic_replace_file installs its staging file with a hard link and unlinks the staging name once it lands. Windows holds the shared file delete-pending until its last handle closes, so a sibling repairer's open of the destination answers ERROR_ACCESS_DENIED rather than the bytes just installed there. The content probe read that as a mismatch and the displacing rename then failed outright, surfacing a lost race as a permission error. Re-enter the loop across a bounded backoff instead, so the probe can observe the sibling's bytes once its handle closes, and report the rename error only after the destination is still not ours on the final attempt.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Closed
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.
Closes #792
CIrun 32875385039 went red onmainata3083f48awith one failure, inembed-runtime verify (windows-latest):Not introduced by
a3083f48a(#791 only touched submodule handling in recursive clones). It is a latent Windows race inatomic_replace_file, and the concurrent-repair test is what exposes it.Mechanism
atomic_replace_filestages bytes in a temp file, installs them withhard_link(tmp, dest), and unlinkstmpvia itsFileGuardas soon as the link lands.tmpanddestare then the same file, so that unlink puts the shared file into Windows' delete-pending state, and NTFS answers every open of it — through either link — withERROR_ACCESS_DENIEDuntil the last handle closes. A sibling repairer'sstd::fs::read(&dest)handle is exactly what holds that window open.Two things then went wrong in the loser thread:
std::fs::read(&dest).ok()content probe conflated that transient error with "the destination holds different bytes", so it proceeded to displace a destination that already held the correct content.std::fs::rename(&dest, &displaced)hit the same delete-pending state and was treated as fatal, surfacing a lost race as a permission failure.The codebase already knows this behavior —
node/spawn.rs:1659and:1757both order handle drops around it — andcompile/launcher.rs'scommit_templatealready uses the right remedy for a no-overwrite commit: on a failed install, re-probe the destination and accept it if a racer published the bytes we wanted.Fix
Bound the displacement failures instead of erroring on the first one, and re-enter the loop so the existing content probe can observe the sibling's bytes once its handle closes. The rename error is reported unchanged, with its original context, only when the destination is still not ours on the final attempt. Backoff is 2ms doubling to 128ms, ~254ms total before giving up.
NotFoundstillcontinues as before, and a genuineEACCESon Unix still surfaces — just after a bounded delay rather than instantly.Verification
cargo test -p nub-core --lib— 438 passed, 0 failed (includes the failing test).cargo clippy -p nub-core --all-targets -- -D warnings— rc=0;cargo fmt --check -p nub-coreclean.Caveat worth stating plainly: delete-pending is Windows-kernel behavior with no Linux analogue, so the local run cannot exercise the path that failed.
embed-runtime verifyruns the full[ubuntu, windows, macos]matrix on pull requests, so the Windows leg re-runs here — but the original failure was intermittent, so one green run is not proof. No new test: the existing concurrent-repair test is the guard, and a deterministic reproduction would need a rename seam injected purely for the test.Claude Opus| 𝕏