patch: replace errno-87 retry with dropping the redundant second open#3
Open
MarshallOfSound wants to merge 1 commit intomainfrom
Open
patch: replace errno-87 retry with dropping the redundant second open#3MarshallOfSound wants to merge 1 commit intomainfrom
MarshallOfSound wants to merge 1 commit intomainfrom
Conversation
Root cause is a bindflt.sys race on handle-relative NtCreateFile when a second relative open arrives while the first handle to the same target is live (see the investigation on electron/electron sam/windows-io-repro). Single opens on the same files, same relative paths, same bindflt emptyDir do not trigger it — so sharing the outer *os.File for ReadAt (which is documented concurrent-safe) removes the only overlapping-open in the manifest loader and makes the retry unnecessary. 7-line deletion; also halves CreateFileW calls per subninja.
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
1 participant
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.
Replaces the
ERROR_INVALID_PARAMETERretry with the root-cause fix.The investigation on
electron/electronbranchsam/windows-io-repropinned the failure tobindflt.sys's handling of handle-relativeNtCreateFileon a mapped directory: a second relative-path open of the same target while the first handle is still being set up intermittently returns errno 87. Single opens of the same files (same relative paths, same emptyDir, same 32-way concurrency) never hit it — so removingreadFile's per-chunk re-open and sharing the outer*os.FileforReadAt(documented concurrent-safe) takes the manifest-load failure rate to zero without a retry.Diff is a 7-line deletion in
fileParser.readFile. Also halves theCreateFileWcalls during the subninja scan.After this merges, tag
v1.5.7-electron.3; we can then drop theopenfile_windows.goretry from the upstream CL and send this instead.