Index: oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectory.java =================================================================== --- oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectory.java (revision 1675344) +++ oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectory.java (working copy) @@ -23,6 +23,7 @@ import java.io.IOException; import java.io.InputStream; import java.util.Collection; +import java.util.Iterator; import java.util.List; import com.google.common.primitives.Ints; @@ -30,6 +31,7 @@ import org.apache.jackrabbit.oak.api.PropertyState; import org.apache.jackrabbit.oak.api.Type; import org.apache.jackrabbit.oak.spi.state.NodeBuilder; +import org.apache.lucene.store.AlreadyClosedException; import org.apache.lucene.store.Directory; import org.apache.lucene.store.IOContext; import org.apache.lucene.store.IndexInput; @@ -37,6 +39,7 @@ import org.apache.lucene.store.Lock; import org.apache.lucene.store.LockFactory; import org.apache.lucene.store.NoLockFactory; +import org.apache.lucene.util.WeakIdentityMap; import static com.google.common.base.Preconditions.checkElementIndex; import static com.google.common.base.Preconditions.checkNotNull; @@ -329,29 +332,41 @@ private static class OakIndexInput extends IndexInput { private final OakIndexFile file; + private boolean isClone = false; + private final WeakIdentityMap clones; public OakIndexInput(String name, NodeBuilder file) { super(name); this.file = new OakIndexFile(name, file); + clones = WeakIdentityMap.newConcurrentHashMap(); } private OakIndexInput(OakIndexInput that) { super(that.toString()); this.file = new OakIndexFile(that.file); + clones = null; } @Override public OakIndexInput clone() { - return new OakIndexInput(this); + // TODO : shouldn't we use super#clone ? + OakIndexInput clonedIndexInput = new OakIndexInput(this); + clonedIndexInput.isClone = true; + if (clones != null) { + clones.put(clonedIndexInput, Boolean.TRUE); + } + return clonedIndexInput; } @Override public void readBytes(byte[] b, int o, int n) throws IOException { + checkNotClosed(); file.readBytes(b, o, n); } @Override public byte readByte() throws IOException { + checkNotClosed(); byte[] b = new byte[1]; readBytes(b, 0, 1); return b[0]; @@ -359,16 +374,19 @@ @Override public void seek(long pos) throws IOException { + checkNotClosed(); file.seek(pos); } @Override public long length() { + checkNotClosed(); return file.length; } @Override public long getFilePointer() { + checkNotClosed(); return file.position; } @@ -376,8 +394,22 @@ public void close() { file.blob = null; file.data = null; + + if (clones != null) { + for (Iterator it = clones.keyIterator(); it.hasNext();) { + final OakIndexInput clone = it.next(); + assert clone.isClone; + clone.close(); + } + } } + private void checkNotClosed() { + if (file.blob == null && file.data == null) { + throw new AlreadyClosedException("Already closed: " + this); + } + } + } private final class OakIndexOutput extends IndexOutput { Index: oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectoryTest.java =================================================================== --- oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectoryTest.java (revision 1675344) +++ oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectoryTest.java (working copy) @@ -31,6 +31,7 @@ import org.apache.jackrabbit.oak.plugins.memory.PropertyStates; import org.apache.jackrabbit.oak.spi.state.NodeBuilder; import org.apache.jackrabbit.oak.spi.state.NodeState; +import org.apache.lucene.store.AlreadyClosedException; import org.apache.lucene.store.Directory; import org.apache.lucene.store.IOContext; import org.apache.lucene.store.IndexInput; @@ -45,6 +46,7 @@ import static org.apache.jackrabbit.oak.plugins.nodetype.write.InitialContent.INITIAL_CONTENT; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; public class OakDirectoryTest { private Random rnd = new Random(); @@ -103,9 +105,130 @@ file.setProperty(PropertyStates.createProperty("jcr:data", blobs, Type.BINARIES)); IndexInput input = dir.openInput("test.txt", IOContext.DEFAULT); - assertEquals((long)blobSize * (dataSize - 1), input.length()); + assertEquals((long) blobSize * (dataSize - 1), input.length()); } + @Test + public void testCloseOnOriginalIndexInput() throws Exception { + Directory dir = createDir(builder); + NodeBuilder file = builder.child(INDEX_DATA_CHILD_NAME).child("test.txt"); + int dataSize = 1024; + List blobs = new ArrayList(dataSize); + for (int i = 0; i < dataSize; i++) { + blobs.add(new ArrayBasedBlob(new byte[0])); + } + file.setProperty(PropertyStates.createProperty("jcr:data", blobs, Type.BINARIES)); + IndexInput input = dir.openInput("test.txt", IOContext.DEFAULT); + input.close(); + assertClosed(input); + } + + @Test + public void testCloseOnClonedIndexInputs() throws Exception { + Directory dir = createDir(builder); + NodeBuilder file = builder.child(INDEX_DATA_CHILD_NAME).child("test.txt"); + int dataSize = 1024; + List blobs = new ArrayList(dataSize); + for (int i = 0; i < dataSize; i++) { + blobs.add(new ArrayBasedBlob(new byte[0])); + } + file.setProperty(PropertyStates.createProperty("jcr:data", blobs, Type.BINARIES)); + IndexInput input = dir.openInput("test.txt", IOContext.DEFAULT); + IndexInput clone1 = input.clone(); + IndexInput clone2 = input.clone(); + input.close(); + assertClosed(input); + assertClosed(clone1); + assertClosed(clone2); + } + + private void assertClosed(IndexInput input) throws IOException { + try { + input.length(); + fail("cannot use IndexInput once closed"); + } catch (AlreadyClosedException e) { + // expected exception + } + try { + input.seek(0); + fail("cannot use IndexInput once closed"); + } catch (AlreadyClosedException e) { + // expected exception + } + try { + input.getFilePointer(); + fail("cannot use IndexInput once closed"); + } catch (AlreadyClosedException e) { + // expected exception + } + try { + input.readInt(); + fail("cannot use IndexInput once closed"); + } catch (AlreadyClosedException e) { + // expected exception + } + try { + input.readShort(); + fail("cannot use IndexInput once closed"); + } catch (AlreadyClosedException e) { + // expected exception + } + try { + input.readLong(); + fail("cannot use IndexInput once closed"); + } catch (AlreadyClosedException e) { + // expected exception + } + try { + input.readByte(); + fail("cannot use IndexInput once closed"); + } catch (AlreadyClosedException e) { + // expected exception + } + try { + input.readString(); + fail("cannot use IndexInput once closed"); + } catch (AlreadyClosedException e) { + // expected exception + } + try { + input.readStringSet(); + fail("cannot use IndexInput once closed"); + } catch (AlreadyClosedException e) { + // expected exception + } + try { + input.readStringStringMap(); + fail("cannot use IndexInput once closed"); + } catch (AlreadyClosedException e) { + // expected exception + } + try { + input.readVInt(); + fail("cannot use IndexInput once closed"); + } catch (AlreadyClosedException e) { + // expected exception + } + try { + input.readVLong(); + fail("cannot use IndexInput once closed"); + } catch (AlreadyClosedException e) { + // expected exception + } + try { + input.readBytes(null, 0, 0); + fail("cannot use IndexInput once closed"); + } catch (AlreadyClosedException e) { + // expected exception + } + try { + input.readBytes(null, 0, 0, false); + fail("cannot use IndexInput once closed"); + } catch (AlreadyClosedException e) { + // expected exception + } + } + byte[] assertWrites(Directory dir, int blobSize) throws IOException { byte[] data = randomBytes(fileSize); IndexOutput o = dir.createOutput("test", IOContext.DEFAULT);