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 e08d656a15
commit 1fa91537a9
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); ii = new SlowOpeningMockIndexInputWrapper(this, name, delegateInput);
} else { } else {
ii = new MockIndexInputWrapper(this, name, delegateInput); ii = new MockIndexInputWrapper(this, name, delegateInput, null);
} }
addFileHandle(ii, name, Handle.Input); addFileHandle(ii, name, Handle.Input);
return ii; return ii;

View File

@ -30,12 +30,19 @@ public class MockIndexInputWrapper extends IndexInput {
private MockDirectoryWrapper dir; private MockDirectoryWrapper dir;
final String name; final String name;
private IndexInput delegate; private IndexInput delegate;
private boolean isClone; private volatile boolean closed;
private boolean closed;
/** Construct an empty output buffer. */ // Which MockIndexInputWrapper we were cloned from, or null if we are not a clone:
public MockIndexInputWrapper(MockDirectoryWrapper dir, String name, IndexInput delegate) { private final MockIndexInputWrapper parent;
/** Sole constructor */
public MockIndexInputWrapper(MockDirectoryWrapper dir, String name, IndexInput delegate, MockIndexInputWrapper parent) {
super("MockIndexInputWrapper(name=" + name + " delegate=" + delegate + ")"); 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.name = name;
this.dir = dir; this.dir = dir;
this.delegate = delegate; this.delegate = delegate;
@ -54,7 +61,7 @@ public class MockIndexInputWrapper extends IndexInput {
// remove the conditional check so we also track that // remove the conditional check so we also track that
// all clones get closed: // all clones get closed:
assert delegate != null; assert delegate != null;
if (!isClone) { if (parent == null) {
dir.removeIndexInput(this, name); dir.removeIndexInput(this, name);
} }
dir.maybeThrowDeterministicException(); dir.maybeThrowDeterministicException();
@ -62,9 +69,13 @@ public class MockIndexInputWrapper extends IndexInput {
} }
private void ensureOpen() { 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) { if (closed) {
throw new RuntimeException("Abusing closed IndexInput!"); throw new RuntimeException("Abusing closed IndexInput!");
} }
if (parent != null && parent.closed) {
throw new RuntimeException("Abusing clone of a closed IndexInput!");
}
} }
@Override @Override
@ -75,8 +86,7 @@ public class MockIndexInputWrapper extends IndexInput {
} }
dir.inputCloneCount.incrementAndGet(); dir.inputCloneCount.incrementAndGet();
IndexInput iiclone = delegate.clone(); IndexInput iiclone = delegate.clone();
MockIndexInputWrapper clone = new MockIndexInputWrapper(dir, name, iiclone); MockIndexInputWrapper clone = new MockIndexInputWrapper(dir, name, iiclone, parent != null ? parent : this);
clone.isClone = true;
// Pending resolution on LUCENE-686 we may want to // Pending resolution on LUCENE-686 we may want to
// uncomment this code so that we also track that all // uncomment this code so that we also track that all
// clones get closed: // clones get closed:
@ -102,8 +112,7 @@ public class MockIndexInputWrapper extends IndexInput {
} }
dir.inputCloneCount.incrementAndGet(); dir.inputCloneCount.incrementAndGet();
IndexInput slice = delegate.slice(sliceDescription, offset, length); IndexInput slice = delegate.slice(sliceDescription, offset, length);
MockIndexInputWrapper clone = new MockIndexInputWrapper(dir, sliceDescription, slice); MockIndexInputWrapper clone = new MockIndexInputWrapper(dir, sliceDescription, slice, parent != null ? parent : this);
clone.isClone = true;
return clone; return clone;
} }

View File

@ -30,7 +30,7 @@ class SlowClosingMockIndexInputWrapper extends MockIndexInputWrapper {
public SlowClosingMockIndexInputWrapper(MockDirectoryWrapper dir, public SlowClosingMockIndexInputWrapper(MockDirectoryWrapper dir,
String name, IndexInput delegate) { String name, IndexInput delegate) {
super(dir, name, delegate); super(dir, name, delegate, null);
} }
@Override @Override

View File

@ -28,7 +28,7 @@ class SlowOpeningMockIndexInputWrapper extends MockIndexInputWrapper {
public SlowOpeningMockIndexInputWrapper(MockDirectoryWrapper dir, public SlowOpeningMockIndexInputWrapper(MockDirectoryWrapper dir,
String name, IndexInput delegate) throws IOException { String name, IndexInput delegate) throws IOException {
super(dir, name, delegate); super(dir, name, delegate, null);
try { try {
Thread.sleep(50); Thread.sleep(50);
} catch (InterruptedException ie) { } 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); 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();
}
} }