LUCENE-6482: Fix class loading deadlock relating to Codec initialization, default codec and SPI discovery

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

lucene/CHANGES.txt

@@ -80,6 +80,9 @@ Bug fixes
 * LUCENE-6523: NRT readers now reflect a new commit even if there is
   no change to the commit user data (Mike McCandless)
 
+* LUCENE-6482: Fix class loading deadlock relating to Codec initialization,
+  default codec and SPI discovery.  (Shikhar Bhushan, Uwe Schindler)
+
 Changes in Runtime Behavior
 
 * LUCENE-6501: The subreader structure in ParallelCompositeReader

lucene/core/src/java/org/apache/lucene/codecs/Codec.java

@@ -17,6 +17,7 @@ package org.apache.lucene.codecs;
  * limitations under the License.
  */
 
+import java.util.Objects;
 import java.util.Set;
 import java.util.ServiceLoader; // javadocs
 
@@ -38,8 +39,26 @@ import org.apache.lucene.util.NamedSPILoader;
  */
 public abstract class Codec implements NamedSPILoader.NamedSPI {
 
-  private static final NamedSPILoader<Codec> loader =
-    new NamedSPILoader<>(Codec.class);
+  /**
+   * This static holder class prevents classloading deadlock by delaying
+   * init of default codecs and available codecs until needed.
+   */
+  private static final class Holder {
+    private static final NamedSPILoader<Codec> LOADER = new NamedSPILoader<>(Codec.class);
+    
+    private Holder() {}
+    
+    static NamedSPILoader<Codec> getLoader() {
+      if (LOADER == null) {
+        throw new IllegalStateException("You tried to lookup a Codec by name before all Codecs could be initialized. "+
+          "This likely happens if you call Codec#forName from a Codec's ctor.");
+      }
+      return LOADER;
+    }
+    
+    // TODO: should we use this, or maybe a system property is better?
+    static Codec defaultCodec = LOADER.lookup("Lucene50");
+  }
 
   private final String name;
 
@@ -91,20 +110,12 @@ public abstract class Codec implements NamedSPILoader.NamedSPI {
   
   /** looks up a codec by name */
   public static Codec forName(String name) {
-    if (loader == null) {
-      throw new IllegalStateException("You called Codec.forName() before all Codecs could be initialized. "+
-          "This likely happens if you call it from a Codec's ctor.");
-    }
-    return loader.lookup(name);
+    return Holder.getLoader().lookup(name);
   }
   
   /** returns a list of all available codec names */
   public static Set<String> availableCodecs() {
-    if (loader == null) {
-      throw new IllegalStateException("You called Codec.availableCodecs() before all Codecs could be initialized. "+
-          "This likely happens if you call it from a Codec's ctor.");
-    }
-    return loader.availableServices();
+    return Holder.getLoader().availableServices();
   }
   
   /** 
@@ -119,24 +130,25 @@ public abstract class Codec implements NamedSPILoader.NamedSPI {
    * of new codecs on the given classpath/classloader!</em>
    */
   public static void reloadCodecs(ClassLoader classloader) {
-    loader.reload(classloader);
+    Holder.getLoader().reload(classloader);
   }
-  
-  private static Codec defaultCodec = Codec.forName("Lucene50");
-  
+    
   /** expert: returns the default codec used for newly created
    *  {@link IndexWriterConfig}s.
    */
-  // TODO: should we use this, or maybe a system property is better?
   public static Codec getDefault() {
-    return defaultCodec;
+    if (Holder.defaultCodec == null) {
+      throw new IllegalStateException("You tried to lookup the default Codec before all Codecs could be initialized. "+
+        "This likely happens if you try to get it from a Codec's ctor.");
+    }
+    return Holder.defaultCodec;
   }
   
   /** expert: sets the default codec used for newly created
    *  {@link IndexWriterConfig}s.
    */
   public static void setDefault(Codec codec) {
-    defaultCodec = codec;
+    Holder.defaultCodec = Objects.requireNonNull(codec);
   }
 
   /**

lucene/core/src/java/org/apache/lucene/codecs/DocValuesFormat.java

@@ -40,8 +40,23 @@ import org.apache.lucene.util.NamedSPILoader;
  * @lucene.experimental */
 public abstract class DocValuesFormat implements NamedSPILoader.NamedSPI {
   
-  private static final NamedSPILoader<DocValuesFormat> loader =
-      new NamedSPILoader<>(DocValuesFormat.class);
+  /**
+   * This static holder class prevents classloading deadlock by delaying
+   * init of doc values formats until needed.
+   */
+  private static final class Holder {
+    private static final NamedSPILoader<DocValuesFormat> LOADER = new NamedSPILoader<>(DocValuesFormat.class);
+    
+    private Holder() {}
+    
+    static NamedSPILoader<DocValuesFormat> getLoader() {
+      if (LOADER == null) {
+        throw new IllegalStateException("You tried to lookup a DocValuesFormat by name before all formats could be initialized. "+
+          "This likely happens if you call DocValuesFormat#forName from a DocValuesFormat's ctor.");
+      }
+      return LOADER;
+    }
+  }
   
   /** Unique name that's used to retrieve this format when
    *  reading the index.
@@ -90,20 +105,12 @@ public abstract class DocValuesFormat implements NamedSPILoader.NamedSPI {
   
   /** looks up a format by name */
   public static DocValuesFormat forName(String name) {
-    if (loader == null) {
-      throw new IllegalStateException("You called DocValuesFormat.forName() before all formats could be initialized. "+
-          "This likely happens if you call it from a DocValuesFormat's ctor.");
-    }
-    return loader.lookup(name);
+    return Holder.getLoader().lookup(name);
   }
   
   /** returns a list of all available format names */
   public static Set<String> availableDocValuesFormats() {
-    if (loader == null) {
-      throw new IllegalStateException("You called DocValuesFormat.availableDocValuesFormats() before all formats could be initialized. "+
-          "This likely happens if you call it from a DocValuesFormat's ctor.");
-    }
-    return loader.availableServices();
+    return Holder.getLoader().availableServices();
   }
   
   /** 
@@ -118,6 +125,6 @@ public abstract class DocValuesFormat implements NamedSPILoader.NamedSPI {
    * of new docvalues formats on the given classpath/classloader!</em>
    */
   public static void reloadDocValuesFormats(ClassLoader classloader) {
-    loader.reload(classloader);
+    Holder.getLoader().reload(classloader);
   }
 }

lucene/core/src/java/org/apache/lucene/codecs/PostingsFormat.java

@@ -41,8 +41,23 @@ import org.apache.lucene.util.NamedSPILoader;
  * @lucene.experimental */
 public abstract class PostingsFormat implements NamedSPILoader.NamedSPI {
 
-  private static final NamedSPILoader<PostingsFormat> loader =
-    new NamedSPILoader<>(PostingsFormat.class);
+  /**
+   * This static holder class prevents classloading deadlock by delaying
+   * init of postings formats until needed.
+   */
+  private static final class Holder {
+    private static final NamedSPILoader<PostingsFormat> LOADER = new NamedSPILoader<>(PostingsFormat.class);
+    
+    private Holder() {}
+    
+    static NamedSPILoader<PostingsFormat> getLoader() {
+      if (LOADER == null) {
+        throw new IllegalStateException("You tried to lookup a PostingsFormat by name before all formats could be initialized. "+
+          "This likely happens if you call PostingsFormat#forName from a PostingsFormat's ctor.");
+      }
+      return LOADER;
+    }
+  }
 
   /** Zero-length {@code PostingsFormat} array. */
   public static final PostingsFormat[] EMPTY = new PostingsFormat[0];
@@ -94,20 +109,12 @@ public abstract class PostingsFormat implements NamedSPILoader.NamedSPI {
   
   /** looks up a format by name */
   public static PostingsFormat forName(String name) {
-    if (loader == null) {
-      throw new IllegalStateException("You called PostingsFormat.forName() before all formats could be initialized. "+
-          "This likely happens if you call it from a PostingsFormat's ctor.");
-    }
-    return loader.lookup(name);
+    return Holder.getLoader().lookup(name);
   }
   
   /** returns a list of all available format names */
   public static Set<String> availablePostingsFormats() {
-    if (loader == null) {
-      throw new IllegalStateException("You called PostingsFormat.availablePostingsFormats() before all formats could be initialized. "+
-          "This likely happens if you call it from a PostingsFormat's ctor.");
-    }
-    return loader.availableServices();
+    return Holder.getLoader().availableServices();
   }
   
   /** 
@@ -122,6 +129,6 @@ public abstract class PostingsFormat implements NamedSPILoader.NamedSPI {
    * of new postings formats on the given classpath/classloader!</em>
    */
   public static void reloadPostingsFormats(ClassLoader classloader) {
-    loader.reload(classloader);
+    Holder.getLoader().reload(classloader);
   }
 }

lucene/core/src/java/org/apache/lucene/util/NamedSPILoader.java

@@ -58,7 +58,7 @@ public final class NamedSPILoader<S extends NamedSPILoader.NamedSPI> implements
    * <p><em>This method is expensive and should only be called for discovery
    * of new service providers on the given classpath/classloader!</em>
    */
-  public synchronized void reload(ClassLoader classloader) {
+  public void reload(ClassLoader classloader) {
     final LinkedHashMap<String,S> services = new LinkedHashMap<>(this.services);
     final SPIClassIterator<S> loader = SPIClassIterator.get(clazz, classloader);
     while (loader.hasNext()) {