Skip to content

os: hold Windows file handles during Stat and readdir - #82528

Open
Shubham-Padkonde wants to merge 1 commit into
golang:masterfrom
Shubham-Padkonde:fix/windows-file-handle-references
Open

Shubham-Padkonde wants to merge 1 commit into
golang:masterfrom
Shubham-Padkonde:fix/windows-file-handle-references

Conversation

@Shubham-Padkonde

Copy link
Copy Markdown

Stat and directory reads on Windows use the native handle without a
poll.FD reference. A concurrent Close can release that handle while the
operation is using it, and the handle may be reused for another file.
Calls after Close also expose native invalid-handle errors rather than
ErrClosed.

Hold the reference with RawControl for the entire operation. Pass the
held handle through directory initialization, buffer refills, and the
non-directory error check. Keep the File alive through the callback and
translate reference-acquisition errors through wrapErr.

Enable TestClosedStat on Windows and cover closed directory operations
before and after priming the directory buffer, plus Stat racing Close
with potential handle reuse. Directory-buffer retirement is separate
work tracked by #82292.

Validation on windows/amd64: os, internal/poll and
internal/syscall/windows pass in short mode; os and internal/poll also
pass with the race detector. The closed-handle regressions fail with
unchanged production source. Focused local tests pass ten repetitions.

The full src/all.bat run passes the affected packages but fails in SWIG
LTO execution and cmd/go. A baseline/fixed comparison reproduces the same
SWIG executable error, Strawberry patch assertions and pkg-config output
mismatch. Four tests interrupted by the full cmd/go deadline pass in
isolation on both versions. The full suite is not claimed to pass.

Focused validation:
https://github.com/Shubham-Padkonde/go/actions/runs/40290214159
Full suite:
https://github.com/Shubham-Padkonde/go/actions/runs/40291229025
Baseline/fixed comparison:
https://github.com/Shubham-Padkonde/go/actions/runs/40294367891

Fixes #82293

@gopherbot

Copy link
Copy Markdown
Contributor

This PR (HEAD: 8eebe03) has been imported to Gerrit for code review.

Please visit Gerrit at https://go-review.googlesource.com/c/go/+/853965.

Important tips:

  • Don't comment on this PR. All discussion takes place in Gerrit.
  • You need a Gmail or other Google account to log in to Gerrit.
  • To change your code in response to feedback:
    • Push a new commit to the branch used by your GitHub PR.
    • A new "patch set" will then appear in Gerrit.
    • Respond to each comment by marking as Done in Gerrit if implemented as suggested. You can alternatively write a reply.
    • Critical: you must click the blue Reply button near the top to publish your Gerrit responses.
    • Multiple commits in the PR will be squashed by GerritBot.
  • The title and description of the GitHub PR are used to construct the final commit message.
    • Edit these as needed via the GitHub web interface (not via Gerrit or git).
    • You should word wrap the PR description at ~76 characters unless you need longer lines (e.g., for tables or URLs).
  • See the Sending a change via GitHub and Reviews sections of the Contribution Guide as well as the FAQ for details.

@gopherbot

Copy link
Copy Markdown
Contributor

Message from Gopher Robot:

Patch Set 1:

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
A maintainer will review your change and provide feedback. See
https://go.dev/doc/contribute#review for more info and tips to get your
patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11 or adds a tag like "wait-release", it means that this CL will be
reviewed as part of the next development cycle. See https://go.dev/s/release
for more details.


Please don’t reply on this GitHub thread. Visit golang.org/cl/853965.
After addressing review feedback, remember to publish your drafts!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

os: Stat and ReadDir on Windows use the file handle without holding a reference

2 participants