Oh that's great! Thanks for the pointer Uwe, I'll fix it. Mike McCandless
http://blog.mikemccandless.com On Tue, Aug 9, 2016 at 7:22 AM, Uwe Schindler <[email protected]> wrote: > Instead of the lambda in assertThrows, use this: > assertThrows(RuntimeException.class, in::readByte) > > A method reference is much better here, as we have no parameter, so javac > can implement this more easy without stupid synthetic method appearing in > stack trace. :) > > Uwe > > Am 9. August 2016 11:03:41 MESZ, schrieb [email protected]: > >Repository: lucene-solr > >Updated Branches: > > refs/heads/master 1164c17e0 -> 04086fbfc > > > > > >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/04086fbf > >Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/04086fbf > >Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/04086fbf > > > >Branch: refs/heads/master > >Commit: 04086fbfc4e1103605267024adad121077ae52d6 > >Parents: 1164c17 > >Author: Mike McCandless <[email protected]> > >Authored: Tue Aug 9 05:03:29 2016 -0400 > >Committer: Mike McCandless <[email protected]> > >Committed: Tue Aug 9 05:03:29 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/ > 04086fbf/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/ > 04086fbf/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/ > 04086fbf/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/ > 04086fbf/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/ > 04086fbf/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(); > >+ } > > } > > -- > Uwe Schindler > H.-H.-Meier-Allee 63, 28213 Bremen > http://www.thetaphi.de > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > >
