svn commit: r1667803 - in /lucene/dev/branches/branch_5x: ./ solr/ solr/core/ solr/core/src/java/org/apache/solr/core/PluginBag.java solr/core/src/java/org/apache/solr/core/RequestHandlers.java

[yo...@apache.org]

Author: yonik
Date: Thu Mar 19 16:14:23 2015
New Revision: 1667803

URL: http://svn.apache.org/r1667803
Log:
SOLR-7262: fix broken thread safety for request handler registry introduced by SOLR-7073

Modified:
    lucene/dev/branches/branch_5x/   (props changed)
    lucene/dev/branches/branch_5x/solr/   (props changed)
    lucene/dev/branches/branch_5x/solr/core/   (props changed)
    lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/core/PluginBag.java
    lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/core/RequestHandlers.java

Modified: lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/core/PluginBag.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/core/PluginBag.java?rev=1667803&r1=1667802&r2=1667803&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/core/PluginBag.java (original)
+++ lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/core/PluginBag.java Thu Mar 19 16:14:23 2015
@@ -27,6 +27,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 
 import org.apache.lucene.analysis.util.ResourceLoader;
 import org.apache.lucene.analysis.util.ResourceLoaderAware;
@@ -49,22 +50,33 @@ import org.slf4j.LoggerFactory;
 public class PluginBag<T> implements AutoCloseable {
   public static Logger log = LoggerFactory.getLogger(PluginBag.class);
 
-  private Map<String, PluginHolder<T>> registry = new HashMap<>();
-  private Map<String, PluginHolder<T>> immutableRegistry = Collections.unmodifiableMap(registry);
+  private final Map<String, PluginHolder<T>> registry;
+  private final Map<String, PluginHolder<T>> immutableRegistry;
   private String def;
-  private Class klass;
+  private final Class klass;
   private SolrCore core;
-  private SolrConfig.SolrPluginInfo meta;
+  private final SolrConfig.SolrPluginInfo meta;
 
-  public PluginBag(Class<T> klass, SolrCore core) {
+  /** Pass needThreadSafety=true if plugins can be added and removed concurrently with lookups. */
+  public PluginBag(Class<T> klass, SolrCore core, boolean needThreadSafety) {
     this.core = core;
     this.klass = klass;
+    // TODO: since reads will dominate writes, we could also think about creating a new instance of a map each time it changes.
+    // Not sure how much benefit this would have over ConcurrentHashMap though
+    // We could also perhaps make this constructor into a factory method to return different implementations depending on thread safety needs.
+    this.registry = needThreadSafety ? new ConcurrentHashMap<String, PluginHolder<T>>() : new HashMap<String, PluginHolder<T>>();
+    this.immutableRegistry = Collections.unmodifiableMap(registry);
     meta = SolrConfig.classVsSolrPluginInfo.get(klass.getName());
     if (meta == null) {
       throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown Plugin : " + klass.getName());
     }
   }
 
+  /** Constructs a non-threadsafe plugin registry */
+  public PluginBag(Class<T> klass, SolrCore core) {
+    this(klass, core, false);
+  }
+
   static void initInstance(Object inst, PluginInfo info, SolrCore core) {
     if (inst instanceof PluginInfoInitialized) {
       ((PluginInfoInitialized) inst).init(info);
@@ -97,6 +109,7 @@ public class PluginBag<T> implements Aut
   }
 
   boolean alias(String src, String target) {
+    if (src == null) return false;
     PluginHolder<T> a = registry.get(src);
     if (a == null) return false;
     PluginHolder<T> b = registry.get(target);

Modified: lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/core/RequestHandlers.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/core/RequestHandlers.java?rev=1667803&r1=1667802&r2=1667803&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/core/RequestHandlers.java (original)
+++ lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/core/RequestHandlers.java Thu Mar 19 16:14:23 2015
@@ -57,7 +57,8 @@ public final class RequestHandlers {
   
   public RequestHandlers(SolrCore core) {
       this.core = core;
-    handlers =  new PluginBag<>(SolrRequestHandler.class, core);
+    // we need a thread safe registry since methods like register are currently documented to be thread safe.
+    handlers =  new PluginBag<>(SolrRequestHandler.class, core, true);
   }
 
   /**