LUCENE-7409: improve MockDirectoryWrapper's IndexInput to detect if a clone is being used after its parent was closed

This commit is contained in:
Mike McCandless 2016-08-09 05:03:29 -04:00
parent 1164c17e0e
commit 04086fbfc4
5 changed files with 57 additions and 12 deletions

View File

@ -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;

View File

@ -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;
}

View File

@ -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

View File

@ -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) {

View File

@ -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();
}
}