Skip to content

Commit

Permalink
LUCENE-9423: Handle exc in NIOFSDirectory#openInput (#1658)
Browse files Browse the repository at this point in the history
If we fail to get the size of a file in the constructor of 
NIOFSIndexInput, then we will leak a FileChannel opened in
NIOFSDirectory#openInput.
  • Loading branch information
dnhatn authored Jul 9, 2020
1 parent 4ae976b commit 20ec57a
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 1 deletion.
13 changes: 12 additions & 1 deletion lucene/core/src/java/org/apache/lucene/store/NIOFSDirectory.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import java.nio.file.StandardOpenOption;
import java.util.concurrent.Future; // javadoc

import org.apache.lucene.util.IOUtils;

/**
* An {@link FSDirectory} implementation that uses java.nio's FileChannel's
* positional read, which allows multiple threads to read from the same file
Expand Down Expand Up @@ -79,7 +81,16 @@ public IndexInput openInput(String name, IOContext context) throws IOException {
ensureCanRead(name);
Path path = getDirectory().resolve(name);
FileChannel fc = FileChannel.open(path, StandardOpenOption.READ);
return new NIOFSIndexInput("NIOFSIndexInput(path=\"" + path + "\")", fc, context);
boolean success = false;
try {
final NIOFSIndexInput indexInput = new NIOFSIndexInput("NIOFSIndexInput(path=\"" + path + "\")", fc, context);
success = true;
return indexInput;
} finally {
if (success == false) {
IOUtils.closeWhileHandlingException(fc);
}
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,17 @@


import java.io.IOException;
import java.net.URI;
import java.nio.channels.FileChannel;
import java.nio.file.FileSystem;
import java.nio.file.OpenOption;
import java.nio.file.Path;
import java.nio.file.attribute.FileAttribute;
import java.util.Set;

import org.apache.lucene.mockfile.FilterFileChannel;
import org.apache.lucene.mockfile.FilterPath;
import org.apache.lucene.mockfile.LeakFS;

/**
* Tests NIOFSDirectory
Expand All @@ -29,4 +39,29 @@ public class TestNIOFSDirectory extends BaseDirectoryTestCase {
protected Directory getDirectory(Path path) throws IOException {
return new NIOFSDirectory(path);
}

public void testHandleExceptionInConstructor() throws Exception {
Path path = createTempDir().toRealPath();
final LeakFS leakFS = new LeakFS(path.getFileSystem()) {
@Override
public FileChannel newFileChannel(Path path, Set<? extends OpenOption> options,
FileAttribute<?>... attrs) throws IOException {
return new FilterFileChannel(super.newFileChannel(path, options, attrs)) {
@Override
public long size() throws IOException {
throw new IOException("simulated");
}
};
}
};
FileSystem fs = leakFS.getFileSystem(URI.create("file:///"));
Path wrapped = new FilterPath(path, fs);
try (Directory dir = new NIOFSDirectory(wrapped)) {
try (IndexOutput out = dir.createOutput("test.bin", IOContext.DEFAULT)) {
out.writeString("hello");
}
final IOException error = expectThrows(IOException.class, () -> dir.openInput("test.bin", IOContext.DEFAULT));
assertEquals("simulated", error.getMessage());
}
}
}

0 comments on commit 20ec57a

Please sign in to comment.