From d2920e32ec7f3f8551a693d33c17b19f1b802145 Mon Sep 17 00:00:00 2001 From: Yun Peng Date: Tue, 27 Nov 2018 02:07:55 -0800 Subject: [PATCH] Revert "WindowsFileSystem: open files with delete-sharing" This reverts commit 1a955020ff7a27b9d49c83aef89e0991380daa5e. Related: https://github.com/bazelbuild/bazel/issues/6731 Closes #6732. PiperOrigin-RevId: 222958770 --- .../build/lib/vfs/AbstractFileSystem.java | 109 +++--------------- .../build/lib/windows/WindowsFileSystem.java | 20 ---- .../build/lib/vfs/JavaIoFileSystemTest.java | 44 ------- .../lib/windows/WindowsFileSystemTest.java | 57 --------- 4 files changed, 16 insertions(+), 214 deletions(-) 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 539cd79d98487d..771e0735c889c4 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 @@ -23,8 +23,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.nio.channels.FileChannel; -import javax.annotation.Nullable; /** This class implements the FileSystem interface using direct calls to the UNIX filesystem. */ @ThreadSafe @@ -58,7 +56,7 @@ protected InputStream getInputStream(Path path) throws IOException { } /** Returns either normal or profiled FileInputStream. */ - private InputStream createFileInputStream(Path path) throws IOException { + private InputStream createFileInputStream(Path path) throws FileNotFoundException { final String name = path.toString(); if (profiler.isActive() && (profiler.isProfiling(ProfilerTask.VFS_READ) @@ -66,41 +64,34 @@ private InputStream createFileInputStream(Path path) throws IOException { long startTime = Profiler.nanoTimeMaybe(); try { // Replace default FileInputStream instance with the custom one that does profiling. - return new ProfiledInputStream(name, newFileInputStream(name)); + return new ProfiledFileInputStream(name); } finally { profiler.logSimpleTask(startTime, ProfilerTask.VFS_OPEN, name); } } else { // Use normal FileInputStream instance if profiler is not enabled. - return newFileInputStream(name); + return new FileInputStream(path.toString()); } } - 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 IOException { + protected OutputStream createFileOutputStream(Path path, boolean append) + throws FileNotFoundException { 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, newFileOutputStream(name, append)); + return new ProfiledFileOutputStream(name, append); } finally { profiler.logSimpleTask(startTime, ProfilerTask.VFS_OPEN, name); } } else { - return newFileOutputStream(name, append); + return new FileOutputStream(name, append); } } @@ -119,34 +110,12 @@ protected OutputStream getOutputStream(Path path, boolean append) throws IOExcep } } - private static final class ProfiledInputStream extends InputStream implements - FileChannelSupplier { + private static final class ProfiledFileInputStream extends FileInputStream { private final String name; - private final InputStream stm; - public ProfiledInputStream(String name, InputStream stm) { + public ProfiledFileInputStream(String name) throws FileNotFoundException { + super(name); 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 @@ -155,7 +124,7 @@ public int read() throws IOException { try { // Note that FileInputStream#read() does *not* call any of our overridden methods, // so there's no concern with double counting here. - return stm.read(); + return super.read(); } finally { profiler.logSimpleTask(startTime, ProfilerTask.VFS_READ, name); } @@ -170,55 +139,19 @@ public int read(byte[] b) throws IOException { public int read(byte[] b, int off, int len) throws IOException { long startTime = Profiler.nanoTimeMaybe(); try { - return stm.read(b, off, len); + return super.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); - } - - @Override - public FileChannel getChannel() { - return stm instanceof FileInputStream - ? ((FileInputStream) stm).getChannel() - : null; - } } - /** - * Interface to return a {@link FileChannel}. - */ - public interface FileChannelSupplier { - @Nullable - FileChannel getChannel(); - } - - private static final class ProfiledFileOutputStream extends OutputStream { + private static final class ProfiledFileOutputStream extends FileOutputStream { private final String name; - private final OutputStream stm; - public ProfiledFileOutputStream(String name, OutputStream stm) { + public ProfiledFileOutputStream(String name, boolean append) throws FileNotFoundException { + super(name, append); this.name = name; - this.stm = stm; - } - - @Override - public void close() throws IOException { - stm.close(); - } - - @Override - public void flush() throws IOException { - stm.flush(); } @Override @@ -230,17 +163,7 @@ public void write(byte[] b) throws IOException { public void write(byte[] b, int off, int len) throws IOException { long startTime = Profiler.nanoTimeMaybe(); try { - 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); + super.write(b, off, len); } 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 c1249cb627ab3a..d39173a5dba482 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,12 +27,8 @@ 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. */ @@ -55,22 +51,6 @@ public String getFileSystemType(Path path) { 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 f34980c8baf67e..cdcb22e4cff721 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,8 +23,6 @@ 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; @@ -138,46 +136,4 @@ private static List getSubDirectories(Path base, int loopi, int subDirecto } 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 861e4aff96ead0..89303fdbd557cc 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,9 +31,6 @@ 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; @@ -335,58 +332,4 @@ public String apply(File input) { 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); - } - } }