Author: tommaso
Date: Wed May 27 07:27:28 2015
New Revision: 1681920
URL: http://svn.apache.org/r1681920
Log:
OAK-2799 - close cloned OakIndexInput instances on close of main one
Modified:
jackrabbit/oak/branches/1.2/ (props changed)
jackrabbit/oak/branches/1.2/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectory.java
jackrabbit/oak/branches/1.2/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectoryTest.java
Propchange: jackrabbit/oak/branches/1.2/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed May 27 07:27:28 2015
@@ -1,3 +1,3 @@
/jackrabbit/oak/branches/1.0:1665962
-/jackrabbit/oak/trunk:1672350,1672468,1672537,1672603,1672642,1672644,1672834-1672835,1673351,1673410,1673414,1673436,1673644,1673662-1673664,1673669,1673695,1674046,1674065,1674075,1674107,1674228,1674880,1675054-1675055,1675332,1675354,1675357,1675382,1675555,1675566,1675593,1676198,1676237,1676407,1676458,1676539,1676670,1676693,1676703,1676725,1677579,1677581,1677609,1677611,1677774,1677939,1677991,1678173,1678323,1678758,1678938,1678954,1679144,1679165,1679191,1679235,1680182,1680222,1680232,1680236,1680461,1680805-1680806,1680903,1681282,1681767
+/jackrabbit/oak/trunk:1672350,1672468,1672537,1672603,1672642,1672644,1672834-1672835,1673351,1673410,1673414,1673436,1673644,1673662-1673664,1673669,1673695,1674046,1674065,1674075,1674107,1674228,1674880,1675054-1675055,1675332,1675354,1675357,1675382,1675555,1675566,1675593,1676198,1676237,1676407,1676458,1676539,1676670,1676693,1676703,1676725,1677579,1677581,1677609,1677611,1677774,1677939,1677991,1678173,1678323,1678758,1678938,1678954,1679144,1679165,1679191,1679235,1680182,1680222,1680232,1680236,1680461,1680805-1680806,1680903,1681282,1681767,1681918
/jackrabbit/trunk:1345480
Modified:
jackrabbit/oak/branches/1.2/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectory.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectory.java?rev=1681920&r1=1681919&r2=1681920&view=diff
==============================================================================
---
jackrabbit/oak/branches/1.2/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectory.java
(original)
+++
jackrabbit/oak/branches/1.2/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectory.java
Wed May 27 07:27:28 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/branches/1.2/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectoryTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectoryTest.java?rev=1681920&r1=1681919&r2=1681920&view=diff
==============================================================================
---
jackrabbit/oak/branches/1.2/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectoryTest.java
(original)
+++
jackrabbit/oak/branches/1.2/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectoryTest.java
Wed May 27 07:27:28 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
+ }
+ }
}
