You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by to...@apache.org on 2015/05/27 09:09:12 UTC
svn commit: r1681918 - in /jackrabbit/oak/trunk/oak-lucene/src:
main/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectory.java
test/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectoryTest.java
Author: tommaso
Date: Wed May 27 07:09:12 2015
New Revision: 1681918
URL: http://svn.apache.org/r1681918
Log:
OAK-2799 - close cloned OakIndexInput instances on close of main one
Modified:
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectory.java
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectoryTest.java
Modified: jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectory.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectory.java?rev=1681918&r1=1681917&r2=1681918&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectory.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectory.java Wed May 27 07:09:12 2015
@@ -21,6 +21,7 @@ import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.util.Collection;
+import java.util.Iterator;
import java.util.List;
import java.util.Set;
@@ -33,6 +34,7 @@ import org.apache.jackrabbit.oak.api.Pro
import org.apache.jackrabbit.oak.api.Type;
import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
import org.apache.jackrabbit.oak.util.PerfLogger;
+import org.apache.lucene.store.AlreadyClosedException;
import org.apache.lucene.store.Directory;
import org.apache.lucene.store.IOContext;
import org.apache.lucene.store.IndexInput;
@@ -40,6 +42,7 @@ import org.apache.lucene.store.IndexOutp
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 org.slf4j.LoggerFactory;
import static com.google.common.base.Preconditions.checkArgument;
@@ -364,29 +367,41 @@ class OakDirectory extends Directory {
private static class OakIndexInput extends IndexInput {
private final OakIndexFile file;
+ private boolean isClone = false;
+ private final WeakIdentityMap<OakIndexInput, Boolean> 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 call 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];
@@ -394,16 +409,19 @@ class OakDirectory extends Directory {
@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;
}
@@ -411,6 +429,20 @@ class OakDirectory extends Directory {
public void close() {
file.blob = null;
file.data = null;
+
+ if (clones != null) {
+ for (Iterator<OakIndexInput> 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);
+ }
}
}
Modified: jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectoryTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectoryTest.java?rev=1681918&r1=1681917&r2=1681918&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectoryTest.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectoryTest.java Wed May 27 07:09:12 2015
@@ -32,6 +32,7 @@ import org.apache.jackrabbit.oak.plugins
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;
@@ -47,6 +48,7 @@ import static org.apache.jackrabbit.oak.
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();
@@ -169,4 +171,125 @@ public class OakDirectoryTest {
rnd.nextBytes(data);
return data;
}
+
+ @Test
+ public void testCloseOnOriginalIndexInput() throws Exception {
+ Directory dir = createDir(builder, false);
+ NodeBuilder file = builder.child(INDEX_DATA_CHILD_NAME).child("test.txt");
+ int dataSize = 1024;
+ List<? super Blob> blobs = new ArrayList<Blob>(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, false);
+ NodeBuilder file = builder.child(INDEX_DATA_CHILD_NAME).child("test.txt");
+ int dataSize = 1024;
+ List<? super Blob> blobs = new ArrayList<Blob>(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
+ }
+ }
}