Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vfs: fix race condition in JavaIoFileSystem.delete #5527

Closed
wants to merge 1 commit into from

Conversation

laszlocsomor
Copy link
Contributor

JavaIoFileSystem.delete() now uses
Files.deleteIfExists() to delete files and empty
directories.

The old code used to have a race condition: a
file could have been deleted or changed to a
non-directory between checking it's a directory
and listing its contents.

WindowsFileSystem.delete() now uses the
DeletePath JNI method (wrapped by
WindowsFileOperations.deletePath) which is more
robust than JavaIoFileSystem.delete(), because it
can delete readonly files and can tolerate some
concurrent filesystem modifications.

Fixes #5513

Change-Id: I5d2720afff8dfd3725880a6d7408d22de5935c3d

JavaIoFileSystem.delete() now uses
Files.deleteIfExists() to delete files and empty
directories.

The old code used to have a race condition: a
file could have been deleted or changed to a
non-directory between checking it's a directory
and listing its contents.

WindowsFileSystem.delete() now uses the
DeletePath JNI method (wrapped by
WindowsFileOperations.deletePath) which is more
robust than JavaIoFileSystem.delete(), because it
can delete readonly files and can tolerate some
concurrent filesystem modifications.

Fixes bazelbuild#5513

Change-Id: I5d2720afff8dfd3725880a6d7408d22de5935c3d
Copy link
Member

@philwo philwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Thanks for this. :)

@laszlocsomor
Copy link
Contributor Author

Thanks!

@bazel-io bazel-io closed this in 49d2031 Jul 9, 2018
@laszlocsomor laszlocsomor deleted the vfs-race branch July 9, 2018 08:27
werkt pushed a commit to werkt/bazel that referenced this pull request Aug 2, 2018
JavaIoFileSystem.delete() now uses
Files.deleteIfExists() to delete files and empty
directories.

The old code used to have a race condition: a
file could have been deleted or changed to a
non-directory between checking it's a directory
and listing its contents.

WindowsFileSystem.delete() now uses the
DeletePath JNI method (wrapped by
WindowsFileOperations.deletePath) which is more
robust than JavaIoFileSystem.delete(), because it
can delete readonly files and can tolerate some
concurrent filesystem modifications.

Fixes bazelbuild#5513

Change-Id: I5d2720afff8dfd3725880a6d7408d22de5935c3d

Closes bazelbuild#5527.

Change-Id: I5d2720afff8dfd3725880a6d7408d22de5935c3d
PiperOrigin-RevId: 203720575
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants