Skip to content

Commit

Permalink
Automated rollback of commit 5cca89f.
Browse files Browse the repository at this point in the history
*** Reason for rollback ***

Break CI Windows nightly build: bazelbuild#3927
https://ci.bazel.io/blue/organizations/jenkins/Global%2Fbazel-tests/detail/bazel-tests/314/pipeline/31

*** Original change description ***

Remove synchronization from FileSystem implementations.

PiperOrigin-RevId: 173243523
  • Loading branch information
meteorcloudy authored and dslomov committed Oct 24, 2017
1 parent 5f5e2b4 commit e28d3af
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -90,16 +90,18 @@ protected OutputStream createFileOutputStream(Path path, boolean append)

@Override
protected OutputStream getOutputStream(Path path, boolean append) throws IOException {
try {
return createFileOutputStream(path, append);
} catch (FileNotFoundException e) {
// Why does it throw a *FileNotFoundException* if it can't write?
// That does not make any sense! And its in a completely different
// format than in other situations, no less!
if (e.getMessage().equals(path + ERR_PERMISSION_DENIED)) {
throw new FileAccessException(e.getMessage());
synchronized (path) {
try {
return createFileOutputStream(path, append);
} catch (FileNotFoundException e) {
// Why does it throw a *FileNotFoundException* if it can't write?
// That does not make any sense! And its in a completely different
// format than in other situations, no less!
if (e.getMessage().equals(path + ERR_PERMISSION_DENIED)) {
throw new FileAccessException(e.getMessage());
}
throw e;
}
throw e;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,24 +200,43 @@ public boolean isFilePathCaseSensitive() {

@Override
protected boolean createDirectory(Path path) throws IOException {
File file = getIoFile(path);
if (file.mkdir()) {
return true;
}

if (fileIsSymbolicLink(file)) {
throw new IOException(path + ERR_FILE_EXISTS);
} else if (file.isDirectory()) {
return false; // directory already existed
} else if (file.exists()) {
throw new IOException(path + ERR_FILE_EXISTS);
} else if (!file.getParentFile().exists()) {
throw new FileNotFoundException(path.getParentDirectory() + ERR_NO_SUCH_FILE_OR_DIR);
} else if (!file.getParentFile().canWrite()) {
throw new FileAccessException(path + ERR_PERMISSION_DENIED);
} else {
// Parent exists, is writable, yet we can't create our directory.
throw new FileNotFoundException(path.getParentDirectory() + ERR_NOT_A_DIRECTORY);
// We always synchronize on the current path before doing it on the parent path and file system
// path structure ensures that this locking order will never be reversed.
// When refactoring, check that subclasses still work as expected and there can be no
// deadlocks.
synchronized (path) {
File file = getIoFile(path);
if (file.mkdir()) {
return true;
}

// We will be checking the state of the parent path as well. Synchronize on it before
// attempting anything.
Path parentDirectory = path.getParentDirectory();
synchronized (parentDirectory) {
if (fileIsSymbolicLink(file)) {
throw new IOException(path + ERR_FILE_EXISTS);
}
if (file.isDirectory()) {
return false; // directory already existed
} else if (file.exists()) {
throw new IOException(path + ERR_FILE_EXISTS);
} else if (!file.getParentFile().exists()) {
throw new FileNotFoundException(path.getParentDirectory() + ERR_NO_SUCH_FILE_OR_DIR);
}
// Parent directory apparently exists - try to create our directory again - protecting
// against the case where parent directory would be created right before us obtaining
// synchronization lock.
if (file.mkdir()) {
return true; // Everything is fine finally.
} else if (!file.getParentFile().canWrite()) {
throw new FileAccessException(path + ERR_PERMISSION_DENIED);
} else {
// Parent exists, is writable, yet we can't create our directory.
throw new FileNotFoundException(path.getParentDirectory() + ERR_NOT_A_DIRECTORY);
}
}
}
}

Expand Down Expand Up @@ -272,6 +291,7 @@ protected PathFragment readSymbolicLink(Path path) throws IOException {

@Override
protected void renameTo(Path sourcePath, Path targetPath) throws IOException {
synchronized (sourcePath) {
File sourceFile = getIoFile(sourcePath);
File targetFile = getIoFile(targetPath);
if (!sourceFile.renameTo(targetFile)) {
Expand All @@ -292,6 +312,7 @@ protected void renameTo(Path sourcePath, Path targetPath) throws IOException {
throw new FileAccessException(sourcePath + " -> " + targetPath + ERR_PERMISSION_DENIED);
}
}
}
}

@Override
Expand All @@ -308,6 +329,7 @@ protected long getFileSize(Path path, boolean followSymlinks) throws IOException
protected boolean delete(Path path) throws IOException {
File file = getIoFile(path);
long startTime = Profiler.nanoTimeMaybe();
synchronized (path) {
try {
if (file.delete()) {
return true;
Expand All @@ -323,6 +345,7 @@ protected boolean delete(Path path) throws IOException {
} finally {
profiler.logSimpleTask(startTime, ProfilerTask.VFS_DELETE, file.getPath());
}
}
}

@Override
Expand Down

0 comments on commit e28d3af

Please sign in to comment.