Index: lucene/core/src/java/org/apache/lucene/store/FSDirectory.java =================================================================== --- lucene/core/src/java/org/apache/lucene/store/FSDirectory.java (revision 1458622) +++ lucene/core/src/java/org/apache/lucene/store/FSDirectory.java (working copy) @@ -17,6 +17,7 @@ * limitations under the License. */ +import java.io.Closeable; import java.io.File; import java.io.FileNotFoundException; import java.io.FilenameFilter; @@ -285,7 +286,8 @@ ensureOpen(); ensureCanWrite(name); - return new FSIndexOutput(this, name); + File file = new File(directory, name); + return new RandomAccessFileFSIndexOutput(this, name, new RandomAccessFile(file, "rw")); } protected void ensureCanWrite(String name) throws IOException { @@ -392,87 +394,22 @@ return chunkSize; } - /** Base class for reading input from a RandomAccessFile */ - protected abstract static class FSIndexInput extends BufferedIndexInput { - /** the underlying RandomAccessFile */ - protected final RandomAccessFile file; - boolean isClone = false; - /** maximum read length on a 32bit JVM to prevent incorrect OOM, see LUCENE-1566 */ - protected final int chunkSize; - /** start offset: non-zero in the slice case */ - protected final long off; - /** end offset (start+length) */ - protected final long end; - - /** Create a new FSIndexInput, reading the entire file from path */ - protected FSIndexInput(String resourceDesc, File path, IOContext context, int chunkSize) throws IOException { - super(resourceDesc, context); - this.file = new RandomAccessFile(path, "r"); - this.chunkSize = chunkSize; - this.off = 0L; - this.end = file.length(); - } - - /** Create a new FSIndexInput, representing a slice of an existing open file */ - protected FSIndexInput(String resourceDesc, RandomAccessFile file, long off, long length, int bufferSize, int chunkSize) { - super(resourceDesc, bufferSize); - this.file = file; - this.chunkSize = chunkSize; - this.off = off; - this.end = off + length; - this.isClone = true; // well, we are sorta? - } - - @Override - public void close() throws IOException { - // only close the file if this is not a clone - if (!isClone) { - file.close(); - } - } - - @Override - public FSIndexInput clone() { - FSIndexInput clone = (FSIndexInput)super.clone(); - clone.isClone = true; - return clone; - } - - @Override - public final long length() { - return end - off; - } - - /** Method used for testing. Returns true if the underlying - * file descriptor is valid. - */ - boolean isFDValid() throws IOException { - return file.getFD().valid(); - } - } - /** - * Writes output with {@link RandomAccessFile#write(byte[], int, int)} + * Writes output using a file accessor object supplied by a subclass */ - protected static class FSIndexOutput extends BufferedIndexOutput { + protected abstract static class FSIndexOutput extends BufferedIndexOutput { + final String name; private final FSDirectory parent; - private final String name; - private final RandomAccessFile file; private volatile boolean isOpen; // remember if the file is open, so that we don't try to close it more than once - public FSIndexOutput(FSDirectory parent, String name) throws IOException { + protected final Closeable resource; + + public FSIndexOutput(FSDirectory parent, String name, Closeable resource) throws IOException { this.parent = parent; this.name = name; - file = new RandomAccessFile(new File(parent.directory, name), "rw"); + this.resource = resource; isOpen = true; } - - /** output methods: */ - @Override - public void flushBuffer(byte[] b, int offset, int size) throws IOException { - assert isOpen; - file.write(b, offset, size); - } @Override public void close() throws IOException { @@ -487,18 +424,38 @@ isOpen = false; if (!success) { try { - file.close(); + resource.close(); } catch (Throwable t) { // Suppress so we don't mask original exception } } else { - file.close(); + resource.close(); } } } } - + + protected void ensureOpen() { + assert isOpen; + } + } + + protected static class RandomAccessFileFSIndexOutput extends FSIndexOutput { + private final RandomAccessFile file; + + public RandomAccessFileFSIndexOutput(FSDirectory parent, String name, RandomAccessFile file) throws IOException { + super(parent, name, file); + this.file = file; + } + @Override + protected void flushBuffer(byte[] b, int offset, int len) + throws IOException { + ensureOpen(); + file.write(b, offset, len); + } + + @Override public long length() throws IOException { return file.length(); } @@ -508,7 +465,7 @@ file.setLength(length); } } - + protected void fsync(String name) throws IOException { File fullFile = new File(directory, name); boolean success = false; Index: lucene/core/src/java/org/apache/lucene/store/MMapDirectory.java =================================================================== --- lucene/core/src/java/org/apache/lucene/store/MMapDirectory.java (revision 1458622) +++ lucene/core/src/java/org/apache/lucene/store/MMapDirectory.java (working copy) @@ -19,11 +19,11 @@ import java.io.IOException; import java.io.File; -import java.io.RandomAccessFile; import java.nio.ByteBuffer; import java.nio.channels.ClosedChannelException; // javadoc @link import java.nio.channels.FileChannel; import java.nio.channels.FileChannel.MapMode; +import java.nio.file.StandardOpenOption; import java.security.AccessController; import java.security.PrivilegedExceptionAction; @@ -133,7 +133,7 @@ this.chunkSizePower = 31 - Integer.numberOfLeadingZeros(maxChunkSize); assert this.chunkSizePower >= 0 && this.chunkSizePower <= 30; } - + /** * true, if this platform supports unmapping mmapped files. */ @@ -189,12 +189,9 @@ @Override public IndexInput openInput(String name, IOContext context) throws IOException { ensureOpen(); - File f = new File(getDirectory(), name); - RandomAccessFile raf = new RandomAccessFile(f, "r"); - try { - return new MMapIndexInput("MMapIndexInput(path=\"" + f + "\")", raf); - } finally { - raf.close(); + File file = new File(getDirectory(), name); + try (FileChannel c = FileChannel.open(file.toPath(), StandardOpenOption.READ)) { + return new MMapIndexInput("MMapIndexInput(path=\"" + file.toString() + "\")", c); } } @@ -218,8 +215,8 @@ private final class MMapIndexInput extends ByteBufferIndexInput { private final boolean useUnmapHack; - MMapIndexInput(String resourceDescription, RandomAccessFile raf) throws IOException { - super(resourceDescription, map(raf, 0, raf.length()), raf.length(), chunkSizePower, getUseUnmap()); + MMapIndexInput(String resourceDescription, FileChannel fc) throws IOException { + super(resourceDescription, map(fc, 0, fc.size()), fc.size(), chunkSizePower, getUseUnmap()); this.useUnmapHack = getUseUnmap(); } @@ -256,9 +253,9 @@ } /** Maps a file into a set of buffers */ - ByteBuffer[] map(RandomAccessFile raf, long offset, long length) throws IOException { + ByteBuffer[] map(FileChannel fc, long offset, long length) throws IOException { if ((length >>> chunkSizePower) >= Integer.MAX_VALUE) - throw new IllegalArgumentException("RandomAccessFile too big for chunk size: " + raf.toString()); + throw new IllegalArgumentException("RandomAccessFile too big for chunk size: " + fc.toString()); final long chunkSize = 1L << chunkSizePower; @@ -268,13 +265,12 @@ ByteBuffer buffers[] = new ByteBuffer[nrBuffers]; long bufferStart = 0L; - FileChannel rafc = raf.getChannel(); for (int bufNr = 0; bufNr < nrBuffers; bufNr++) { int bufSize = (int) ( (length > (bufferStart + chunkSize)) ? chunkSize : (length - bufferStart) ); - buffers[bufNr] = rafc.map(MapMode.READ_ONLY, offset + bufferStart, bufSize); + buffers[bufNr] = fc.map(MapMode.READ_ONLY, offset + bufferStart, bufSize); bufferStart += bufSize; } Index: lucene/core/src/java/org/apache/lucene/store/NIOFSDirectory.java =================================================================== --- lucene/core/src/java/org/apache/lucene/store/NIOFSDirectory.java (revision 1458622) +++ lucene/core/src/java/org/apache/lucene/store/NIOFSDirectory.java (working copy) @@ -20,10 +20,10 @@ import java.io.File; import java.io.EOFException; import java.io.IOException; -import java.io.RandomAccessFile; import java.nio.ByteBuffer; import java.nio.channels.ClosedChannelException; // javadoc @link import java.nio.channels.FileChannel; +import java.nio.file.StandardOpenOption; import java.util.concurrent.Future; // javadoc /** @@ -77,7 +77,9 @@ @Override public IndexInput openInput(String name, IOContext context) throws IOException { ensureOpen(); - return new NIOFSIndexInput(new File(getDirectory(), name), context, getReadChunkSize()); + File path = new File(getDirectory(), name); + FileChannel fc = FileChannel.open(path.toPath(), StandardOpenOption.READ); + return new NIOFSIndexInput("NIOFSIndexInput(path=\"" + path + "\")", fc, context, getReadChunkSize()); } @Override @@ -85,7 +87,7 @@ final IOContext context) throws IOException { ensureOpen(); final File path = new File(getDirectory(), name); - final RandomAccessFile descriptor = new RandomAccessFile(path, "r"); + final FileChannel descriptor = FileChannel.open(path.toPath(), StandardOpenOption.READ); return new Directory.IndexInputSlicer() { @Override @@ -95,7 +97,7 @@ @Override public IndexInput openSlice(String sliceDescription, long offset, long length) { - return new NIOFSIndexInput(sliceDescription, path, descriptor, descriptor.getChannel(), offset, + return new NIOFSIndexInput("NIOFSIndexInput(" + sliceDescription + " in path=\"" + path + "\" slice=" + offset + ":" + (offset+length) + ")", descriptor, offset, length, BufferedIndexInput.bufferSize(context), getReadChunkSize()); } }; @@ -104,22 +106,55 @@ /** * Reads bytes with {@link FileChannel#read(ByteBuffer, long)} */ - protected static class NIOFSIndexInput extends FSIndexInput { - + protected static class NIOFSIndexInput extends BufferedIndexInput { + /** the file channel we will read from */ + protected final FileChannel channel; + /** is this instance a clone and hence does not own the file to close it */ + boolean isClone = false; + /** maximum read length on a 32bit JVM to prevent incorrect OOM, see LUCENE-1566 */ + protected final int chunkSize; + /** start offset: non-zero in the slice case */ + protected final long off; + /** end offset (start+length) */ + protected final long end; + private ByteBuffer byteBuf; // wraps the buffer for NIO - final FileChannel channel; - - public NIOFSIndexInput(File path, IOContext context, int chunkSize) throws IOException { - super("NIOFSIndexInput(path=\"" + path + "\")", path, context, chunkSize); - channel = file.getChannel(); + public NIOFSIndexInput(String resourceDesc, FileChannel fc, IOContext context, int chunkSize) throws IOException { + super(resourceDesc, context); + this.channel = fc; + this.chunkSize = chunkSize; + this.off = 0L; + this.end = fc.size(); } - public NIOFSIndexInput(String sliceDescription, File path, RandomAccessFile file, FileChannel fc, long off, long length, int bufferSize, int chunkSize) { - super("NIOFSIndexInput(" + sliceDescription + " in path=\"" + path + "\" slice=" + off + ":" + (off+length) + ")", file, off, length, bufferSize, chunkSize); - channel = fc; - isClone = true; + public NIOFSIndexInput(String resourceDesc, FileChannel fc, long off, long length, int bufferSize, int chunkSize) { + super(resourceDesc, bufferSize); + this.channel = fc; + this.chunkSize = chunkSize; + this.off = off; + this.end = off + length; + this.isClone = true; } + + @Override + public void close() throws IOException { + if (!isClone) { + channel.close(); + } + } + + @Override + public NIOFSIndexInput clone() { + NIOFSIndexInput clone = (NIOFSIndexInput)super.clone(); + clone.isClone = false; + return clone; + } + + @Override + public final long length() { + return end - off; + } @Override protected void newBuffer(byte[] newBuffer) { @@ -186,5 +221,4 @@ @Override protected void seekInternal(long pos) throws IOException {} } - } Index: lucene/core/src/java/org/apache/lucene/store/SimpleFSDirectory.java =================================================================== --- lucene/core/src/java/org/apache/lucene/store/SimpleFSDirectory.java (revision 1458622) +++ lucene/core/src/java/org/apache/lucene/store/SimpleFSDirectory.java (working copy) @@ -55,7 +55,8 @@ public IndexInput openInput(String name, IOContext context) throws IOException { ensureOpen(); final File path = new File(directory, name); - return new SimpleFSIndexInput("SimpleFSIndexInput(path=\"" + path.getPath() + "\")", path, context, getReadChunkSize()); + RandomAccessFile raf = new RandomAccessFile(path, "r"); + return new SimpleFSIndexInput("SimpleFSIndexInput(path=\"" + path.getPath() + "\")", raf, context, getReadChunkSize()); } @Override @@ -83,15 +84,53 @@ * Reads bytes with {@link RandomAccessFile#seek(long)} followed by * {@link RandomAccessFile#read(byte[], int, int)}. */ - protected static class SimpleFSIndexInput extends FSIndexInput { - - public SimpleFSIndexInput(String resourceDesc, File path, IOContext context, int chunkSize) throws IOException { - super(resourceDesc, path, context, chunkSize); + protected static class SimpleFSIndexInput extends BufferedIndexInput { + /** the file channel we will read from */ + protected final RandomAccessFile file; + /** is this instance a clone and hence does not own the file to close it */ + boolean isClone = false; + /** maximum read length on a 32bit JVM to prevent incorrect OOM, see LUCENE-1566 */ + protected final int chunkSize; + /** start offset: non-zero in the slice case */ + protected final long off; + /** end offset (start+length) */ + protected final long end; + + public SimpleFSIndexInput(String resourceDesc, RandomAccessFile file, IOContext context, int chunkSize) throws IOException { + super(resourceDesc, context); + this.file = file; + this.chunkSize = chunkSize; + this.off = 0L; + this.end = file.length(); } public SimpleFSIndexInput(String resourceDesc, RandomAccessFile file, long off, long length, int bufferSize, int chunkSize) { - super(resourceDesc, file, off, length, bufferSize, chunkSize); + super(resourceDesc, bufferSize); + this.file = file; + this.chunkSize = chunkSize; + this.off = off; + this.end = off + length; + this.isClone = true; } + + @Override + public void close() throws IOException { + if (!isClone) { + file.close(); + } + } + + @Override + public SimpleFSIndexInput clone() { + SimpleFSIndexInput clone = (SimpleFSIndexInput)super.clone(); + clone.isClone = true; + return clone; + } + + @Override + public final long length() { + return end - off; + } /** IndexInput methods */ @Override @@ -136,5 +175,9 @@ @Override protected void seekInternal(long position) { } + + boolean isFDValid() throws IOException { + return file.getFD().valid(); + } } } Index: lucene/core/src/test/org/apache/lucene/store/TestBufferedIndexInput.java =================================================================== --- lucene/core/src/test/org/apache/lucene/store/TestBufferedIndexInput.java (revision 1458622) +++ lucene/core/src/test/org/apache/lucene/store/TestBufferedIndexInput.java (working copy) @@ -21,6 +21,9 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.OutputStream; +import java.io.RandomAccessFile; +import java.nio.channels.FileChannel; +import java.nio.file.StandardOpenOption; import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -93,12 +96,13 @@ writeBytes(tmpInputFile, TEST_FILE_LENGTH); // run test with chunk size of 10 bytes - runReadBytesAndClose(new SimpleFSIndexInput("SimpleFSIndexInput(path=\"" + tmpInputFile + "\")", tmpInputFile, - newIOContext(random()), 10), inputBufferSize, random()); + runReadBytesAndClose(new SimpleFSIndexInput("SimpleFSIndexInput(path=\"" + tmpInputFile + "\")", + new RandomAccessFile(tmpInputFile, "r"), newIOContext(random()), 10), inputBufferSize, random()); // run test with chunk size of 10 bytes - runReadBytesAndClose(new NIOFSIndexInput(tmpInputFile, - newIOContext(random()), 10), inputBufferSize, random()); + runReadBytesAndClose(new NIOFSIndexInput("NIOFSIndexInput(path=\"" + tmpInputFile + "\")", + FileChannel.open(tmpInputFile.toPath(), StandardOpenOption.READ), newIOContext(random()), 10), + inputBufferSize, random()); } private void runReadBytesAndClose(IndexInput input, int bufferSize, Random r)