LUCENE-6483: Ensure core closed listeners are called on the same cache key as the reader which has been used to register the listener.

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1680049 13f79535-47bb-0310-9956-ffa450edef68

lucene/CHANGES.txt

@@ -170,6 +170,9 @@ Bug Fixes
 * LUCENE-6468: Fixed NPE with empty Kuromoji user dictionary.
   (Jun Ohtani via Christian Moen)
 
+* LUCENE-6483: Ensure core closed listeners are called on the same cache key as
+  the reader which has been used to register the listener. (Adrien Grand)
+
 API Changes
 
 * LUCENE-6377: SearcherFactory#newSearcher now accepts the previous reader

lucene/core/src/java/org/apache/lucene/index/FilterLeafReader.java

@@ -19,6 +19,7 @@ package org.apache.lucene.index;
 
 import java.io.IOException;
 import java.util.Iterator;
+import java.util.Objects;
 
 import org.apache.lucene.search.CachingWrapperQuery;
 import org.apache.lucene.util.AttributeSource;
@@ -305,14 +306,67 @@ public class FilterLeafReader extends LeafReader {
     in.registerParentReader(this);
   }
 
+  /**
+   * A CoreClosedListener wrapper that adjusts the core cache key that
+   * the wrapper is called with. This is useful if the core cache key
+   * of a reader is different from the key of the wrapped reader.
+   */
+  private static class CoreClosedListenerWrapper implements CoreClosedListener {
+
+    public static CoreClosedListener wrap(CoreClosedListener listener, Object thisCoreKey, Object inCoreKey) {
+      if (thisCoreKey == inCoreKey) {
+        // this reader has the same core cache key as its parent, nothing to do
+        return listener;
+      } else {
+        // we don't have the same cache key as the wrapped reader, we need to wrap
+        // the listener to call it with the correct cache key
+        return new CoreClosedListenerWrapper(listener, thisCoreKey, inCoreKey);
+      }
+    }
+
+    private final CoreClosedListener in;
+    private final Object thisCoreKey;
+    private final Object inCoreKey;
+
+    private CoreClosedListenerWrapper(CoreClosedListener in, Object thisCoreKey, Object inCoreKey) {
+      this.in = in;
+      this.thisCoreKey = thisCoreKey;
+      this.inCoreKey = inCoreKey;
+    }
+
+    @Override
+    public void onClose(Object ownerCoreCacheKey) throws IOException {
+      assert inCoreKey == ownerCoreCacheKey;
+      in.onClose(thisCoreKey);
+    }
+
+    // NOTE: equals/hashcore are important for removeCoreClosedListener to work
+    // correctly
+
+    @Override
+    public boolean equals(Object obj) {
+      if (obj == null || obj.getClass() != CoreClosedListenerWrapper.class) {
+        return false;
+      }
+      CoreClosedListenerWrapper that = (CoreClosedListenerWrapper) obj;
+      return in.equals(that.in) && thisCoreKey == that.thisCoreKey;
+    }
+
+    @Override
+    public int hashCode() {
+      return Objects.hash(getClass(), in, thisCoreKey);
+    }
+
+  }
+
   @Override
-  public void addCoreClosedListener(CoreClosedListener listener) {
-    in.addCoreClosedListener(listener);
+  public void addCoreClosedListener(final CoreClosedListener listener) {
+    in.addCoreClosedListener(CoreClosedListenerWrapper.wrap(listener, getCoreCacheKey(), in.getCoreCacheKey()));
   }
 
   @Override
   public void removeCoreClosedListener(CoreClosedListener listener) {
-    in.removeCoreClosedListener(listener);
+    in.removeCoreClosedListener(CoreClosedListenerWrapper.wrap(listener, getCoreCacheKey(), in.getCoreCacheKey()));
   }
 
   @Override

lucene/core/src/java/org/apache/lucene/index/LeafReader.java

@@ -79,7 +79,9 @@ public abstract class LeafReader extends IndexReader {
    */
   public static interface CoreClosedListener {
     /** Invoked when the shared core of the original {@code
-     *  SegmentReader} has closed. */
+     *  SegmentReader} has closed. The provided {@code
+     *  ownerCoreCacheKey} will be the same key as the one
+     *  returned by {@link LeafReader#getCoreCacheKey()}. */
     public void onClose(Object ownerCoreCacheKey) throws IOException;
   }
 

lucene/core/src/test/org/apache/lucene/index/TestIndexReaderClose.java

@@ -94,7 +94,7 @@ public class TestIndexReaderClose extends LuceneTestCase {
     dir.close();
   }
 
-  public void testCoreListenerOnWrapper() throws IOException {
+  public void testCoreListenerOnSlowCompositeReaderWrapper() throws IOException {
     RandomIndexWriter w = new RandomIndexWriter(random(), newDirectory());
     final int numDocs = TestUtil.nextInt(random(), 1, 5);
     for (int i = 0; i < numDocs; ++i) {
@@ -114,7 +114,57 @@ public class TestIndexReaderClose extends LuceneTestCase {
     AtomicInteger counter = new AtomicInteger(numListeners);
     
     for (int i = 0; i < numListeners; ++i) {
-      CountCoreListener listener = new CountCoreListener(counter);
+      CountCoreListener listener = new CountCoreListener(counter, leafReader.getCoreCacheKey());
+      listeners.add(listener);
+      leafReader.addCoreClosedListener(listener);
+    }
+    for (int i = 0; i < 100; ++i) {
+      leafReader.addCoreClosedListener(listeners.get(random().nextInt(listeners.size())));
+    }
+    final int removed = random().nextInt(numListeners);
+    Collections.shuffle(listeners, random());
+    for (int i = 0; i < removed; ++i) {
+      leafReader.removeCoreClosedListener(listeners.get(i));
+    }
+    assertEquals(numListeners, counter.get());
+    // make sure listeners are registered on the wrapped reader and that closing any of them has the same effect
+    if (random().nextBoolean()) {
+      reader.close();
+    } else {
+      leafReader.close();
+    }
+    assertEquals(removed, counter.get());
+    w.w.getDirectory().close();
+  }
+
+  public void testCoreListenerOnWrapperWithDifferentCacheKey() throws IOException {
+    RandomIndexWriter w = new RandomIndexWriter(random(), newDirectory());
+    final int numDocs = TestUtil.nextInt(random(), 1, 5);
+    for (int i = 0; i < numDocs; ++i) {
+      w.addDocument(new Document());
+      if (random().nextBoolean()) {
+        w.commit();
+      }
+    }
+    w.commit();
+    w.close();
+
+    final IndexReader reader = DirectoryReader.open(w.w.getDirectory());
+    // We explicitly define a different cache key
+    final Object coreCacheKey = new Object();
+    final LeafReader leafReader = new FilterLeafReader(SlowCompositeReaderWrapper.wrap(reader)) {
+      @Override
+      public Object getCoreCacheKey() {
+        return coreCacheKey;
+      }
+    };
+
+    final int numListeners = TestUtil.nextInt(random(), 1, 10);
+    final List<LeafReader.CoreClosedListener> listeners = new ArrayList<>();
+    AtomicInteger counter = new AtomicInteger(numListeners);
+    
+    for (int i = 0; i < numListeners; ++i) {
+      CountCoreListener listener = new CountCoreListener(counter, coreCacheKey);
       listeners.add(listener);
       leafReader.addCoreClosedListener(listener);
     }
@@ -140,13 +190,16 @@ public class TestIndexReaderClose extends LuceneTestCase {
   private static final class CountCoreListener implements LeafReader.CoreClosedListener {
 
     private final AtomicInteger count;
+    private final Object coreCacheKey;
 
-    public CountCoreListener(AtomicInteger count) {
+    public CountCoreListener(AtomicInteger count, Object coreCacheKey) {
       this.count = count;
+      this.coreCacheKey = coreCacheKey;
     }
 
     @Override
     public void onClose(Object coreCacheKey) {
+      assertSame(this.coreCacheKey, coreCacheKey);
       count.decrementAndGet();
     }
 

lucene/test-framework/src/java/org/apache/lucene/index/AssertingLeafReader.java

@@ -47,6 +47,15 @@ public class AssertingLeafReader extends FilterLeafReader {
     assert in.numDocs() <= in.maxDoc();
     assert in.numDeletedDocs() + in.numDocs() == in.maxDoc();
     assert !in.hasDeletions() || in.numDeletedDocs() > 0 && in.numDocs() < in.maxDoc();
+
+    addCoreClosedListener(new CoreClosedListener() {
+      @Override
+      public void onClose(Object ownerCoreCacheKey) throws IOException {
+        final Object expectedKey = getCoreCacheKey();
+        assert expectedKey == ownerCoreCacheKey
+            : "Core closed listener called on a different key " + expectedKey + " <> " + ownerCoreCacheKey;
+      }
+    });
   }
 
   @Override