You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by jp...@apache.org on 2017/04/13 06:54:06 UTC

[2/2] lucene-solr:master: LUCENE-7781: Call ensureOpen when registering closed listeners.

LUCENE-7781: Call ensureOpen when registering closed listeners.


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/d3494c2c
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/d3494c2c
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/d3494c2c

Branch: refs/heads/master
Commit: d3494c2cd6a7c65b960e24dfff2459efeba29f81
Parents: 4cb00cc
Author: Adrien Grand <jp...@gmail.com>
Authored: Thu Apr 13 08:20:56 2017 +0200
Committer: Adrien Grand <jp...@gmail.com>
Committed: Thu Apr 13 08:41:47 2017 +0200

----------------------------------------------------------------------
 .../org/apache/lucene/index/SegmentReader.java  | 25 ++++++++++--
 .../lucene/index/StandardDirectoryReader.java   |  3 +-
 .../lucene/index/TestIndexReaderClose.java      | 40 ++++++++++++--------
 .../lucene/index/OwnCacheKeyMultiReader.java    |  3 +-
 4 files changed, 49 insertions(+), 22 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/d3494c2c/lucene/core/src/java/org/apache/lucene/index/SegmentReader.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/java/org/apache/lucene/index/SegmentReader.java b/lucene/core/src/java/org/apache/lucene/index/SegmentReader.java
index ccbcdf9..930340c 100644
--- a/lucene/core/src/java/org/apache/lucene/index/SegmentReader.java
+++ b/lucene/core/src/java/org/apache/lucene/index/SegmentReader.java
@@ -294,7 +294,7 @@ public final class SegmentReader extends CodecReader {
     synchronized(readerClosedListeners) {
       for(ClosedListener listener : readerClosedListeners) {
         try {
-          listener.onClose(cacheHelper.getKey());
+          listener.onClose(readerCacheHelper.getKey());
         } catch (Throwable t) {
           if (th == null) {
             th = t;
@@ -307,7 +307,7 @@ public final class SegmentReader extends CodecReader {
     }
   }
 
-  private final IndexReader.CacheHelper cacheHelper = new IndexReader.CacheHelper() {
+  private final IndexReader.CacheHelper readerCacheHelper = new IndexReader.CacheHelper() {
     private final IndexReader.CacheKey cacheKey = new IndexReader.CacheKey();
 
     @Override
@@ -317,18 +317,35 @@ public final class SegmentReader extends CodecReader {
 
     @Override
     public void addClosedListener(ClosedListener listener) {
+      ensureOpen();
       readerClosedListeners.add(listener);
     }
   };
 
   @Override
   public CacheHelper getReaderCacheHelper() {
-    return cacheHelper;
+    return readerCacheHelper;
   }
 
+  /** Wrap the cache helper of the core to add ensureOpen() calls that make
+   *  sure users do not register closed listeners on closed indices. */
+  private final IndexReader.CacheHelper coreCacheHelper = new IndexReader.CacheHelper() {
+
+    @Override
+    public CacheKey getKey() {
+      return core.getCacheHelper().getKey();
+    }
+
+    @Override
+    public void addClosedListener(ClosedListener listener) {
+      ensureOpen();
+      core.getCacheHelper().addClosedListener(listener);
+    }
+  };
+
   @Override
   public CacheHelper getCoreCacheHelper() {
-    return core.getCacheHelper();
+    return coreCacheHelper;
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/d3494c2c/lucene/core/src/java/org/apache/lucene/index/StandardDirectoryReader.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/java/org/apache/lucene/index/StandardDirectoryReader.java b/lucene/core/src/java/org/apache/lucene/index/StandardDirectoryReader.java
index f0e7e98..bedf17e 100644
--- a/lucene/core/src/java/org/apache/lucene/index/StandardDirectoryReader.java
+++ b/lucene/core/src/java/org/apache/lucene/index/StandardDirectoryReader.java
@@ -484,7 +484,8 @@ public final class StandardDirectoryReader extends DirectoryReader {
 
     @Override
     public void addClosedListener(ClosedListener listener) {
-        readerClosedListeners.add(listener);
+      ensureOpen();
+      readerClosedListeners.add(listener);
     }
 
   };

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/d3494c2c/lucene/core/src/test/org/apache/lucene/index/TestIndexReaderClose.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/test/org/apache/lucene/index/TestIndexReaderClose.java b/lucene/core/src/test/org/apache/lucene/index/TestIndexReaderClose.java
index 20088a5..b99666e 100644
--- a/lucene/core/src/test/org/apache/lucene/index/TestIndexReaderClose.java
+++ b/lucene/core/src/test/org/apache/lucene/index/TestIndexReaderClose.java
@@ -73,7 +73,7 @@ public class TestIndexReaderClose extends LuceneTestCase {
             reader.getReaderCacheHelper().addClosedListener(new FaultyListener());
           } else {
             count.incrementAndGet();
-            reader.getReaderCacheHelper().addClosedListener(new CountListener(count));
+            reader.getReaderCacheHelper().addClosedListener(new CountListener(count, reader.getReaderCacheHelper().getKey()));
           }
       }
       if (!faultySet && !throwOnClose) {
@@ -123,7 +123,7 @@ public class TestIndexReaderClose extends LuceneTestCase {
     AtomicInteger counter = new AtomicInteger(numListeners);
 
     for (int i = 0; i < numListeners; ++i) {
-      CountCoreListener listener = new CountCoreListener(counter, leafReader.getCoreCacheHelper().getKey());
+      CountListener listener = new CountListener(counter, leafReader.getCoreCacheHelper().getKey());
       listeners.add(listener);
       leafReader.getCoreCacheHelper().addClosedListener(listener);
     }
@@ -141,12 +141,12 @@ public class TestIndexReaderClose extends LuceneTestCase {
     w.w.getDirectory().close();
   }
 
-  private static final class CountCoreListener implements IndexReader.ClosedListener {
+  private static final class CountListener implements IndexReader.ClosedListener {
 
     private final AtomicInteger count;
     private final Object coreCacheKey;
 
-    public CountCoreListener(AtomicInteger count, Object coreCacheKey) {
+    public CountListener(AtomicInteger count, Object coreCacheKey) {
       this.count = count;
       this.coreCacheKey = coreCacheKey;
     }
@@ -159,25 +159,33 @@ public class TestIndexReaderClose extends LuceneTestCase {
 
   }
 
-  private static final class CountListener implements IndexReader.ClosedListener  {
-    private final AtomicInteger count;
-
-    public CountListener(AtomicInteger count) {
-      this.count = count;
-    }
+  private static final class FaultyListener implements IndexReader.ClosedListener {
 
     @Override
     public void onClose(IndexReader.CacheKey cacheKey) {
-      count.decrementAndGet();
+      throw new IllegalStateException("GRRRRRRRRRRRR!");
     }
   }
 
-  private static final class FaultyListener implements IndexReader.ClosedListener {
+  public void testRegisterListenerOnClosedReader() throws IOException {
+    Directory dir = newDirectory();
+    IndexWriter w = new IndexWriter(dir, newIndexWriterConfig());
+    w.addDocument(new Document());
+    DirectoryReader r = DirectoryReader.open(w);
+    w.close();
 
-    @Override
-    public void onClose(IndexReader.CacheKey cacheKey) {
-      throw new IllegalStateException("GRRRRRRRRRRRR!");
-    }
+    // The reader is open, everything should work
+    r.getReaderCacheHelper().addClosedListener(key -> {});
+    r.leaves().get(0).reader().getReaderCacheHelper().addClosedListener(key -> {});
+    r.leaves().get(0).reader().getCoreCacheHelper().addClosedListener(key -> {});
+
+    // But now we close
+    r.close();
+    expectThrows(AlreadyClosedException.class, () -> r.getReaderCacheHelper().addClosedListener(key -> {}));
+    expectThrows(AlreadyClosedException.class, () -> r.leaves().get(0).reader().getReaderCacheHelper().addClosedListener(key -> {}));
+    expectThrows(AlreadyClosedException.class, () -> r.leaves().get(0).reader().getCoreCacheHelper().addClosedListener(key -> {}));
+
+    dir.close();
   }
 
 }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/d3494c2c/lucene/test-framework/src/java/org/apache/lucene/index/OwnCacheKeyMultiReader.java
----------------------------------------------------------------------
diff --git a/lucene/test-framework/src/java/org/apache/lucene/index/OwnCacheKeyMultiReader.java b/lucene/test-framework/src/java/org/apache/lucene/index/OwnCacheKeyMultiReader.java
index 45aabfe..a412ed8 100644
--- a/lucene/test-framework/src/java/org/apache/lucene/index/OwnCacheKeyMultiReader.java
+++ b/lucene/test-framework/src/java/org/apache/lucene/index/OwnCacheKeyMultiReader.java
@@ -40,7 +40,8 @@ public final class OwnCacheKeyMultiReader extends MultiReader {
 
     @Override
     public void addClosedListener(ClosedListener listener) {
-        readerClosedListeners.add(listener);
+      ensureOpen();
+      readerClosedListeners.add(listener);
     }
 
   };