You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by mi...@apache.org on 2016/08/09 09:06:51 UTC
lucene-solr:branch_6x: LUCENE-7409: improve MockDirectoryWrapper's
IndexInput to detect if a clone is being used after its parent was closed
Repository: lucene-solr
Updated Branches:
refs/heads/branch_6x e08d656a1 -> 1fa91537a
LUCENE-7409: improve MockDirectoryWrapper's IndexInput to detect if a clone is being used after its parent was closed
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/1fa91537
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/1fa91537
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/1fa91537
Branch: refs/heads/branch_6x
Commit: 1fa91537a9485ea08a0ff834d4306b414150d500
Parents: e08d656
Author: Mike McCandless <mi...@apache.org>
Authored: Tue Aug 9 05:03:29 2016 -0400
Committer: Mike McCandless <mi...@apache.org>
Committed: Tue Aug 9 05:04:09 2016 -0400
----------------------------------------------------------------------
.../lucene/store/MockDirectoryWrapper.java | 2 +-
.../lucene/store/MockIndexInputWrapper.java | 27 ++++++++++-----
.../store/SlowClosingMockIndexInputWrapper.java | 2 +-
.../store/SlowOpeningMockIndexInputWrapper.java | 2 +-
.../lucene/store/TestMockDirectoryWrapper.java | 36 ++++++++++++++++++++
5 files changed, 57 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/1fa91537/lucene/test-framework/src/java/org/apache/lucene/store/MockDirectoryWrapper.java
----------------------------------------------------------------------
diff --git a/lucene/test-framework/src/java/org/apache/lucene/store/MockDirectoryWrapper.java b/lucene/test-framework/src/java/org/apache/lucene/store/MockDirectoryWrapper.java
index e78968d..1ff9470 100644
--- a/lucene/test-framework/src/java/org/apache/lucene/store/MockDirectoryWrapper.java
+++ b/lucene/test-framework/src/java/org/apache/lucene/store/MockDirectoryWrapper.java
@@ -771,7 +771,7 @@ public class MockDirectoryWrapper extends BaseDirectoryWrapper {
}
ii = new SlowOpeningMockIndexInputWrapper(this, name, delegateInput);
} else {
- ii = new MockIndexInputWrapper(this, name, delegateInput);
+ ii = new MockIndexInputWrapper(this, name, delegateInput, null);
}
addFileHandle(ii, name, Handle.Input);
return ii;
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/1fa91537/lucene/test-framework/src/java/org/apache/lucene/store/MockIndexInputWrapper.java
----------------------------------------------------------------------
diff --git a/lucene/test-framework/src/java/org/apache/lucene/store/MockIndexInputWrapper.java b/lucene/test-framework/src/java/org/apache/lucene/store/MockIndexInputWrapper.java
index f62d67b..f68e18c 100644
--- a/lucene/test-framework/src/java/org/apache/lucene/store/MockIndexInputWrapper.java
+++ b/lucene/test-framework/src/java/org/apache/lucene/store/MockIndexInputWrapper.java
@@ -30,12 +30,19 @@ public class MockIndexInputWrapper extends IndexInput {
private MockDirectoryWrapper dir;
final String name;
private IndexInput delegate;
- private boolean isClone;
- private boolean closed;
+ private volatile boolean closed;
- /** Construct an empty output buffer. */
- public MockIndexInputWrapper(MockDirectoryWrapper dir, String name, IndexInput delegate) {
+ // Which MockIndexInputWrapper we were cloned from, or null if we are not a clone:
+ private final MockIndexInputWrapper parent;
+
+ /** Sole constructor */
+ public MockIndexInputWrapper(MockDirectoryWrapper dir, String name, IndexInput delegate, MockIndexInputWrapper parent) {
super("MockIndexInputWrapper(name=" + name + " delegate=" + delegate + ")");
+
+ // If we are a clone then our parent better not be a clone!
+ assert parent == null || parent.parent == null;
+
+ this.parent = parent;
this.name = name;
this.dir = dir;
this.delegate = delegate;
@@ -54,7 +61,7 @@ public class MockIndexInputWrapper extends IndexInput {
// remove the conditional check so we also track that
// all clones get closed:
assert delegate != null;
- if (!isClone) {
+ if (parent == null) {
dir.removeIndexInput(this, name);
}
dir.maybeThrowDeterministicException();
@@ -62,9 +69,13 @@ public class MockIndexInputWrapper extends IndexInput {
}
private void ensureOpen() {
+ // TODO: not great this is a volatile read (closed) ... we should deploy heavy JVM voodoo like SwitchPoint to avoid this
if (closed) {
throw new RuntimeException("Abusing closed IndexInput!");
}
+ if (parent != null && parent.closed) {
+ throw new RuntimeException("Abusing clone of a closed IndexInput!");
+ }
}
@Override
@@ -75,8 +86,7 @@ public class MockIndexInputWrapper extends IndexInput {
}
dir.inputCloneCount.incrementAndGet();
IndexInput iiclone = delegate.clone();
- MockIndexInputWrapper clone = new MockIndexInputWrapper(dir, name, iiclone);
- clone.isClone = true;
+ MockIndexInputWrapper clone = new MockIndexInputWrapper(dir, name, iiclone, parent != null ? parent : this);
// Pending resolution on LUCENE-686 we may want to
// uncomment this code so that we also track that all
// clones get closed:
@@ -102,8 +112,7 @@ public class MockIndexInputWrapper extends IndexInput {
}
dir.inputCloneCount.incrementAndGet();
IndexInput slice = delegate.slice(sliceDescription, offset, length);
- MockIndexInputWrapper clone = new MockIndexInputWrapper(dir, sliceDescription, slice);
- clone.isClone = true;
+ MockIndexInputWrapper clone = new MockIndexInputWrapper(dir, sliceDescription, slice, parent != null ? parent : this);
return clone;
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/1fa91537/lucene/test-framework/src/java/org/apache/lucene/store/SlowClosingMockIndexInputWrapper.java
----------------------------------------------------------------------
diff --git a/lucene/test-framework/src/java/org/apache/lucene/store/SlowClosingMockIndexInputWrapper.java b/lucene/test-framework/src/java/org/apache/lucene/store/SlowClosingMockIndexInputWrapper.java
index 2be2e27..e6c3857 100644
--- a/lucene/test-framework/src/java/org/apache/lucene/store/SlowClosingMockIndexInputWrapper.java
+++ b/lucene/test-framework/src/java/org/apache/lucene/store/SlowClosingMockIndexInputWrapper.java
@@ -30,7 +30,7 @@ class SlowClosingMockIndexInputWrapper extends MockIndexInputWrapper {
public SlowClosingMockIndexInputWrapper(MockDirectoryWrapper dir,
String name, IndexInput delegate) {
- super(dir, name, delegate);
+ super(dir, name, delegate, null);
}
@Override
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/1fa91537/lucene/test-framework/src/java/org/apache/lucene/store/SlowOpeningMockIndexInputWrapper.java
----------------------------------------------------------------------
diff --git a/lucene/test-framework/src/java/org/apache/lucene/store/SlowOpeningMockIndexInputWrapper.java b/lucene/test-framework/src/java/org/apache/lucene/store/SlowOpeningMockIndexInputWrapper.java
index 4cc2b19..1e95451e 100644
--- a/lucene/test-framework/src/java/org/apache/lucene/store/SlowOpeningMockIndexInputWrapper.java
+++ b/lucene/test-framework/src/java/org/apache/lucene/store/SlowOpeningMockIndexInputWrapper.java
@@ -28,7 +28,7 @@ class SlowOpeningMockIndexInputWrapper extends MockIndexInputWrapper {
public SlowOpeningMockIndexInputWrapper(MockDirectoryWrapper dir,
String name, IndexInput delegate) throws IOException {
- super(dir, name, delegate);
+ super(dir, name, delegate, null);
try {
Thread.sleep(50);
} catch (InterruptedException ie) {
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/1fa91537/lucene/test-framework/src/test/org/apache/lucene/store/TestMockDirectoryWrapper.java
----------------------------------------------------------------------
diff --git a/lucene/test-framework/src/test/org/apache/lucene/store/TestMockDirectoryWrapper.java b/lucene/test-framework/src/test/org/apache/lucene/store/TestMockDirectoryWrapper.java
index ae453e5..145b40b 100644
--- a/lucene/test-framework/src/test/org/apache/lucene/store/TestMockDirectoryWrapper.java
+++ b/lucene/test-framework/src/test/org/apache/lucene/store/TestMockDirectoryWrapper.java
@@ -171,4 +171,40 @@ public class TestMockDirectoryWrapper extends BaseDirectoryTestCase {
assertTrue("MockDirectoryWrapper on dir=" + dir + " failed to corrupt an unsync'd file", changed);
}
+
+ public void testAbuseClosedIndexInput() throws Exception {
+ MockDirectoryWrapper dir = newMockDirectory();
+ IndexOutput out = dir.createOutput("foo", IOContext.DEFAULT);
+ out.writeByte((byte) 42);
+ out.close();
+ final IndexInput in = dir.openInput("foo", IOContext.DEFAULT);
+ in.close();
+ expectThrows(RuntimeException.class, () -> {in.readByte();});
+ dir.close();
+ }
+
+ public void testAbuseCloneAfterParentClosed() throws Exception {
+ MockDirectoryWrapper dir = newMockDirectory();
+ IndexOutput out = dir.createOutput("foo", IOContext.DEFAULT);
+ out.writeByte((byte) 42);
+ out.close();
+ IndexInput in = dir.openInput("foo", IOContext.DEFAULT);
+ final IndexInput clone = in.clone();
+ in.close();
+ expectThrows(RuntimeException.class, () -> {clone.readByte();});
+ dir.close();
+ }
+
+ public void testAbuseCloneOfCloneAfterParentClosed() throws Exception {
+ MockDirectoryWrapper dir = newMockDirectory();
+ IndexOutput out = dir.createOutput("foo", IOContext.DEFAULT);
+ out.writeByte((byte) 42);
+ out.close();
+ IndexInput in = dir.openInput("foo", IOContext.DEFAULT);
+ IndexInput clone1 = in.clone();
+ IndexInput clone2 = clone1.clone();
+ in.close();
+ expectThrows(RuntimeException.class, () -> {clone2.readByte();});
+ dir.close();
+ }
}