diff options
author | Laszlo Csomor <laszlocsomor@google.com> | 2018-07-09 00:53:34 -0700 |
---|---|---|
committer | Copybara-Service <copybara-piper@google.com> | 2018-07-09 00:54:56 -0700 |
commit | 49d2031743231a979297bfc5d0a4463fabc0e21f (patch) | |
tree | 680a8c449200db31a7fdb89184abf8539a49af3d /src/main/java/com/google/devtools/build/lib/windows | |
parent | 7d9eedf969a326320e996baccccd4ae04f1e2deb (diff) |
vfs: fix race condition in JavaIoFileSystem.delete
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 https://github.com/bazelbuild/bazel/issues/5513
Change-Id: I5d2720afff8dfd3725880a6d7408d22de5935c3d
Closes #5527.
Change-Id: I5d2720afff8dfd3725880a6d7408d22de5935c3d
PiperOrigin-RevId: 203720575
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/windows')
3 files changed, 44 insertions, 0 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/windows/BUILD b/src/main/java/com/google/devtools/build/lib/windows/BUILD index 4b6e08c73a..61eb83acee 100644 --- a/src/main/java/com/google/devtools/build/lib/windows/BUILD +++ b/src/main/java/com/google/devtools/build/lib/windows/BUILD @@ -33,6 +33,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib:os_util", "//src/main/java/com/google/devtools/build/lib/clock", "//src/main/java/com/google/devtools/build/lib/concurrent", + "//src/main/java/com/google/devtools/build/lib/profiler", "//src/main/java/com/google/devtools/build/lib/shell", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/windows/jni", diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java index 0fd7028d6b..869ba85d62 100644 --- a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java @@ -15,6 +15,8 @@ package com.google.devtools.build.lib.windows; import com.google.common.annotations.VisibleForTesting; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; +import com.google.devtools.build.lib.profiler.Profiler; +import com.google.devtools.build.lib.profiler.ProfilerTask; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.FileStatus; import com.google.devtools.build.lib.vfs.JavaIoFileSystem; @@ -49,6 +51,20 @@ public class WindowsFileSystem extends JavaIoFileSystem { } @Override + public boolean delete(Path path) throws IOException { + long startTime = Profiler.nanoTimeMaybe(); + try { + return WindowsFileOperations.deletePath(path.getPathString()); + } catch (java.nio.file.DirectoryNotEmptyException e) { + throw new IOException(path.getPathString() + ERR_DIRECTORY_NOT_EMPTY); + } catch (java.nio.file.AccessDeniedException e) { + throw new IOException(path.getPathString() + ERR_PERMISSION_DENIED); + } finally { + profiler.logSimpleTask(startTime, ProfilerTask.VFS_DELETE, path.getPathString()); + } + } + + @Override protected void createSymbolicLink(Path linkPath, PathFragment targetFragment) throws IOException { Path targetPath = targetFragment.isAbsolute() diff --git a/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsFileOperations.java b/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsFileOperations.java index 32868c204d..6a16ab8aab 100644 --- a/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsFileOperations.java +++ b/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsFileOperations.java @@ -49,12 +49,21 @@ public class WindowsFileOperations { private static final int IS_JUNCTION_NO = 1; private static final int IS_JUNCTION_ERROR = 2; + // Keep DELETE_PATH_* values in sync with src/main/native/windows/file.cc. + private static final int DELETE_PATH_SUCCESS = 0; + private static final int DELETE_PATH_DOES_NOT_EXIST = 1; + private static final int DELETE_PATH_DIRECTORY_NOT_EMPTY = 2; + private static final int DELETE_PATH_ACCESS_DENIED = 3; + private static final int DELETE_PATH_ERROR = 4; + private static native int nativeIsJunction(String path, String[] error); private static native boolean nativeGetLongPath(String path, String[] result, String[] error); private static native boolean nativeCreateJunction(String name, String target, String[] error); + private static native int nativeDeletePath(String path, String[] error); + /** Determines whether `path` is a junction point or directory symlink. */ public static boolean isJunction(String path) throws IOException { WindowsJniLoader.loadJni(); @@ -121,4 +130,22 @@ public class WindowsFileOperations { String.format("Cannot create junction (name=%s, target=%s): %s", name, target, error[0])); } } + + public static boolean deletePath(String path) throws IOException { + WindowsJniLoader.loadJni(); + String[] error = new String[] {null}; + int result = nativeDeletePath(asLongPath(path), error); + switch (result) { + case DELETE_PATH_SUCCESS: + return true; + case DELETE_PATH_DOES_NOT_EXIST: + return false; + case DELETE_PATH_DIRECTORY_NOT_EMPTY: + throw new java.nio.file.DirectoryNotEmptyException(path); + case DELETE_PATH_ACCESS_DENIED: + throw new java.nio.file.AccessDeniedException(path); + default: + throw new IOException(String.format("Cannot delete path '%s': %s", path, error[0])); + } + } } |