Skip to content

Commit

Permalink
Revert "WindowsFileSystem: open files with delete-sharing"
Browse files Browse the repository at this point in the history 
This reverts commit 1a95502.

Related: #6731

Closes #6732.

PiperOrigin-RevId: 222958770
  • Loading branch information
@meteorcloudy
meteorcloudy authored and Copybara-Service committed Nov 27, 2018
1 parent 37dc726 commit d2920e3
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 214 deletions.
109 changes: 16 additions & 93 deletions src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -58,49 +56,42 @@ 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)
|| profiler.isProfiling(ProfilerTask.VFS_OPEN))) {
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);
}
}

Expand All @@ -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
Expand All @@ -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);
}
Expand All @@ -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
Expand All @@ -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);
}
Expand Down
20 changes: 0 additions & 20 deletions src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand All @@ -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();
Expand Down
44 changes: 0 additions & 44 deletions src/test/java/com/google/devtools/build/lib/vfs/JavaIoFileSystemTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -138,46 +136,4 @@ private static List<Path> 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'});
}
}
}
57 changes: 0 additions & 57 deletions src/test/java/com/google/devtools/build/lib/windows/WindowsFileSystemTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
}

0 comments on commit d2920e3

Please sign in to comment.