docs/attachments/LUCENE-3485/LUCENE-3485.patch

Index: lucene/contrib/facet/src/test/org/apache/lucene/facet/taxonomy/lucene/TestLuceneTaxonomyReader.java
===================================================================
--- lucene/contrib/facet/src/test/org/apache/lucene/facet/taxonomy/lucene/TestLuceneTaxonomyReader.java	(revision 0)
+++ lucene/contrib/facet/src/test/org/apache/lucene/facet/taxonomy/lucene/TestLuceneTaxonomyReader.java	(revision 0)
@@ -0,0 +1,84 @@
+package org.apache.lucene.facet.taxonomy.lucene;
+
+import org.apache.lucene.facet.taxonomy.CategoryPath;
+import org.apache.lucene.store.AlreadyClosedException;
+import org.apache.lucene.store.Directory;
+import org.apache.lucene.util.LuceneTestCase;
+import org.junit.Test;
+
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+public class TestLuceneTaxonomyReader extends LuceneTestCase {
+
+  @Test
+  public void testCloseAfterIncRef() throws Exception {
+    // Verifies that nothing is committed to the underlying Directory, if
+    // commit() wasn't called.
+    Directory dir = newDirectory();
+    LuceneTaxonomyWriter ltw = new LuceneTaxonomyWriter(dir);
+    ltw.addCategory(new CategoryPath("a"));
+    ltw.close(); // first commit, so that an index will be created
+    
+    LuceneTaxonomyReader ltr = new LuceneTaxonomyReader(dir);
+    ltr.incRef();
+    ltr.close();
+    
+    // should not fail as we incRef() before close
+    ltr.getSize();
+    ltr.decRef();
+    
+    dir.close();
+  }
+  
+  @Test
+  public void testCloseTwice() throws Exception {
+    // Verifies that nothing is committed to the underlying Directory, if
+    // commit() wasn't called.
+    Directory dir = newDirectory();
+    LuceneTaxonomyWriter ltw = new LuceneTaxonomyWriter(dir);
+    ltw.addCategory(new CategoryPath("a"));
+    ltw.close(); // first commit, so that an index will be created
+    
+    LuceneTaxonomyReader ltr = new LuceneTaxonomyReader(dir);
+    ltr.close();
+    ltr.close(); // no exception should be thrown, we're
+    
+    dir.close();
+  }
+  
+  @Test
+  public void testAlreadyClosed() throws Exception {
+    // Verifies that nothing is committed to the underlying Directory, if
+    // commit() wasn't called.
+    Directory dir = newDirectory();
+    LuceneTaxonomyWriter ltw = new LuceneTaxonomyWriter(dir);
+    ltw.addCategory(new CategoryPath("a"));
+    ltw.close(); // first commit, so that an index will be created
+    
+    LuceneTaxonomyReader ltr = new LuceneTaxonomyReader(dir);
+    ltr.close();
+    try {
+      ltr.getSize();
+      fail("An AlreadyClosedException should have been thrown here");
+    } catch (AlreadyClosedException ace) {
+      // goot!
+    }
+    dir.close();
+  }
+  
+}
Index: lucene/contrib/facet/src/java/org/apache/lucene/facet/taxonomy/lucene/LuceneTaxonomyReader.java
===================================================================
--- lucene/contrib/facet/src/java/org/apache/lucene/facet/taxonomy/lucene/LuceneTaxonomyReader.java	(revision 1178826)
+++ lucene/contrib/facet/src/java/org/apache/lucene/facet/taxonomy/lucene/LuceneTaxonomyReader.java	(working copy)
@@ -1,6 +1,5 @@
 package org.apache.lucene.facet.taxonomy.lucene;
 
-import java.io.File;
 import java.io.IOException;
 import java.util.Iterator;
 import java.util.Map;
@@ -10,15 +9,14 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import org.apache.lucene.facet.taxonomy.CategoryPath;
+import org.apache.lucene.facet.taxonomy.TaxonomyReader;
 import org.apache.lucene.index.CorruptIndexException;
 import org.apache.lucene.index.IndexReader;
 import org.apache.lucene.index.Term;
 import org.apache.lucene.index.TermDocs;
+import org.apache.lucene.store.AlreadyClosedException;
 import org.apache.lucene.store.Directory;
-import org.apache.lucene.store.FSDirectory;
-
-import org.apache.lucene.facet.taxonomy.CategoryPath;
-import org.apache.lucene.facet.taxonomy.TaxonomyReader;
 import org.apache.lucene.util.collections.LRUHashMap;
 
 /**
@@ -98,6 +96,8 @@
 
   private char delimiter = Consts.DEFAULT_DELIMITER;
 
+  private volatile boolean closed = false;
+  
   /**
    * Open for reading a taxonomy stored in a given {@link Directory}.
    * @param directory
@@ -125,41 +125,16 @@
     return IndexReader.open(directory);
   }
 
-  // convenience constructors... deprecated because they cause confusion
-  // because they use parent directory instead of the actual directory.
-  private Directory ourDirectory = null; // remember directory to close later, but only if we opened it here
   /**
-   * Open for reading a taxonomy stored in a subdirectory of a given
-   * directory on the file system.
-   * @param parentDir The parent directory of the taxonomy's directory
-   * (usually this would be the directory holding the index).
-   * @param name The name of the taxonomy, and the subdirectory holding it. 
-   * @throws CorruptIndexException if the Taxonomy is corrupted.
-   * @throws IOException if another error occurred.
-   */  
-  @Deprecated
-  public LuceneTaxonomyReader(File parentDir, String name)
-  throws CorruptIndexException, IOException {
-    this(FSDirectory.open(new File(parentDir, name)));
-    ourDirectory = indexReader.directory(); // remember to close the directory we opened
+   * @throws AlreadyClosedException if this IndexReader is closed
+   */
+  protected final void ensureOpen() throws AlreadyClosedException {
+    if (indexReader.getRefCount() <= 0) {
+      throw new AlreadyClosedException("this TaxonomyReader is closed");
+    }
   }
-
+  
   /**
-   * Open for reading a taxonomy stored in a subdirectory of a given
-   * directory on the file system.
-   * @param parentDir The parent directory of the taxonomy's directory.
-   * @param name The name of the taxonomy, and the subdirectory holding it. 
-   * @throws CorruptIndexException if the Taxonomy is corrupted.
-   * @throws IOException if another error occurred.
-   */  
-  @Deprecated
-  public LuceneTaxonomyReader(String parentDir, String name)
-  throws CorruptIndexException, IOException {
-    this(FSDirectory.open(new File(parentDir, name)));
-    ourDirectory = indexReader.directory(); // rememebr to close the directory we opened
-  }
-
-  /**
    * setCacheSize controls the maximum allowed size of each of the caches
    * used by {@link #getPath(int)} and {@link #getOrdinal(CategoryPath)}.
    * <P>
@@ -169,6 +144,7 @@
    * @param size the new maximum cache size, in number of entries.
    */
   public void setCacheSize(int size) {
+    ensureOpen();
     synchronized(getCategoryCache) {
       getCategoryCache.setMaxSize(size);
     }
@@ -188,10 +164,12 @@
    * LuceneTaxonomyReader objects you create.
    */
   public void setDelimiter(char delimiter) {
+    ensureOpen();
     this.delimiter = delimiter;
   }
 
   public int getOrdinal(CategoryPath categoryPath) throws IOException {
+    ensureOpen();
     if (categoryPath.length()==0) {
       return ROOT_ORDINAL;
     }
@@ -233,6 +211,7 @@
   }
 
   public CategoryPath getPath(int ordinal) throws CorruptIndexException, IOException {
+    ensureOpen();
     // TODO (Facet): Currently, the LRU cache we use (getCategoryCache) holds
     // strings with delimiters, not CategoryPath objects, so even if
     // we have a cache hit, we need to process the string and build a new
@@ -249,6 +228,7 @@
   }
 
   public boolean getPath(int ordinal, CategoryPath result) throws CorruptIndexException, IOException {
+    ensureOpen();
     String label = getLabel(ordinal);
     if (label==null) {
       return false;
@@ -259,6 +239,7 @@
   }
 
   private String getLabel(int catID) throws CorruptIndexException, IOException {
+    ensureOpen();
     // First try to find the answer in the LRU cache. It is very
     // unfortunate that we need to allocate an Integer object here -
     // it would have been better if we used a hash table specifically
@@ -307,6 +288,7 @@
   }
 
   public int getParent(int ordinal) {
+    ensureOpen();
     // Note how we don't need to hold the read lock to do the following,
     // because the array reference is volatile, ensuring the correct
     // visibility and ordering: if we get the new reference, the new
@@ -337,6 +319,7 @@
    */
 
   public int[] getParentArray() {
+    ensureOpen();
     // Note how we don't need to hold the read lock to do the following,
     // because the array reference is volatile, ensuring the correct
     // visibility and ordering: if we get the new reference, the new
@@ -348,6 +331,7 @@
   // method in this class) to ensure that it never gets called concurrently
   // with itself.
   public synchronized void refresh() throws IOException {
+    ensureOpen();
     /*
      * Since refresh() can be a lengthy operation, it is very important that we
      * avoid locking out all readers for its duration. This is why we don't hold
@@ -409,13 +393,25 @@
   }
 
   public void close() throws IOException {
-    indexReader.close();
-    if (ourDirectory!=null) {
-      ourDirectory.close();
+    if (!closed) {
+      decRef();
+      closed = true;
     }
   }
+  
+  /** Actually close this {@link TaxonomyReader}, free up resources */
+  private void doClose() throws IOException {
+    closed = true;
+    indexReader.close();
 
+    parentArray = null;
+    childrenArrays = null;
+    getCategoryCache.clear();
+    getOrdinalCache.clear();
+  }
+
   public int getSize() {
+    ensureOpen();
     indexReaderLock.readLock().lock();
     try {
       return indexReader.numDocs();
@@ -425,6 +421,7 @@
   }
 
   public Map<String, String> getCommitUserData() {
+    ensureOpen();
     return indexReader.getCommitUserData();
   }
   
@@ -432,6 +429,7 @@
   Object childrenArraysRebuild = new Object();
 
   public ChildrenArrays getChildrenArrays() {
+    ensureOpen();
     // Check if the taxonomy grew since we built the array, and if it
     // did, create new (and larger) arrays and fill them as required.
     // We do all this under a lock, two prevent to concurrent calls to
@@ -485,6 +483,7 @@
   }
 
   public String toString(int max) {
+    ensureOpen();
     StringBuilder sb = new StringBuilder();
     int upperl = Math.min(max, this.indexReader.maxDoc());
     for (int i = 0; i < upperl; i++) {
@@ -530,6 +529,7 @@
    * @return lucene indexReader
    */
   IndexReader getInternalIndexReader() {
+    ensureOpen();
     return this.indexReader;
   }
 
@@ -540,13 +540,20 @@
    * @throws IOException 
    */
   public void decRef() throws IOException {
-    this.indexReader.decRef();
+    ensureOpen();
+    if (indexReader.getRefCount() == 1) {
+      // Do not decRef the indexReader - doClose does it by calling reader.close()
+      doClose();
+    } else {
+      indexReader.decRef();
+    }
   }
   
   /**
    * Expert: returns the current refCount for this taxonomy reader
    */
   public int getRefCount() {
+    ensureOpen();
     return this.indexReader.getRefCount();
   }
   
@@ -558,6 +565,7 @@
    * otherwise the reader may never be closed. 
    */
   public void incRef() {
+    ensureOpen();
     this.indexReader.incRef();
   }
 }