You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by rm...@apache.org on 2011/12/13 22:19:30 UTC

svn commit: r1213932 - in /lucene/dev/branches/branch_3x: ./ lucene/ lucene/backwards/src/test/org/apache/lucene/index/ lucene/contrib/instantiated/src/java/org/apache/lucene/store/instantiated/ lucene/contrib/memory/src/java/org/apache/lucene/index/me...

Author: rmuir
Date: Tue Dec 13 21:19:30 2011
New Revision: 1213932

URL: http://svn.apache.org/viewvc?rev=1213932&view=rev
Log:
LUCENE-3644: fix problems with IR's readerFinishedListener

Modified:
    lucene/dev/branches/branch_3x/   (props changed)
    lucene/dev/branches/branch_3x/lucene/   (props changed)
    lucene/dev/branches/branch_3x/lucene/CHANGES.txt
    lucene/dev/branches/branch_3x/lucene/backwards/src/test/org/apache/lucene/index/TestIndexReader.java
    lucene/dev/branches/branch_3x/lucene/contrib/instantiated/src/java/org/apache/lucene/store/instantiated/InstantiatedIndexReader.java
    lucene/dev/branches/branch_3x/lucene/contrib/memory/src/java/org/apache/lucene/index/memory/MemoryIndex.java
    lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/DirectoryReader.java
    lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/FilterIndexReader.java
    lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/IndexReader.java
    lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/IndexWriter.java
    lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/MultiReader.java
    lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/ParallelReader.java
    lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/ReadOnlyDirectoryReader.java
    lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/SegmentCoreReaders.java
    lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/SegmentReader.java
    lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/search/FieldCacheImpl.java
    lucene/dev/branches/branch_3x/lucene/src/test/org/apache/lucene/index/TestIndexReader.java

Modified: lucene/dev/branches/branch_3x/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_3x/lucene/CHANGES.txt?rev=1213932&r1=1213931&r2=1213932&view=diff
==============================================================================
--- lucene/dev/branches/branch_3x/lucene/CHANGES.txt (original)
+++ lucene/dev/branches/branch_3x/lucene/CHANGES.txt Tue Dec 13 21:19:30 2011
@@ -24,6 +24,14 @@ Changes in backwards compatibility polic
   even return your own subclass of IndexSearcher. The SearcherWarmer and
   ExecutorService parameters on these classes were removed, as they are
   subsumed by SearcherFactory.  (Shai Erera, Mike McCandless, Robert Muir)
+
+* LUCENE-3644: The expert ReaderFinishedListener api suffered problems (propagated
+  down to subreaders, but was not called on SegmentReaders, unless they were
+  the owner of the reader core, and other ambiguities). The API is revised:
+  You can set ReaderClosedListeners on any IndexReader, and onClose is called
+  when that reader is closed.  SegmentReader has CoreClosedListeners that you
+  can register to know when a shared reader core is closed.  
+  (Uwe Schindler, Mike McCandless, Robert Muir)
   
 Security fixes
 

Modified: lucene/dev/branches/branch_3x/lucene/backwards/src/test/org/apache/lucene/index/TestIndexReader.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_3x/lucene/backwards/src/test/org/apache/lucene/index/TestIndexReader.java?rev=1213932&r1=1213931&r2=1213932&view=diff
==============================================================================
--- lucene/dev/branches/branch_3x/lucene/backwards/src/test/org/apache/lucene/index/TestIndexReader.java (original)
+++ lucene/dev/branches/branch_3x/lucene/backwards/src/test/org/apache/lucene/index/TestIndexReader.java Tue Dec 13 21:19:30 2011
@@ -1298,44 +1298,6 @@ public class TestIndexReader extends Luc
     dir.close();
   }
 
-  // LUCENE-2474
-  public void testReaderFinishedListener() throws Exception {
-    Directory dir = newDirectory();
-    IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer(random)).setMergePolicy(newLogMergePolicy()));
-    ((LogMergePolicy) writer.getConfig().getMergePolicy()).setMergeFactor(3);
-    writer.setInfoStream(VERBOSE ? System.out : null);
-    writer.addDocument(new Document());
-    writer.commit();
-    writer.addDocument(new Document());
-    writer.commit();
-    final IndexReader reader = writer.getReader();
-    final int[] closeCount = new int[1];
-    final IndexReader.ReaderFinishedListener listener = new IndexReader.ReaderFinishedListener() {
-      public void finished(IndexReader reader) {
-        closeCount[0]++;
-      }
-    };
-
-    reader.addReaderFinishedListener(listener);
-
-    reader.close();
-
-    // Just the top reader
-    assertEquals(1, closeCount[0]);
-    writer.close();
-
-    // Now also the subs
-    assertEquals(3, closeCount[0]);
-
-    IndexReader reader2 = IndexReader.open(dir);
-    reader2.addReaderFinishedListener(listener);
-
-    closeCount[0] = 0;
-    reader2.close();
-    assertEquals(3, closeCount[0]);
-    dir.close();
-  }
-
   public void testOOBDocID() throws Exception {
     Directory dir = newDirectory();
     IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer(random)));

Modified: lucene/dev/branches/branch_3x/lucene/contrib/instantiated/src/java/org/apache/lucene/store/instantiated/InstantiatedIndexReader.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_3x/lucene/contrib/instantiated/src/java/org/apache/lucene/store/instantiated/InstantiatedIndexReader.java?rev=1213932&r1=1213931&r2=1213932&view=diff
==============================================================================
--- lucene/dev/branches/branch_3x/lucene/contrib/instantiated/src/java/org/apache/lucene/store/instantiated/InstantiatedIndexReader.java (original)
+++ lucene/dev/branches/branch_3x/lucene/contrib/instantiated/src/java/org/apache/lucene/store/instantiated/InstantiatedIndexReader.java Tue Dec 13 21:19:30 2011
@@ -47,7 +47,6 @@ public class InstantiatedIndexReader ext
   public InstantiatedIndexReader(InstantiatedIndex index) {
     super();
     this.index = index;
-    readerFinishedListeners = Collections.synchronizedSet(new HashSet<ReaderFinishedListener>());
   }
 
   @Deprecated

Modified: lucene/dev/branches/branch_3x/lucene/contrib/memory/src/java/org/apache/lucene/index/memory/MemoryIndex.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_3x/lucene/contrib/memory/src/java/org/apache/lucene/index/memory/MemoryIndex.java?rev=1213932&r1=1213931&r2=1213932&view=diff
==============================================================================
--- lucene/dev/branches/branch_3x/lucene/contrib/memory/src/java/org/apache/lucene/index/memory/MemoryIndex.java (original)
+++ lucene/dev/branches/branch_3x/lucene/contrib/memory/src/java/org/apache/lucene/index/memory/MemoryIndex.java Tue Dec 13 21:19:30 2011
@@ -25,7 +25,6 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Map;
 
@@ -740,7 +739,6 @@ public class MemoryIndex implements Seri
     
     private MemoryIndexReader() {
       super(); // avoid as much superclass baggage as possible
-      readerFinishedListeners = Collections.synchronizedSet(new HashSet<ReaderFinishedListener>());
     }
     
     private Info getInfo(String fieldName) {

Modified: lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/DirectoryReader.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/DirectoryReader.java?rev=1213932&r1=1213931&r2=1213932&view=diff
==============================================================================
--- lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/DirectoryReader.java (original)
+++ lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/DirectoryReader.java Tue Dec 13 21:19:30 2011
@@ -29,7 +29,6 @@ import java.util.List;
 
 import java.util.Map;
 import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
 
 import org.apache.lucene.document.Document;
 import org.apache.lucene.document.FieldSelector;
@@ -38,7 +37,6 @@ import org.apache.lucene.store.Directory
 import org.apache.lucene.store.Lock;
 import org.apache.lucene.store.LockObtainFailedException;
 import org.apache.lucene.util.IOUtils;
-import org.apache.lucene.util.MapBackedSet;
 
 /** 
  * An IndexReader which reads indexes with multiple segments.
@@ -79,27 +77,21 @@ class DirectoryReader extends IndexReade
         SegmentInfos infos = new SegmentInfos();
         infos.read(directory, segmentFileName);
         if (readOnly)
-          return new ReadOnlyDirectoryReader(directory, infos, deletionPolicy, termInfosIndexDivisor, null);
+          return new ReadOnlyDirectoryReader(directory, infos, deletionPolicy, termInfosIndexDivisor);
         else
-          return new DirectoryReader(directory, infos, deletionPolicy, false, termInfosIndexDivisor, null);
+          return new DirectoryReader(directory, infos, deletionPolicy, false, termInfosIndexDivisor);
       }
     }.run(commit);
   }
 
   /** Construct reading the named set of readers. */
-  DirectoryReader(Directory directory, SegmentInfos sis, IndexDeletionPolicy deletionPolicy, boolean readOnly, int termInfosIndexDivisor,
-                  Collection<ReaderFinishedListener> readerFinishedListeners) throws IOException {
+  DirectoryReader(Directory directory, SegmentInfos sis, IndexDeletionPolicy deletionPolicy, boolean readOnly, int termInfosIndexDivisor) throws IOException {
     this.directory = directory;
     this.readOnly = readOnly;
     this.segmentInfos = sis;
     this.deletionPolicy = deletionPolicy;
     this.termInfosIndexDivisor = termInfosIndexDivisor;
 
-    if (readerFinishedListeners == null) {
-      this.readerFinishedListeners = new MapBackedSet<ReaderFinishedListener>(new ConcurrentHashMap<ReaderFinishedListener,Boolean>());
-    } else {
-      this.readerFinishedListeners = readerFinishedListeners;
-    }
     applyAllDeletes = false;
 
     // To reduce the chance of hitting FileNotFound
@@ -113,7 +105,6 @@ class DirectoryReader extends IndexReade
       boolean success = false;
       try {
         readers[i] = SegmentReader.get(readOnly, sis.info(i), termInfosIndexDivisor);
-        readers[i].readerFinishedListeners = this.readerFinishedListeners;
         success = true;
       } catch(IOException ex) {
         prior = ex;
@@ -133,7 +124,6 @@ class DirectoryReader extends IndexReade
     this.applyAllDeletes = applyAllDeletes;       // saved for reopen
 
     this.termInfosIndexDivisor = termInfosIndexDivisor;
-    readerFinishedListeners = writer.getReaderFinishedListeners();
 
     // IndexWriter synchronizes externally before calling
     // us, which ensures infos will not change; so there's
@@ -153,7 +143,6 @@ class DirectoryReader extends IndexReade
         assert info.dir == dir;
         final SegmentReader reader = writer.readerPool.getReadOnlyClone(info, true, termInfosIndexDivisor);
         if (reader.numDocs() > 0 || writer.getKeepFullyDeletedSegments()) {
-          reader.readerFinishedListeners = readerFinishedListeners;
           readers.add(reader);
           infosUpto++;
         } else {
@@ -176,14 +165,11 @@ class DirectoryReader extends IndexReade
 
   /** This constructor is only used for {@link #doOpenIfChanged()} */
   DirectoryReader(Directory directory, SegmentInfos infos, SegmentReader[] oldReaders, int[] oldStarts,
-                  Map<String,byte[]> oldNormsCache, boolean readOnly, boolean doClone, int termInfosIndexDivisor,
-                  Collection<ReaderFinishedListener> readerFinishedListeners) throws IOException {
+                  Map<String,byte[]> oldNormsCache, boolean readOnly, boolean doClone, int termInfosIndexDivisor) throws IOException {
     this.directory = directory;
     this.readOnly = readOnly;
     this.segmentInfos = infos;
     this.termInfosIndexDivisor = termInfosIndexDivisor;
-    assert readerFinishedListeners != null;
-    this.readerFinishedListeners = readerFinishedListeners;
     applyAllDeletes = false;
 
     // we put the old SegmentReaders in a map, that allows us
@@ -225,7 +211,6 @@ class DirectoryReader extends IndexReade
 
           // this is a new reader; in case we hit an exception we can close it safely
           newReader = SegmentReader.get(readOnly, infos.info(i), termInfosIndexDivisor);
-          newReader.readerFinishedListeners = readerFinishedListeners;
           readerShared[i] = false;
           newReaders[i] = newReader;
         } else {
@@ -236,7 +221,6 @@ class DirectoryReader extends IndexReade
             readerShared[i] = true;
             newReaders[i].incRef();
           } else {
-            assert newReader.readerFinishedListeners == readerFinishedListeners;
             readerShared[i] = false;
             // Steal ref returned to us by reopenSegment:
             newReaders[i] = newReader;
@@ -377,7 +361,6 @@ class DirectoryReader extends IndexReade
       writeLock = null;
       hasChanges = false;
     }
-    assert newReader.readerFinishedListeners != null;
 
     return newReader;
   }
@@ -431,7 +414,6 @@ class DirectoryReader extends IndexReade
       return null;
     }
 
-    reader.readerFinishedListeners = readerFinishedListeners;
     return reader;
   }
 
@@ -501,9 +483,9 @@ class DirectoryReader extends IndexReade
   private synchronized DirectoryReader doOpenIfChanged(SegmentInfos infos, boolean doClone, boolean openReadOnly) throws CorruptIndexException, IOException {
     DirectoryReader reader;
     if (openReadOnly) {
-      reader = new ReadOnlyDirectoryReader(directory, infos, subReaders, starts, normsCache, doClone, termInfosIndexDivisor, readerFinishedListeners);
+      reader = new ReadOnlyDirectoryReader(directory, infos, subReaders, starts, normsCache, doClone, termInfosIndexDivisor);
     } else {
-      reader = new DirectoryReader(directory, infos, subReaders, starts, normsCache, false, doClone, termInfosIndexDivisor, readerFinishedListeners);
+      reader = new DirectoryReader(directory, infos, subReaders, starts, normsCache, false, doClone, termInfosIndexDivisor);
     }
     return reader;
   }

Modified: lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/FilterIndexReader.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/FilterIndexReader.java?rev=1213932&r1=1213931&r2=1213932&view=diff
==============================================================================
--- lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/FilterIndexReader.java (original)
+++ lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/FilterIndexReader.java Tue Dec 13 21:19:30 2011
@@ -20,12 +20,10 @@ package org.apache.lucene.index;
 import org.apache.lucene.document.Document;
 import org.apache.lucene.document.FieldSelector;
 import org.apache.lucene.store.Directory;
-import org.apache.lucene.util.MapBackedSet;
 
 import java.io.IOException;
 import java.util.Collection;
 import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
 
 /**  A <code>FilterIndexReader</code> contains another IndexReader, which it
  * uses as its basic source of data, possibly transforming the data along the
@@ -114,7 +112,6 @@ public class FilterIndexReader extends I
   public FilterIndexReader(IndexReader in) {
     super();
     this.in = in;
-    readerFinishedListeners = new MapBackedSet<ReaderFinishedListener>(new ConcurrentHashMap<ReaderFinishedListener,Boolean>());
   }
 
   @Override
@@ -335,17 +332,4 @@ public class FilterIndexReader extends I
     buffer.append(')');
     return buffer.toString();
   }
-
-  @Override
-  public void addReaderFinishedListener(ReaderFinishedListener listener) {
-    super.addReaderFinishedListener(listener);
-    in.addReaderFinishedListener(listener);
-  }
-
-  @Override
-  public void removeReaderFinishedListener(ReaderFinishedListener listener) {
-    super.removeReaderFinishedListener(listener);
-    in.removeReaderFinishedListener(listener);
-  }
 }
-

Modified: lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/IndexReader.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/IndexReader.java?rev=1213932&r1=1213931&r2=1213932&view=diff
==============================================================================
--- lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/IndexReader.java (original)
+++ lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/IndexReader.java Tue Dec 13 21:19:30 2011
@@ -23,15 +23,17 @@ import java.io.FileOutputStream;
 import java.io.IOException;
 import java.util.Collection;
 import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import org.apache.lucene.document.Document;
 import org.apache.lucene.document.FieldSelector;
-import org.apache.lucene.search.FieldCache; // javadocs
 import org.apache.lucene.search.SearcherManager; // javadocs
 import org.apache.lucene.search.Similarity;
 import org.apache.lucene.store.*;
 import org.apache.lucene.util.ArrayUtil;
+import org.apache.lucene.util.MapBackedSet;
 import org.apache.lucene.util.CommandLineUtil;
 import org.apache.lucene.util.VirtualMethod;
 
@@ -82,62 +84,40 @@ public abstract class IndexReader implem
 
   /**
    * A custom listener that's invoked when the IndexReader
-   * is finished.
-   *
-   * <p>For a SegmentReader, this listener is called only
-   * once all SegmentReaders sharing the same core are
-   * closed.  At this point it is safe for apps to evict
-   * this reader from any caches keyed on {@link
-   * #getCoreCacheKey}.  This is the same interface that
-   * {@link FieldCache} uses, internally, to evict
-   * entries.</p>
-   *
-   * <p>For other readers, this listener is called when they
-   * are closed.</p>
+   * is closed.
    *
    * @lucene.experimental
    */
-  public static interface ReaderFinishedListener {
-    public void finished(IndexReader reader);
+  public static interface ReaderClosedListener {
+    public void onClose(IndexReader reader);
   }
 
-  // Impls must set this if they may call add/removeReaderFinishedListener:
-  protected volatile Collection<ReaderFinishedListener> readerFinishedListeners;
+  private final Set<ReaderClosedListener> readerClosedListeners = 
+      new MapBackedSet<ReaderClosedListener>(new ConcurrentHashMap<ReaderClosedListener, Boolean>());
 
-  /** Expert: adds a {@link ReaderFinishedListener}.  The
-   * provided listener is also added to any sub-readers, if
-   * this is a composite reader.  Also, any reader reopened
-   * or cloned from this one will also copy the listeners at
-   * the time of reopen.
+  /** Expert: adds a {@link ReaderClosedListener}.  The
+   * provided listener will be invoked when this reader is closed.
    *
    * @lucene.experimental */
-  public void addReaderFinishedListener(ReaderFinishedListener listener) {
+  public final void addReaderClosedListener(ReaderClosedListener listener) {
     ensureOpen();
-    readerFinishedListeners.add(listener);
+    readerClosedListeners.add(listener);
   }
 
-  /** Expert: remove a previously added {@link ReaderFinishedListener}.
+  /** Expert: remove a previously added {@link ReaderClosedListener}.
    *
    * @lucene.experimental */
-  public void removeReaderFinishedListener(ReaderFinishedListener listener) {
+  public final void removeReaderClosedListener(ReaderClosedListener listener) {
     ensureOpen();
-    readerFinishedListeners.remove(listener);
+    readerClosedListeners.remove(listener);
   }
 
-  protected void notifyReaderFinishedListeners() {
-    // Defensive (should never be null -- all impls must set
-    // this):
-    if (readerFinishedListeners != null) {
-      for(ReaderFinishedListener listener : readerFinishedListeners) {
-        listener.finished(this);
-      }
+  private final void notifyReaderClosedListeners() {
+    for(ReaderClosedListener listener : readerClosedListeners) {
+      listener.onClose(this);
     }
   }
 
-  protected void readerFinished() {
-    notifyReaderFinishedListeners();
-  }
-
   /**
    * Constants describing field properties, for example used for
    * {@link IndexReader#getFieldNames(FieldOption)}.
@@ -280,7 +260,7 @@ public abstract class IndexReader implem
           refCount.incrementAndGet();
         }
       }
-      readerFinished();
+      notifyReaderClosedListeners();
     } else if (rc < 0) {
       throw new IllegalStateException("too many decRef calls: refCount is " + rc + " after decrement");
     }

Modified: lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/IndexWriter.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/IndexWriter.java?rev=1213932&r1=1213931&r2=1213932&view=diff
==============================================================================
--- lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/IndexWriter.java (original)
+++ lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/IndexWriter.java Tue Dec 13 21:19:30 2011
@@ -31,7 +31,6 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import org.apache.lucene.analysis.Analyzer;
@@ -47,7 +46,6 @@ import org.apache.lucene.store.Directory
 import org.apache.lucene.store.Lock;
 import org.apache.lucene.store.LockObtainFailedException;
 import org.apache.lucene.util.Constants;
-import org.apache.lucene.util.MapBackedSet;
 import org.apache.lucene.util.StringHelper;
 import org.apache.lucene.util.ThreadInterruptedException;
 import org.apache.lucene.util.TwoPhaseCommit;
@@ -465,13 +463,6 @@ public class IndexWriter implements Clos
     return r;
   }
 
-  // Used for all SegmentReaders we open
-  private final Collection<IndexReader.ReaderFinishedListener> readerFinishedListeners = new MapBackedSet<IndexReader.ReaderFinishedListener>(new ConcurrentHashMap<IndexReader.ReaderFinishedListener,Boolean>());
-
-  Collection<IndexReader.ReaderFinishedListener> getReaderFinishedListeners() throws IOException {
-    return readerFinishedListeners;
-  }
-
   /** Holds shared SegmentReader instances. IndexWriter uses
    *  SegmentReaders for 1) applying deletes, 2) doing
    *  merges, 3) handing out a real-time reader.  This pool
@@ -704,7 +695,6 @@ public class IndexWriter implements Clos
         // synchronized
         // Returns a ref, which we xfer to readerMap:
         sr = SegmentReader.get(false, info.dir, info, readBufferSize, doOpenStores, termsIndexDivisor);
-        sr.readerFinishedListeners = readerFinishedListeners;
 
         if (info.dir == directory) {
           // Only pool if reader is not external

Modified: lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/MultiReader.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/MultiReader.java?rev=1213932&r1=1213931&r2=1213932&view=diff
==============================================================================
--- lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/MultiReader.java (original)
+++ lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/MultiReader.java Tue Dec 13 21:19:30 2011
@@ -22,7 +22,6 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
 
 import org.apache.lucene.document.Document;
 import org.apache.lucene.document.FieldSelector;
@@ -30,7 +29,6 @@ import org.apache.lucene.index.Directory
 import org.apache.lucene.index.DirectoryReader.MultiTermEnum;
 import org.apache.lucene.index.DirectoryReader.MultiTermPositions;
 import org.apache.lucene.search.Similarity;
-import org.apache.lucene.util.MapBackedSet;
 
 /** An IndexReader which reads multiple indexes, appending
  * their content. */
@@ -63,8 +61,7 @@ public class MultiReader extends IndexRe
    * @param subReaders set of (sub)readers
    */
   public MultiReader(IndexReader[] subReaders, boolean closeSubReaders) {
-    this(subReaders.clone(), new boolean[subReaders.length],
-      new MapBackedSet<ReaderFinishedListener>(new ConcurrentHashMap<ReaderFinishedListener,Boolean>()));
+    this(subReaders.clone(), new boolean[subReaders.length]);
     for (int i = 0; i < subReaders.length; i++) {
       if (!closeSubReaders) {
         subReaders[i].incRef();
@@ -75,11 +72,9 @@ public class MultiReader extends IndexRe
     }
   }
   
-  private MultiReader(IndexReader[] subReaders, boolean[] decrefOnClose,
-                      Collection<ReaderFinishedListener> readerFinishedListeners) {
+  private MultiReader(IndexReader[] subReaders, boolean[] decrefOnClose) {
     this.subReaders = subReaders;
     this.decrefOnClose = decrefOnClose;
-    this.readerFinishedListeners = readerFinishedListeners;
     starts = new int[subReaders.length + 1];    // build starts array
     int maxDoc = 0;
     for (int i = 0; i < subReaders.length; i++) {
@@ -179,7 +174,7 @@ public class MultiReader extends IndexRe
           newDecrefOnClose[i] = true;
         }
       }
-      return new MultiReader(newSubReaders, newDecrefOnClose, readerFinishedListeners);
+      return new MultiReader(newSubReaders, newDecrefOnClose);
     } else {
       return null;
     }
@@ -465,20 +460,4 @@ public class MultiReader extends IndexRe
   public IndexReader[] getSequentialSubReaders() {
     return subReaders;
   }
-
-  @Override
-  public void addReaderFinishedListener(ReaderFinishedListener listener) {
-    super.addReaderFinishedListener(listener);
-    for(IndexReader sub : subReaders) {
-      sub.addReaderFinishedListener(listener);
-    }
-  }
-
-  @Override
-  public void removeReaderFinishedListener(ReaderFinishedListener listener) {
-    super.removeReaderFinishedListener(listener);
-    for(IndexReader sub : subReaders) {
-      sub.removeReaderFinishedListener(listener);
-    }
-  }
 }

Modified: lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/ParallelReader.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/ParallelReader.java?rev=1213932&r1=1213931&r2=1213932&view=diff
==============================================================================
--- lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/ParallelReader.java (original)
+++ lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/ParallelReader.java Tue Dec 13 21:19:30 2011
@@ -21,11 +21,9 @@ import org.apache.lucene.document.Docume
 import org.apache.lucene.document.FieldSelector;
 import org.apache.lucene.document.FieldSelectorResult;
 import org.apache.lucene.document.Fieldable;
-import org.apache.lucene.util.MapBackedSet;
 
 import java.io.IOException;
 import java.util.*;
-import java.util.concurrent.ConcurrentHashMap;
 
 
 /** An IndexReader which reads multiple, parallel indexes.  Each index added
@@ -69,7 +67,6 @@ public class ParallelReader extends Inde
   public ParallelReader(boolean closeSubReaders) throws IOException {
     super();
     this.incRefReaders = !closeSubReaders;
-    readerFinishedListeners = new MapBackedSet<ReaderFinishedListener>(new ConcurrentHashMap<ReaderFinishedListener,Boolean>());
   }
 
   /** {@inheritDoc} */
@@ -658,22 +655,6 @@ public class ParallelReader extends Inde
       return ((TermPositions) termDocs).isPayloadAvailable();
     }
   }
-
-  @Override
-  public void addReaderFinishedListener(ReaderFinishedListener listener) {
-    super.addReaderFinishedListener(listener);
-    for (IndexReader reader : readers) {
-      reader.addReaderFinishedListener(listener);
-    }
-  }
-
-  @Override
-  public void removeReaderFinishedListener(ReaderFinishedListener listener) {
-    super.removeReaderFinishedListener(listener);
-    for (IndexReader reader : readers) {
-      reader.removeReaderFinishedListener(listener);
-    }
-  }
 }
 
 

Modified: lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/ReadOnlyDirectoryReader.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/ReadOnlyDirectoryReader.java?rev=1213932&r1=1213931&r2=1213932&view=diff
==============================================================================
--- lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/ReadOnlyDirectoryReader.java (original)
+++ lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/ReadOnlyDirectoryReader.java Tue Dec 13 21:19:30 2011
@@ -21,17 +21,15 @@ import org.apache.lucene.store.Directory
 
 import java.io.IOException;
 import java.util.Map;
-import java.util.Collection;
 
 class ReadOnlyDirectoryReader extends DirectoryReader {
-  ReadOnlyDirectoryReader(Directory directory, SegmentInfos sis, IndexDeletionPolicy deletionPolicy, int termInfosIndexDivisor,
-                          Collection<ReaderFinishedListener> readerFinishedListeners) throws IOException {
-    super(directory, sis, deletionPolicy, true, termInfosIndexDivisor, readerFinishedListeners);
+  ReadOnlyDirectoryReader(Directory directory, SegmentInfos sis, IndexDeletionPolicy deletionPolicy, int termInfosIndexDivisor) throws IOException {
+    super(directory, sis, deletionPolicy, true, termInfosIndexDivisor);
   }
 
   ReadOnlyDirectoryReader(Directory directory, SegmentInfos infos, SegmentReader[] oldReaders, int[] oldStarts,  Map<String,byte[]> oldNormsCache, boolean doClone,
-                          int termInfosIndexDivisor, Collection<ReaderFinishedListener> readerFinishedListeners) throws IOException {
-    super(directory, infos, oldReaders, oldStarts, oldNormsCache, true, doClone, termInfosIndexDivisor, readerFinishedListeners);
+                          int termInfosIndexDivisor) throws IOException {
+    super(directory, infos, oldReaders, oldStarts, oldNormsCache, true, doClone, termInfosIndexDivisor);
   }
   
   ReadOnlyDirectoryReader(IndexWriter writer, SegmentInfos infos, int termInfosIndexDivisor, boolean applyAllDeletes) throws IOException {

Modified: lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/SegmentCoreReaders.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/SegmentCoreReaders.java?rev=1213932&r1=1213931&r2=1213932&view=diff
==============================================================================
--- lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/SegmentCoreReaders.java (original)
+++ lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/SegmentCoreReaders.java Tue Dec 13 21:19:30 2011
@@ -18,11 +18,15 @@ package org.apache.lucene.index;
  */
 
 import java.io.IOException;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import org.apache.lucene.index.SegmentReader.CoreClosedListener;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.IndexInput;
 import org.apache.lucene.util.IOUtils;
+import org.apache.lucene.util.MapBackedSet;
 
 /** Holds core readers that are shared (unchanged) when
  * SegmentReader is cloned or reopened */
@@ -55,6 +59,9 @@ final class SegmentCoreReaders {
   CompoundFileReader cfsReader;
   CompoundFileReader storeCFSReader;
 
+  final Set<CoreClosedListener> coreClosedListeners = 
+      new MapBackedSet<CoreClosedListener>(new ConcurrentHashMap<CoreClosedListener, Boolean>());
+
   SegmentCoreReaders(SegmentReader owner, Directory dir, SegmentInfo si, int readBufferSize, int termsIndexDivisor) throws IOException {
     segment = si.name;
     this.readBufferSize = readBufferSize;
@@ -164,8 +171,8 @@ final class SegmentCoreReaders {
                     fieldsReaderOrig, cfsReader, storeCFSReader);
       tis = null;
       // Now, notify any ReaderFinished listeners:
-      if (owner != null) {
-        owner.notifyReaderFinishedListeners();
+      for (CoreClosedListener listener : coreClosedListeners) {
+        listener.onClose(owner);
       }
     }
   }

Modified: lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/SegmentReader.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/SegmentReader.java?rev=1213932&r1=1213931&r2=1213932&view=diff
==============================================================================
--- lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/SegmentReader.java (original)
+++ lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/index/SegmentReader.java Tue Dec 13 21:19:30 2011
@@ -31,6 +31,7 @@ import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 import org.apache.lucene.document.Document;
 import org.apache.lucene.document.FieldSelector;
+import org.apache.lucene.search.FieldCache;
 import org.apache.lucene.search.Similarity;
 import org.apache.lucene.index.FieldInfo.IndexOptions;
 import org.apache.lucene.store.BufferedIndexInput;
@@ -255,7 +256,6 @@ public class SegmentReader extends Index
       clone.si = si;
       clone.readBufferSize = readBufferSize;
       clone.pendingDeleteCount = pendingDeleteCount;
-      clone.readerFinishedListeners = readerFinishedListeners;
 
       if (!openReadOnly && hasChanges) {
         // My pending changes transfer to the new reader
@@ -936,14 +936,33 @@ public class SegmentReader extends Index
   public int getTermInfosIndexDivisor() {
     return core.termsIndexDivisor;
   }
-
-  @Override
-  protected void readerFinished() {
-    // Do nothing here -- we have more careful control on
-    // when to notify that a SegmentReader has finished,
-    // because a given core is shared across many cloned
-    // SegmentReaders.  We only notify once that core is no
-    // longer used (all SegmentReaders sharing it have been
-    // closed).
+  
+  /**
+   * Called when the shared core for this SegmentReader
+   * is closed.
+   * <p>
+   * This listener is called only once all SegmentReaders 
+   * sharing the same core are closed.  At this point it 
+   * is safe for apps to evict this reader from any caches 
+   * keyed on {@link #getCoreCacheKey}.  This is the same 
+   * interface that {@link FieldCache} uses, internally, 
+   * to evict entries.</p>
+   * 
+   * @lucene.experimental
+   */
+  public static interface CoreClosedListener {
+    public void onClose(SegmentReader owner);
+  }
+  
+  /** Expert: adds a CoreClosedListener to this reader's shared core */
+  public void addCoreClosedListener(CoreClosedListener listener) {
+    ensureOpen();
+    core.coreClosedListeners.add(listener);
+  }
+  
+  /** Expert: removes a CoreClosedListener from this reader's shared core */
+  public void removeCoreClosedListener(CoreClosedListener listener) {
+    ensureOpen();
+    core.coreClosedListeners.remove(listener);
   }
 }

Modified: lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/search/FieldCacheImpl.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/search/FieldCacheImpl.java?rev=1213932&r1=1213931&r2=1213932&view=diff
==============================================================================
--- lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/search/FieldCacheImpl.java (original)
+++ lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/search/FieldCacheImpl.java Tue Dec 13 21:19:30 2011
@@ -26,6 +26,7 @@ import java.util.Map;
 import java.util.WeakHashMap;
 
 import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.index.SegmentReader;
 import org.apache.lucene.index.Term;
 import org.apache.lucene.index.TermDocs;
 import org.apache.lucene.index.TermEnum;
@@ -134,11 +135,12 @@ class FieldCacheImpl implements FieldCac
    */
   static final class StopFillCacheException extends RuntimeException {
   }
-
-  final static IndexReader.ReaderFinishedListener purgeReader = new IndexReader.ReaderFinishedListener() {
+  
+  // per-segment fieldcaches don't purge until the shared core closes.
+  final static SegmentReader.CoreClosedListener purgeCore = new SegmentReader.CoreClosedListener() {
     // @Override -- not until Java 1.6
-    public void finished(IndexReader reader) {
-      FieldCache.DEFAULT.purge(reader);
+    public void onClose(SegmentReader owner) {
+      FieldCache.DEFAULT.purge(owner);
     }
   };
 
@@ -177,7 +179,16 @@ class FieldCacheImpl implements FieldCac
           // First time this reader is using FieldCache
           innerCache = new HashMap<Entry,Object>();
           readerCache.put(readerKey, innerCache);
-          reader.addReaderFinishedListener(purgeReader);
+          if (reader instanceof SegmentReader) {
+            ((SegmentReader) reader).addCoreClosedListener(purgeCore);
+          } else {
+            reader.addReaderClosedListener(new IndexReader.ReaderClosedListener() {
+              @Override
+              public void onClose(IndexReader reader) {
+                FieldCache.DEFAULT.purge(reader);
+              }
+            });
+          }
         }
         if (innerCache.get(key) == null) {
           innerCache.put(key, value);
@@ -198,7 +209,16 @@ class FieldCacheImpl implements FieldCac
           // First time this reader is using FieldCache
           innerCache = new HashMap<Entry,Object>();
           readerCache.put(readerKey, innerCache);
-          reader.addReaderFinishedListener(purgeReader);
+          if (reader instanceof SegmentReader) {
+            ((SegmentReader) reader).addCoreClosedListener(purgeCore);
+          } else {
+            reader.addReaderClosedListener(new IndexReader.ReaderClosedListener() {
+              @Override
+              public void onClose(IndexReader reader) {
+                FieldCache.DEFAULT.purge(reader);
+              }
+            });           
+          }
           value = null;
         } else {
           value = innerCache.get(key);

Modified: lucene/dev/branches/branch_3x/lucene/src/test/org/apache/lucene/index/TestIndexReader.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_3x/lucene/src/test/org/apache/lucene/index/TestIndexReader.java?rev=1213932&r1=1213931&r2=1213932&view=diff
==============================================================================
--- lucene/dev/branches/branch_3x/lucene/src/test/org/apache/lucene/index/TestIndexReader.java (original)
+++ lucene/dev/branches/branch_3x/lucene/src/test/org/apache/lucene/index/TestIndexReader.java Tue Dec 13 21:19:30 2011
@@ -22,11 +22,11 @@ import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
-import java.util.HashMap;
 import java.util.Random;
 import java.util.Set;
 import java.util.SortedSet;
@@ -44,8 +44,8 @@ import org.apache.lucene.search.FieldCac
 import org.apache.lucene.store.AlreadyClosedException;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.LockObtainFailedException;
-import org.apache.lucene.store.NoSuchDirectoryException;
 import org.apache.lucene.store.LockReleaseFailedException;
+import org.apache.lucene.store.NoSuchDirectoryException;
 import org.apache.lucene.util.LuceneTestCase;
 import org.apache.lucene.util._TestUtil;
 
@@ -1310,29 +1310,26 @@ public class TestIndexReader extends Luc
     writer.commit();
     final IndexReader reader = writer.getReader();
     final int[] closeCount = new int[1];
-    final IndexReader.ReaderFinishedListener listener = new IndexReader.ReaderFinishedListener() {
-      public void finished(IndexReader reader) {
+    final IndexReader.ReaderClosedListener listener = new IndexReader.ReaderClosedListener() {
+      public void onClose(IndexReader reader) {
         closeCount[0]++;
       }
     };
 
-    reader.addReaderFinishedListener(listener);
+    reader.addReaderClosedListener(listener);
 
     reader.close();
 
-    // Just the top reader
+    // Close the top reader, its the only one that should be closed
     assertEquals(1, closeCount[0]);
     writer.close();
 
-    // Now also the subs
-    assertEquals(3, closeCount[0]);
-
     IndexReader reader2 = IndexReader.open(dir);
-    reader2.addReaderFinishedListener(listener);
+    reader2.addReaderClosedListener(listener);
 
     closeCount[0] = 0;
     reader2.close();
-    assertEquals(3, closeCount[0]);
+    assertEquals(1, closeCount[0]);
     dir.close();
   }