WindowsFileSystem: open files with delete-sharing
Open files using `Files.new{Input,Output}Stream`
instead of `new File{Input,Output}Stream`.
The new method opens files with deletion sharing,
the old one does not.
This patch will hopefully fix the class of errors
where Bazel fails to delete a file that's already
open.
Change-Id: I1a841ea6ba644ac09dde4838007ba1ecab46defb
Closes #6092.
Change-Id: I1a841ea6ba644ac09dde4838007ba1ecab46defb
PiperOrigin-RevId: 211976021
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java
index 771e073..16848c2 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java
@@ -56,7 +56,7 @@
}
/** Returns either normal or profiled FileInputStream. */
- private InputStream createFileInputStream(Path path) throws FileNotFoundException {
+ private InputStream createFileInputStream(Path path) throws IOException {
final String name = path.toString();
if (profiler.isActive()
&& (profiler.isProfiling(ProfilerTask.VFS_READ)
@@ -64,34 +64,41 @@
long startTime = Profiler.nanoTimeMaybe();
try {
// Replace default FileInputStream instance with the custom one that does profiling.
- return new ProfiledFileInputStream(name);
+ return new ProfiledFileInputStream(name, newFileInputStream(name));
} finally {
profiler.logSimpleTask(startTime, ProfilerTask.VFS_OPEN, name);
}
} else {
// Use normal FileInputStream instance if profiler is not enabled.
- return new FileInputStream(path.toString());
+ return newFileInputStream(name);
}
}
+ protected InputStream newFileInputStream(String path) throws IOException {
+ return new FileInputStream(path);
+ }
+
+ protected OutputStream newFileOutputStream(String path, boolean append) throws IOException {
+ return new FileOutputStream(path, append);
+ }
+
/**
* Returns either normal or profiled FileOutputStream. Should be used by subclasses to create
* default OutputStream instance.
*/
- protected OutputStream createFileOutputStream(Path path, boolean append)
- throws FileNotFoundException {
+ protected OutputStream createFileOutputStream(Path path, boolean append) throws IOException {
final String name = path.toString();
if (profiler.isActive()
&& (profiler.isProfiling(ProfilerTask.VFS_WRITE)
|| profiler.isProfiling(ProfilerTask.VFS_OPEN))) {
long startTime = Profiler.nanoTimeMaybe();
try {
- return new ProfiledFileOutputStream(name, append);
+ return new ProfiledFileOutputStream(name, newFileOutputStream(name, append));
} finally {
profiler.logSimpleTask(startTime, ProfilerTask.VFS_OPEN, name);
}
} else {
- return new FileOutputStream(name, append);
+ return newFileOutputStream(name, append);
}
}
@@ -110,12 +117,33 @@
}
}
- private static final class ProfiledFileInputStream extends FileInputStream {
+ private static final class ProfiledFileInputStream extends InputStream {
private final String name;
+ private final InputStream stm;
- public ProfiledFileInputStream(String name) throws FileNotFoundException {
- super(name);
+ public ProfiledFileInputStream(String name, InputStream stm) {
this.name = name;
+ this.stm = stm;
+ }
+
+ @Override
+ public int available() throws IOException {
+ return stm.available();
+ }
+
+ @Override
+ public void close() throws IOException {
+ stm.close();
+ }
+
+ @Override
+ public void mark(int readlimit) {
+ stm.mark(readlimit);
+ }
+
+ @Override
+ public boolean markSupported() {
+ return stm.markSupported();
}
@Override
@@ -124,7 +152,7 @@
try {
// Note that FileInputStream#read() does *not* call any of our overridden methods,
// so there's no concern with double counting here.
- return super.read();
+ return stm.read();
} finally {
profiler.logSimpleTask(startTime, ProfilerTask.VFS_READ, name);
}
@@ -139,19 +167,40 @@
public int read(byte[] b, int off, int len) throws IOException {
long startTime = Profiler.nanoTimeMaybe();
try {
- return super.read(b, off, len);
+ return stm.read(b, off, len);
} finally {
profiler.logSimpleTask(startTime, ProfilerTask.VFS_READ, name);
}
}
+
+ @Override
+ public void reset() throws IOException {
+ stm.reset();
+ }
+
+ @Override
+ public long skip(long n) throws IOException {
+ return stm.skip(n);
+ }
}
- private static final class ProfiledFileOutputStream extends FileOutputStream {
+ private static final class ProfiledFileOutputStream extends OutputStream {
private final String name;
+ private final OutputStream stm;
- public ProfiledFileOutputStream(String name, boolean append) throws FileNotFoundException {
- super(name, append);
+ public ProfiledFileOutputStream(String name, OutputStream stm) {
this.name = name;
+ this.stm = stm;
+ }
+
+ @Override
+ public void close() throws IOException {
+ stm.close();
+ }
+
+ @Override
+ public void flush() throws IOException {
+ stm.flush();
}
@Override
@@ -163,7 +212,17 @@
public void write(byte[] b, int off, int len) throws IOException {
long startTime = Profiler.nanoTimeMaybe();
try {
- super.write(b, off, len);
+ stm.write(b, off, len);
+ } finally {
+ profiler.logSimpleTask(startTime, ProfilerTask.VFS_WRITE, name);
+ }
+ }
+
+ @Override
+ public void write(int b) throws IOException {
+ long startTime = Profiler.nanoTimeMaybe();
+ try {
+ stm.write(b);
} finally {
profiler.logSimpleTask(startTime, ProfilerTask.VFS_WRITE, name);
}
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 d39173a..c1249cb 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
@@ -27,8 +27,12 @@
import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
import java.nio.file.Files;
import java.nio.file.LinkOption;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
import java.nio.file.attribute.DosFileAttributes;
/** File system implementation for Windows. */
@@ -51,6 +55,22 @@
return "ntfs";
}
+ private static java.nio.file.Path getNioPath(String path) {
+ return Paths.get(path);
+ }
+
+ @Override
+ protected InputStream newFileInputStream(String path) throws IOException {
+ return Files.newInputStream(getNioPath(path));
+ }
+
+ @Override
+ protected OutputStream newFileOutputStream(String path, boolean append) throws IOException {
+ return append
+ ? Files.newOutputStream(getNioPath(path), StandardOpenOption.APPEND)
+ : Files.newOutputStream(getNioPath(path));
+ }
+
@Override
public boolean delete(Path path) throws IOException {
long startTime = Profiler.nanoTimeMaybe();
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/JavaIoFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/vfs/JavaIoFileSystemTest.java
index cdcb22e..f34980c 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/JavaIoFileSystemTest.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/JavaIoFileSystemTest.java
@@ -23,6 +23,8 @@
import com.google.devtools.build.lib.testutil.ManualClock;
import com.google.devtools.build.lib.testutil.TestUtils;
import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
import java.nio.file.Files;
import java.nio.file.LinkOption;
import java.nio.file.Paths;
@@ -136,4 +138,46 @@
}
return subDirs;
}
+
+ @Test
+ public void testDeleteFileOpenForReading() throws Exception {
+ Path path = absolutize("foo.txt");
+ FileSystemUtils.writeIsoLatin1(path, "foo");
+ // Sanity check: attempt reading from the file.
+ try (InputStream is = path.getInputStream()) {
+ byte[] contents = new byte[3];
+ assertThat(is.read(contents)).isEqualTo(3);
+ assertThat(contents).isEqualTo(new byte[] {'f', 'o', 'o'});
+ }
+ // Actual test: attempt deleting the file while open, then reading from it.
+ try (InputStream is = path.getInputStream()) {
+ assertThat(path.delete()).isTrue();
+ assertThat(path.exists()).isFalse();
+
+ byte[] contents = new byte[3];
+ assertThat(is.read(contents)).isEqualTo(3);
+ assertThat(contents).isEqualTo(new byte[] {'f', 'o', 'o'});
+ }
+ }
+
+ @Test
+ public void testDeleteFileOpenForWriting() throws Exception {
+ Path path = absolutize("foo.txt");
+ // Sanity check: attempt writing to the file.
+ assertThat(path.exists()).isFalse();
+ try (OutputStream os = path.getOutputStream(/* append */ false)) {
+ os.write(new byte[] {'b', 'a', 'r'});
+ }
+ try (InputStream is = path.getInputStream()) {
+ byte[] contents = new byte[3];
+ assertThat(is.read(contents)).isEqualTo(3);
+ assertThat(contents).isEqualTo(new byte[] {'b', 'a', 'r'});
+ }
+ // Actual test: attempt deleting the file while open, then writing to it.
+ try (OutputStream os = path.getOutputStream(/* append */ false)) {
+ assertThat(path.delete()).isTrue();
+ assertThat(path.exists()).isFalse();
+ os.write(new byte[] {'b', 'a', 'r'});
+ }
+ }
}
diff --git a/src/test/java/com/google/devtools/build/lib/windows/WindowsFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/windows/WindowsFileSystemTest.java
index 89303fd..861e4af 100644
--- a/src/test/java/com/google/devtools/build/lib/windows/WindowsFileSystemTest.java
+++ b/src/test/java/com/google/devtools/build/lib/windows/WindowsFileSystemTest.java
@@ -31,6 +31,9 @@
import com.google.devtools.build.lib.windows.util.WindowsTestUtil;
import java.io.File;
import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.nio.file.AccessDeniedException;
import java.nio.file.Files;
import java.util.Arrays;
import java.util.HashMap;
@@ -332,4 +335,58 @@
assertThat(p.isFile()).isTrue();
}
}
+
+ @Test
+ public void testDeleteFileOpenForReading() throws Exception {
+ testUtil.scratchFile("foo.txt", "foo");
+ Path path = testUtil.createVfsPath(fs, "foo.txt");
+ // Sanity check: attempt reading from the file.
+ try (InputStream is = path.getInputStream()) {
+ byte[] contents = new byte[3];
+ assertThat(is.read(contents)).isEqualTo(3);
+ assertThat(contents).isEqualTo(new byte[] {'f', 'o', 'o'});
+ }
+ // Actual test: attempt deleting the file while open, then reading from it.
+ try (InputStream is = path.getInputStream()) {
+ assertThat(path.delete()).isTrue();
+ assertThat(path.exists()).isFalse();
+
+ byte[] contents = new byte[3];
+ assertThat(is.read(contents)).isEqualTo(3);
+ assertThat(contents).isEqualTo(new byte[] {'f', 'o', 'o'});
+ }
+ }
+
+ @Test
+ public void testDeleteFileOpenForWriting() throws Exception {
+ testUtil.scratchFile("bar.txt", "bar");
+ Path path = testUtil.createVfsPath(fs, "bar.txt");
+ // Sanity check: attempt writing to the file.
+ try (InputStream is = path.getInputStream()) {
+ byte[] contents = new byte[3];
+ assertThat(is.read(contents)).isEqualTo(3);
+ assertThat(contents).isEqualTo(new byte[] {'b', 'a', 'r'});
+ }
+ // Actual test: attempt deleting the file while open, then writing to it.
+ try (OutputStream os = path.getOutputStream(/* append */ false)) {
+ assertThat(path.delete()).isTrue();
+ assertThat(path.exists()).isFalse();
+ os.write(new byte[] {'b', 'a', 'r'});
+ }
+ }
+
+ @Test
+ public void testCannotOpenDirectoryAsFile() throws Exception {
+ testUtil.scratchFile("foo/bar.txt", "bar");
+ Path file = testUtil.createVfsPath(fs, "foo/bar.txt");
+ Path dir = file.getParentDirectory();
+ // Sanity check: we can open the file.
+ try (InputStream is = file.getInputStream()) {}
+ assertThat(dir.isDirectory()).isTrue();
+ try (InputStream is = dir.getInputStream()) {
+ fail("Expected failure");
+ } catch (IOException e) {
+ assertThat(e).isInstanceOf(AccessDeniedException.class);
+ }
+ }
}
