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

Bazel server: race condition in JavaIoFileSystem.delete() #5513

Closed
laszlocsomor opened this issue Jul 4, 2018 · 0 comments
Closed

Bazel server: race condition in JavaIoFileSystem.delete() #5513

laszlocsomor opened this issue Jul 4, 2018 · 0 comments
Assignees

Comments

@laszlocsomor
Copy link
Contributor

Description of the problem / feature request:

There's a race condition in:

if (file.isDirectory() && file.list().length > 0) {

If the directory is deleted or replaced by a file between the file.isDirectory() check and the file.list() call, then file.list() returns null and throws an NPE.

There may be other such race conditions in Bazel's VFS library.

What's the output of bazel info release?

release 0.15.0

@laszlocsomor laszlocsomor self-assigned this Jul 4, 2018
laszlocsomor added a commit to laszlocsomor/bazel that referenced this issue Jul 5, 2018
Implement a native version of the Java
Path.delete().

The new JNI function is more robust than Java IO
file deletion function because it can also delete
readonly files.

The new JNI function can tolerate some concurrent
modification errors, i.e. when other processes
move or remove the file we are trying to delete.
We'll use this method in WindowsFileSystem.delete
to reduce the likelihood of such concurrent
modifications crashing Bazel.

See bazelbuild#5513

Change-Id: I8a03568171b6911f688587b65ca14031ed86b54b
RELNOTES: none
laszlocsomor added a commit to laszlocsomor/bazel that referenced this issue Jul 5, 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).

Fixes bazelbuild#5513

Change-Id: Ie515e0ad35d0e6f94e1e9354d27cd7e9afd09d00
laszlocsomor added a commit to laszlocsomor/bazel that referenced this issue Jul 5, 2018
Implement a native version of the Java
Path.delete().

The new JNI function is more robust than Java IO
file deletion function because it can also delete
readonly files.

The new JNI function can tolerate some concurrent
modification errors, i.e. when other processes
move or remove the file we are trying to delete.
We'll use this method in WindowsFileSystem.delete
to reduce the likelihood of such concurrent
modifications crashing Bazel.

See bazelbuild#5513

Change-Id: I8a03568171b6911f688587b65ca14031ed86b54b
RELNOTES: none
laszlocsomor added a commit to laszlocsomor/bazel that referenced this issue Jul 5, 2018
Implement a native DeletePath method that can
delete files, directories, and junctions. The
method can tolerate when concurrent processes
delete the file.

The new JNI function is more robust than Java IO
file deletion function because it can also delete
readonly files.

See bazelbuild#5513

Change-Id: I8a03568171b6911f688587b65ca14031ed86b54b
RELNOTES: none
laszlocsomor added a commit to laszlocsomor/bazel that referenced this issue Jul 5, 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).

Fixes bazelbuild#5513

Change-Id: I5d2720afff8dfd3725880a6d7408d22de5935c3d
bazel-io pushed a commit that referenced this issue Jul 6, 2018
Implement a native DeletePath method that can
delete files, directories, and junctions. The
method should tolerate when concurrent
processes delete the file.

The new JNI function is more robust than Java IO
file deletion function because it can also delete
readonly files.

See #5513

Closes #5520.

Change-Id: I21ea36dd64960b294e2b51600273bf4290ad7c0f
PiperOrigin-RevId: 203448581
laszlocsomor added a commit to laszlocsomor/bazel that referenced this issue Jul 6, 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
werkt pushed a commit to werkt/bazel that referenced this issue Aug 2, 2018
Implement a native DeletePath method that can
delete files, directories, and junctions. The
method should tolerate when concurrent
processes delete the file.

The new JNI function is more robust than Java IO
file deletion function because it can also delete
readonly files.

See bazelbuild#5513

Closes bazelbuild#5520.

Change-Id: I21ea36dd64960b294e2b51600273bf4290ad7c0f
PiperOrigin-RevId: 203448581
werkt pushed a commit to werkt/bazel that referenced this issue 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
Projects
None yet
Development

No branches or pull requests

2 participants