Automated rollback of commit 82e68b75304438c96ff878a0c2b8d18b42002486.
Fixes #4322, #4306.
*** Reason for rollback ***
Introduces a deadlock (see https://github.com/bazelbuild/bazel/issues/4322)
*** Original change description ***
Make FileSystem operate on LocalPath instead of Path.
PiperOrigin-RevId: 179549866
diff --git a/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java b/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java
index 4a79ee1..7b82dcc 100644
--- a/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java
@@ -16,7 +16,6 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
-import com.google.common.util.concurrent.Striped;
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;
@@ -25,19 +24,18 @@
import com.google.devtools.build.lib.vfs.AbstractFileSystemWithCustomStat;
import com.google.devtools.build.lib.vfs.Dirent;
import com.google.devtools.build.lib.vfs.FileStatus;
-import com.google.devtools.build.lib.vfs.LocalPath;
+import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
-import java.util.concurrent.locks.Lock;
/**
* This class implements the FileSystem interface using direct calls to the UNIX filesystem.
*/
@ThreadSafe
public class UnixFileSystem extends AbstractFileSystemWithCustomStat {
- private final Striped<Lock> pathLock = Striped.lock(64);
public UnixFileSystem() {
}
@@ -102,7 +100,7 @@
}
@Override
- protected Collection<String> getDirectoryEntries(LocalPath path) throws IOException {
+ protected Collection<String> getDirectoryEntries(Path path) throws IOException {
String name = path.getPathString();
String[] entries;
long startTime = Profiler.nanoTimeMaybe();
@@ -119,7 +117,7 @@
}
@Override
- protected String resolveOneLink(LocalPath path) throws IOException {
+ protected PathFragment resolveOneLink(Path path) throws IOException {
// Beware, this seemingly simple code belies the complex specification of
// FileSystem.resolveOneLink().
return stat(path, false).isSymbolicLink()
@@ -147,7 +145,7 @@
}
@Override
- protected Collection<Dirent> readdir(LocalPath path, boolean followSymlinks) throws IOException {
+ protected Collection<Dirent> readdir(Path path, boolean followSymlinks) throws IOException {
String name = path.getPathString();
long startTime = Profiler.nanoTimeMaybe();
try {
@@ -166,12 +164,12 @@
}
@Override
- protected FileStatus stat(LocalPath path, boolean followSymlinks) throws IOException {
+ protected FileStatus stat(Path path, boolean followSymlinks) throws IOException {
return statInternal(path, followSymlinks);
}
@VisibleForTesting
- protected UnixFileStatus statInternal(LocalPath path, boolean followSymlinks) throws IOException {
+ protected UnixFileStatus statInternal(Path path, boolean followSymlinks) throws IOException {
String name = path.getPathString();
long startTime = Profiler.nanoTimeMaybe();
try {
@@ -187,7 +185,7 @@
// This is a performance optimization in the case where clients
// catch and don't re-throw.
@Override
- protected FileStatus statNullable(LocalPath path, boolean followSymlinks) {
+ protected FileStatus statNullable(Path path, boolean followSymlinks) {
String name = path.getPathString();
long startTime = Profiler.nanoTimeMaybe();
try {
@@ -201,16 +199,16 @@
}
@Override
- protected boolean exists(LocalPath path, boolean followSymlinks) {
+ protected boolean exists(Path path, boolean followSymlinks) {
return statNullable(path, followSymlinks) != null;
}
/**
- * Return true iff the {@code stat} of {@code path} resulted in an {@code ENOENT} or {@code
- * ENOTDIR} error.
+ * Return true iff the {@code stat} of {@code path} resulted in an {@code ENOENT}
+ * or {@code ENOTDIR} error.
*/
@Override
- protected FileStatus statIfFound(LocalPath path, boolean followSymlinks) throws IOException {
+ protected FileStatus statIfFound(Path path, boolean followSymlinks) throws IOException {
String name = path.getPathString();
long startTime = Profiler.nanoTimeMaybe();
try {
@@ -236,29 +234,29 @@
}
@Override
- protected boolean isReadable(LocalPath path) throws IOException {
+ protected boolean isReadable(Path path) throws IOException {
return (statInternal(path, true).getPermissions() & 0400) != 0;
}
@Override
- protected boolean isWritable(LocalPath path) throws IOException {
+ protected boolean isWritable(Path path) throws IOException {
return (statInternal(path, true).getPermissions() & 0200) != 0;
}
@Override
- protected boolean isExecutable(LocalPath path) throws IOException {
+ protected boolean isExecutable(Path path) throws IOException {
return (statInternal(path, true).getPermissions() & 0100) != 0;
}
/**
- * Adds or remove the bits specified in "permissionBits" to the permission mask of the file
- * specified by {@code path}. If the argument {@code add} is true, the specified permissions are
- * added, otherwise they are removed.
+ * Adds or remove the bits specified in "permissionBits" to the permission
+ * mask of the file specified by {@code path}. If the argument {@code add} is
+ * true, the specified permissions are added, otherwise they are removed.
*
* @throws IOException if there was an error writing the file's metadata
*/
- private void modifyPermissionBits(LocalPath path, int permissionBits, boolean add)
- throws IOException {
+ private void modifyPermissionBits(Path path, int permissionBits, boolean add)
+ throws IOException {
synchronized (path) {
int oldMode = statInternal(path, true).getPermissions();
int newMode = add ? (oldMode | permissionBits) : (oldMode & ~permissionBits);
@@ -267,39 +265,39 @@
}
@Override
- protected void setReadable(LocalPath path, boolean readable) throws IOException {
+ protected void setReadable(Path path, boolean readable) throws IOException {
modifyPermissionBits(path, 0400, readable);
}
@Override
- public void setWritable(LocalPath path, boolean writable) throws IOException {
+ public void setWritable(Path path, boolean writable) throws IOException {
modifyPermissionBits(path, 0200, writable);
}
@Override
- protected void setExecutable(LocalPath path, boolean executable) throws IOException {
+ protected void setExecutable(Path path, boolean executable) throws IOException {
modifyPermissionBits(path, 0111, executable);
}
@Override
- protected void chmod(LocalPath path, int mode) throws IOException {
+ protected void chmod(Path path, int mode) throws IOException {
synchronized (path) {
NativePosixFiles.chmod(path.toString(), mode);
}
}
@Override
- public boolean supportsModifications(LocalPath path) {
+ public boolean supportsModifications(Path path) {
return true;
}
@Override
- public boolean supportsSymbolicLinksNatively(LocalPath path) {
+ public boolean supportsSymbolicLinksNatively(Path path) {
return true;
}
@Override
- public boolean supportsHardLinksNatively(LocalPath path) {
+ public boolean supportsHardLinksNatively(Path path) {
return true;
}
@@ -309,10 +307,8 @@
}
@Override
- public boolean createDirectory(LocalPath path) throws IOException {
- Lock lock = getPathLock(path);
- lock.lock();
- try {
+ public boolean createDirectory(Path path) throws IOException {
+ synchronized (path) {
// Note: UNIX mkdir(2), FilesystemUtils.mkdir() and createDirectory all
// have different ways of representing failure!
if (NativePosixFiles.mkdir(path.toString(), 0777)) {
@@ -325,30 +321,25 @@
} else {
throw new IOException(path + " (File exists)");
}
- } finally {
- lock.unlock();
}
}
@Override
- protected void createSymbolicLink(LocalPath linkPath, String targetFragment) throws IOException {
- Lock lock = getPathLock(linkPath);
- lock.lock();
- try {
- NativePosixFiles.symlink(targetFragment, linkPath.toString());
- } finally {
- lock.unlock();
+ protected void createSymbolicLink(Path linkPath, PathFragment targetFragment)
+ throws IOException {
+ synchronized (linkPath) {
+ NativePosixFiles.symlink(targetFragment.toString(), linkPath.toString());
}
}
@Override
- protected String readSymbolicLink(LocalPath path) throws IOException {
+ protected PathFragment readSymbolicLink(Path path) throws IOException {
// Note that the default implementation of readSymbolicLinkUnchecked calls this method and thus
// is optimal since we only make one system call in here.
String name = path.toString();
long startTime = Profiler.nanoTimeMaybe();
try {
- return NativePosixFiles.readlink(name);
+ return PathFragment.create(NativePosixFiles.readlink(name));
} catch (IOException e) {
// EINVAL => not a symbolic link. Anything else is a real error.
throw e.getMessage().endsWith("(Invalid argument)") ? new NotASymlinkException(path) : e;
@@ -358,45 +349,38 @@
}
@Override
- public void renameTo(LocalPath sourcePath, LocalPath targetPath) throws IOException {
- Lock lock = getPathLock(sourcePath);
- lock.lock();
- try {
+ public void renameTo(Path sourcePath, Path targetPath) throws IOException {
+ synchronized (sourcePath) {
NativePosixFiles.rename(sourcePath.toString(), targetPath.toString());
- } finally {
- lock.unlock();
}
}
@Override
- protected long getFileSize(LocalPath path, boolean followSymlinks) throws IOException {
+ protected long getFileSize(Path path, boolean followSymlinks) throws IOException {
return stat(path, followSymlinks).getSize();
}
@Override
- public boolean delete(LocalPath path) throws IOException {
+ public boolean delete(Path path) throws IOException {
String name = path.toString();
long startTime = Profiler.nanoTimeMaybe();
- Lock lock = getPathLock(path);
- lock.lock();
- try {
- return NativePosixFiles.remove(name);
- } finally {
- lock.unlock();
- profiler.logSimpleTask(startTime, ProfilerTask.VFS_DELETE, name);
+ synchronized (path) {
+ try {
+ return NativePosixFiles.remove(name);
+ } finally {
+ profiler.logSimpleTask(startTime, ProfilerTask.VFS_DELETE, name);
+ }
}
}
@Override
- protected long getLastModifiedTime(LocalPath path, boolean followSymlinks) throws IOException {
+ protected long getLastModifiedTime(Path path, boolean followSymlinks) throws IOException {
return stat(path, followSymlinks).getLastModifiedTime();
}
@Override
- public void setLastModifiedTime(LocalPath path, long newTime) throws IOException {
- Lock lock = getPathLock(path);
- lock.lock();
- try {
+ public void setLastModifiedTime(Path path, long newTime) throws IOException {
+ synchronized (path) {
if (newTime == -1L) { // "now"
NativePosixFiles.utime(path.toString(), true, 0);
} else {
@@ -404,13 +388,11 @@
int unixTime = (int) (newTime / 1000);
NativePosixFiles.utime(path.toString(), false, unixTime);
}
- } finally {
- lock.unlock();
}
}
@Override
- public byte[] getxattr(LocalPath path, String name) throws IOException {
+ public byte[] getxattr(Path path, String name) throws IOException {
String pathName = path.toString();
long startTime = Profiler.nanoTimeMaybe();
try {
@@ -425,7 +407,7 @@
}
@Override
- protected byte[] getDigest(LocalPath path, HashFunction hashFunction) throws IOException {
+ protected byte[] getDigest(Path path, HashFunction hashFunction) throws IOException {
String name = path.toString();
long startTime = Profiler.nanoTimeMaybe();
try {
@@ -439,13 +421,8 @@
}
@Override
- protected void createFSDependentHardLink(LocalPath linkPath, LocalPath originalPath)
+ protected void createFSDependentHardLink(Path linkPath, Path originalPath)
throws IOException {
NativePosixFiles.link(originalPath.toString(), linkPath.toString());
}
-
- /** Returns a per-path lock. The lock is re-entrant. */
- protected Lock getPathLock(LocalPath path) {
- return pathLock.get(path);
- }
}
