LUCENE-651: Fixed a race condition in initialization of FieldCache.

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

CHANGES.txt

@@ -185,12 +185,16 @@ Bug fixes
     to recognize ordered spans if they overlaped with unordered spans.
     (Paul Elschot via Chris Hostetter)
 
-20. LUCENE-706: Updated fileformats.xml|html concerning the docdelta value in the frequency file. (Johan Stuyts, Doron Cohen via Grant Ingersoll)
+20. LUCENE-706: Updated fileformats.xml|html concerning the docdelta value
+    in the frequency file. (Johan Stuyts, Doron Cohen via Grant Ingersoll)
 
 21. LUCENE-715: Fixed private constructor in IndexWriter.java to
     properly release the acquired write lock if there is an
     IOException after acquiring the write lock but before finishing
-    instantiation (Matthew Bogosian via Mike McCandless)
+    instantiation. (Matthew Bogosian via Mike McCandless)
+
+22. LUCENE-651: Fixed a race condition in initialization of FieldCache.
+    (Oliver Hutchison via Otis Gospodnetic)
 
 Optimizations
 

src/java/org/apache/lucene/search/FieldCacheImpl.java

@@ -40,8 +40,52 @@ import java.util.HashMap;
  */
 class FieldCacheImpl
 implements FieldCache {
+	
+  /** Expert: Internal cache. */
+  abstract static class Cache {
+    private final Map readerCache = new WeakHashMap();
+    
+    protected abstract Object createValue(IndexReader reader, Object key)
+        throws IOException;
 
-  /** Expert: Every key in the internal cache is of this type. */
+    public Object get(IndexReader reader, Object key) throws IOException {
+      Map innerCache;
+      Object value;
+      synchronized (readerCache) {
+        innerCache = (Map) readerCache.get(reader);
+        if (innerCache == null) {
+          innerCache = new HashMap();
+          readerCache.put(reader, innerCache);
+          value = null;
+        } else {
+          value = innerCache.get(key);
+        }
+        if (value == null) {
+          value = new CreationPlaceholder();
+          innerCache.put(reader, value);
+        }
+      }
+      if (value instanceof CreationPlaceholder) {
+        synchronized (value) {
+          CreationPlaceholder progress = (CreationPlaceholder) value;
+          if (progress.value == null) {
+            progress.value = createValue(reader, key);
+            synchronized (readerCache) {
+              innerCache.put(key, progress.value);
+            }
+          }
+          return progress.value;
+        }
+      }
+      return value;
+    }
+  }
+
+  static final class CreationPlaceholder {
+    Object value;
+  }
+
+  /** Expert: Every composite-key in the internal cache is of this type. */
   static class Entry {
     final String field;        // which Fieldable
     final int type;            // which SortField type
@@ -99,66 +143,24 @@ implements FieldCache {
       }
     };
 
-  /** The internal cache. Maps Entry to array of interpreted term values. **/
-  final Map cache = new WeakHashMap();
-
-  /** See if an object is in the cache. */
-  Object lookup (IndexReader reader, String field, int type, Locale locale) {
-    Entry entry = new Entry (field, type, locale);
-    synchronized (this) {
-      HashMap readerCache = (HashMap)cache.get(reader);
-      if (readerCache == null) return null;
-      return readerCache.get (entry);
-    }
-  }
-
-  /** See if a custom object is in the cache. */
-  Object lookup (IndexReader reader, String field, Object comparer) {
-    Entry entry = new Entry (field, comparer);
-    synchronized (this) {
-      HashMap readerCache = (HashMap)cache.get(reader);
-      if (readerCache == null) return null;
-      return readerCache.get (entry);
-    }
-  }
-
-  /** Put an object into the cache. */
-  Object store (IndexReader reader, String field, int type, Locale locale, Object value) {
-    Entry entry = new Entry (field, type, locale);
-    synchronized (this) {
-      HashMap readerCache = (HashMap)cache.get(reader);
-      if (readerCache == null) {
-        readerCache = new HashMap();
-        cache.put(reader,readerCache);
-      }
-      return readerCache.put (entry, value);
-    }
-  }
-
-  /** Put a custom object into the cache. */
-  Object store (IndexReader reader, String field, Object comparer, Object value) {
-    Entry entry = new Entry (field, comparer);
-    synchronized (this) {
-      HashMap readerCache = (HashMap)cache.get(reader);
-      if (readerCache == null) {
-        readerCache = new HashMap();
-        cache.put(reader, readerCache);
-      }
-      return readerCache.put (entry, value);
-    }
-  }
-
   // inherit javadocs
   public int[] getInts (IndexReader reader, String field) throws IOException {
     return getInts(reader, field, INT_PARSER);
   }
 
   // inherit javadocs
-  public int[] getInts (IndexReader reader, String field, IntParser parser)
+  public int[] getInts(IndexReader reader, String field, IntParser parser)
-  throws IOException {
+      throws IOException {
-    field = field.intern();
+    return (int[]) intsCache.get(reader, new Entry(field, parser));
-    Object ret = lookup (reader, field, parser);
+  }
-    if (ret == null) {
+
+  Cache intsCache = new Cache() {
+
+    protected Object createValue(IndexReader reader, Object entryKey)
+        throws IOException {
+      Entry entry = (Entry) entryKey;
+      String field = entry.field;
+      IntParser parser = (IntParser) entry.custom;
       final int[] retArray = new int[reader.maxDoc()];
       TermDocs termDocs = reader.termDocs();
       TermEnum termEnum = reader.terms (new Term (field, ""));
@@ -176,11 +178,9 @@ implements FieldCache {
         termDocs.close();
         termEnum.close();
       }
-      store (reader, field, parser, retArray);
       return retArray;
     }
-    return (int[]) ret;
+  };
-  }
 
   // inherit javadocs
   public float[] getFloats (IndexReader reader, String field)
@@ -189,11 +189,18 @@ implements FieldCache {
   }
 
   // inherit javadocs
-  public float[] getFloats (IndexReader reader, String field,
+  public float[] getFloats(IndexReader reader, String field, FloatParser parser)
-                            FloatParser parser) throws IOException {
+      throws IOException {
-    field = field.intern();
+    return (float[]) floatsCache.get(reader, new Entry(field, parser));
-    Object ret = lookup (reader, field, parser);
+  }
-    if (ret == null) {
+
+  Cache floatsCache = new Cache() {
+
+    protected Object createValue(IndexReader reader, Object entryKey)
+        throws IOException {
+      Entry entry = (Entry) entryKey;
+      String field = entry.field;
+      FloatParser parser = (FloatParser) entry.custom;
       final float[] retArray = new float[reader.maxDoc()];
       TermDocs termDocs = reader.termDocs();
       TermEnum termEnum = reader.terms (new Term (field, ""));
@@ -211,18 +218,21 @@ implements FieldCache {
         termDocs.close();
         termEnum.close();
       }
-      store (reader, field, parser, retArray);
       return retArray;
     }
-    return (float[]) ret;
+  };
-  }
 
   // inherit javadocs
-  public String[] getStrings (IndexReader reader, String field)
+  public String[] getStrings(IndexReader reader, String field)
-  throws IOException {
+      throws IOException {
-    field = field.intern();
+    return (String[]) stringsCache.get(reader, field);
-    Object ret = lookup (reader, field, SortField.STRING, null);
+  }
-    if (ret == null) {
+
+  Cache stringsCache = new Cache() {
+
+    protected Object createValue(IndexReader reader, Object fieldKey)
+        throws IOException {
+      String field = ((String) fieldKey).intern();
       final String[] retArray = new String[reader.maxDoc()];
       TermDocs termDocs = reader.termDocs();
       TermEnum termEnum = reader.terms (new Term (field, ""));
@@ -240,18 +250,21 @@ implements FieldCache {
         termDocs.close();
         termEnum.close();
       }
-      store (reader, field, SortField.STRING, null, retArray);
       return retArray;
     }
-    return (String[]) ret;
+  };
-  }
 
   // inherit javadocs
-  public StringIndex getStringIndex (IndexReader reader, String field)
+  public StringIndex getStringIndex(IndexReader reader, String field)
-  throws IOException {
+      throws IOException {
-    field = field.intern();
+    return (StringIndex) stringsIndexCache.get(reader, field);
-    Object ret = lookup (reader, field, STRING_INDEX, null);
+  }
-    if (ret == null) {
+
+  Cache stringsIndexCache = new Cache() {
+
+    protected Object createValue(IndexReader reader, Object fieldKey)
+        throws IOException {
+      String field = ((String) fieldKey).intern();
       final int[] retArray = new int[reader.maxDoc()];
       String[] mterms = new String[reader.maxDoc()+1];
       TermDocs termDocs = reader.termDocs();
@@ -301,11 +314,9 @@ implements FieldCache {
       }
 
       StringIndex value = new StringIndex (retArray, mterms);
-      store (reader, field, STRING_INDEX, null, value);
       return value;
     }
-    return (StringIndex) ret;
+  };
-  }
 
   /** The pattern used to detect integer values in a field */
   /** removed for java 1.3 compatibility
@@ -318,18 +329,23 @@ implements FieldCache {
    * protected static final Object pFloats = Pattern.compile ("[0-9+\\-\\.eEfFdD]+");
    */
 
-  // inherit javadocs
+	// inherit javadocs
-  public Object getAuto (IndexReader reader, String field)
+  public Object getAuto(IndexReader reader, String field) throws IOException {
-  throws IOException {
+    return autoCache.get(reader, field);
-    field = field.intern();
+  }
-    Object ret = lookup (reader, field, SortField.AUTO, null);
+
-    if (ret == null) {
+  Cache autoCache = new Cache() {
+
+    protected Object createValue(IndexReader reader, Object fieldKey)
+        throws IOException {
+      String field = ((String)fieldKey).intern();
       TermEnum enumerator = reader.terms (new Term (field, ""));
       try {
         Term term = enumerator.term();
         if (term == null) {
           throw new RuntimeException ("no terms in field " + field + " - cannot determine sort type");
         }
+        Object ret = null;
         if (term.field() == field) {
           String termtext = term.text().trim();
 
@@ -354,27 +370,30 @@ implements FieldCache {
             } catch (NumberFormatException nfe2) {
               ret = getStringIndex (reader, field);
             }
-          }
+          }          
-          if (ret != null) {
-            store (reader, field, SortField.AUTO, null, ret);
-          }
         } else {
           throw new RuntimeException ("field \"" + field + "\" does not appear to be indexed");
         }
+        return ret;
       } finally {
         enumerator.close();
       }
-
     }
-    return ret;
+  };
-  }
 
   // inherit javadocs
-  public Comparable[] getCustom (IndexReader reader, String field, SortComparator comparator)
+  public Comparable[] getCustom(IndexReader reader, String field,
-  throws IOException {
+      SortComparator comparator) throws IOException {
-    field = field.intern();
+    return (Comparable[]) customCache.get(reader, new Entry(field, comparator));
-    Object ret = lookup (reader, field, comparator);
+  }
-    if (ret == null) {
+
+  Cache customCache = new Cache() {
+
+    protected Object createValue(IndexReader reader, Object entryKey)
+        throws IOException {
+      Entry entry = (Entry) entryKey;
+      String field = entry.field;
+      SortComparator comparator = (SortComparator) entry.custom;
       final Comparable[] retArray = new Comparable[reader.maxDoc()];
       TermDocs termDocs = reader.termDocs();
       TermEnum termEnum = reader.terms (new Term (field, ""));
@@ -392,11 +411,9 @@ implements FieldCache {
         termDocs.close();
         termEnum.close();
       }
-      store (reader, field, comparator, retArray);
       return retArray;
     }
-    return (Comparable[]) ret;
+  };
-  }
+  
-
 }
 

src/java/org/apache/lucene/search/FieldSortedHitQueue.java

@@ -21,9 +21,6 @@ import org.apache.lucene.index.IndexReader;
 import org.apache.lucene.util.PriorityQueue;
 
 import java.io.IOException;
-import java.util.WeakHashMap;
-import java.util.HashMap;
-import java.util.Map;
 import java.util.Locale;
 import java.text.Collator;
 
@@ -147,44 +144,29 @@ extends PriorityQueue {
   SortField[] getFields() {
     return fields;
   }
-
+  
-  /** Internal cache of comparators. Similar to FieldCache, only
+  static ScoreDocComparator getCachedComparator (IndexReader reader, String field, int type, Locale locale, SortComparatorSource factory)
-   *  caches comparators instead of term values. */
-  static final Map Comparators = new WeakHashMap();
-
-  /** Returns a comparator if it is in the cache. */
-  static ScoreDocComparator lookup (IndexReader reader, String field, int type, Locale locale, Object factory) {
-    FieldCacheImpl.Entry entry = (factory != null)
-      ? new FieldCacheImpl.Entry (field, factory)
-      : new FieldCacheImpl.Entry (field, type, locale);
-    synchronized (Comparators) {
-      HashMap readerCache = (HashMap)Comparators.get(reader);
-      if (readerCache == null) return null;
-      return (ScoreDocComparator) readerCache.get (entry);
-    }
-  }
-
-  /** Stores a comparator into the cache. */
-  static Object store (IndexReader reader, String field, int type, Locale locale, Object factory, Object value) {
-    FieldCacheImpl.Entry entry = (factory != null)
-      ? new FieldCacheImpl.Entry (field, factory)
-      : new FieldCacheImpl.Entry (field, type, locale);
-    synchronized (Comparators) {
-      HashMap readerCache = (HashMap)Comparators.get(reader);
-      if (readerCache == null) {
-        readerCache = new HashMap();
-        Comparators.put(reader,readerCache);
-      }
-      return readerCache.put (entry, value);
-    }
-  }
-
-  static ScoreDocComparator getCachedComparator (IndexReader reader, String fieldname, int type, Locale locale, SortComparatorSource factory)
   throws IOException {
     if (type == SortField.DOC) return ScoreDocComparator.INDEXORDER;
     if (type == SortField.SCORE) return ScoreDocComparator.RELEVANCE;
-    ScoreDocComparator comparator = lookup (reader, fieldname, type, locale, factory);
+    FieldCacheImpl.Entry entry = (factory != null)
-    if (comparator == null) {
+      ? new FieldCacheImpl.Entry (field, factory)
+      : new FieldCacheImpl.Entry (field, type, locale);
+    return (ScoreDocComparator)Comparators.get(reader, entry);
+  }
+
+  /** Internal cache of comparators. Similar to FieldCache, only
+   *  caches comparators instead of term values. */
+  static final FieldCacheImpl.Cache Comparators = new FieldCacheImpl.Cache() {
+
+    protected Object createValue(IndexReader reader, Object entryKey)
+        throws IOException {
+      FieldCacheImpl.Entry entry = (FieldCacheImpl.Entry) entryKey;
+      String fieldname = entry.field;
+      int type = entry.type;
+      Locale locale = entry.locale;
+      SortComparatorSource factory = (SortComparatorSource) entry.custom;
+      ScoreDocComparator comparator;
       switch (type) {
         case SortField.AUTO:
           comparator = comparatorAuto (reader, fieldname);
@@ -205,10 +187,9 @@ extends PriorityQueue {
         default:
           throw new RuntimeException ("unknown field type: "+type);
       }
-      store (reader, fieldname, type, locale, factory, comparator);
+      return comparator;
     }
-    return comparator;
+  };
-  }
 
   /**
    * Returns a comparator for sorting hits according to a field containing integers.