Removed static part from PluginRegistry to avoid static initializer (#12799)

Removed static part from PluginRegistry to avoid static initializer, transforming it into a singleton. Reflections library is not thread safe in jar archives scan, so there must be only one instance of PluginRegistry

Author: andsel

logstash-core/src/main/java/org/logstash/plugins/PluginLookup.java

@@ -32,6 +32,7 @@
 import org.jruby.runtime.builtin.IRubyObject;
 import org.logstash.RubyUtil;
 import org.logstash.plugins.discovery.PluginRegistry;
+import org.logstash.plugins.factory.PluginFactoryExt;
 
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -40,20 +41,23 @@
  * Java Implementation of the plugin that is implemented by wrapping the Ruby
  * {@code LogStash::Plugin} class for the Ruby plugin lookup.
  */
-public final class PluginLookup {
+public final class PluginLookup implements PluginFactoryExt.PluginResolver {
 
     private static final IRubyObject RUBY_REGISTRY = RubyUtil.RUBY.executeScript(
             "require 'logstash/plugins/registry'\nrequire 'logstash/plugin'\nLogStash::Plugin",
             ""
     );
 
-    private PluginLookup() {
-        // Utility Class
+    private final PluginRegistry pluginRegistry;
+
+    public PluginLookup(PluginRegistry pluginRegistry) {
+        this.pluginRegistry = pluginRegistry;
     }
 
     @SuppressWarnings("rawtypes")
-    public static PluginLookup.PluginClass lookup(final PluginLookup.PluginType type, final String name) {
-        Class<?> javaClass = PluginRegistry.getPluginClass(type, name);
+    @Override
+    public PluginClass resolve(PluginType type, String name) {
+        Class<?> javaClass = pluginRegistry.getPluginClass(type, name);
         if (javaClass != null) {
 
             if (!PluginValidator.validatePlugin(type, javaClass)) {

logstash-core/src/main/java/org/logstash/plugins/discovery/PluginRegistry.java

@@ -37,40 +37,58 @@
 import java.util.Set;
 
 /**
- * Registry for built-in Java plugins (not installed via logstash-plugin)
- */
+ * Registry for built-in Java plugins (not installed via logstash-plugin).
+ * This is singleton ofr two reasons:
+ * <ul>
+ *  <li>it's a registry so no need for multiple instances</li>
+ *  <li>the Reflections library used need to run in single thread during discovery phase</li>
+ * </ul>
+ * */
 public final class PluginRegistry {
 
-    private static final Map<String, Class<Input>> INPUTS = new HashMap<>();
-    private static final Map<String, Class<Filter>> FILTERS = new HashMap<>();
-    private static final Map<String, Class<Output>> OUTPUTS = new HashMap<>();
-    private static final Map<String, Class<Codec>> CODECS = new HashMap<>();
+    private final Map<String, Class<Input>> inputs = new HashMap<>();
+    private final Map<String, Class<Filter>> filters = new HashMap<>();
+    private final Map<String, Class<Output>> outputs = new HashMap<>();
+    private final Map<String, Class<Codec>> codecs = new HashMap<>();
+    private static final Object LOCK = new Object();
+    private static volatile PluginRegistry INSTANCE;
 
-    static {
+    private PluginRegistry() {
         discoverPlugins();
     }
 
-    private PluginRegistry() {} // utility class
-
+    public static PluginRegistry getInstance() {
+        if (INSTANCE == null) {
+            synchronized (LOCK) {
+                if (INSTANCE == null) {
+                    INSTANCE = new PluginRegistry();
+                }
+            }
+        }
+        return INSTANCE;
+    }
+    
     @SuppressWarnings("unchecked")
-    private static void discoverPlugins() {
+    private void discoverPlugins() {
+        // the constructor of Reflection must be called only by one thread, else there is a
+        // risk that the first thread that completes close the Zip files for the others.
         Reflections reflections = new Reflections("org.logstash.plugins");
         Set<Class<?>> annotated = reflections.getTypesAnnotatedWith(LogstashPlugin.class);
         for (final Class<?> cls : annotated) {
             for (final Annotation annotation : cls.getAnnotations()) {
                 if (annotation instanceof LogstashPlugin) {
                     String name = ((LogstashPlugin) annotation).name();
                     if (Filter.class.isAssignableFrom(cls)) {
-                        FILTERS.put(name, (Class<Filter>) cls);
+                        filters.put(name, (Class<Filter>) cls);
                     }
                     if (Output.class.isAssignableFrom(cls)) {
-                        OUTPUTS.put(name, (Class<Output>) cls);
+                        outputs.put(name, (Class<Output>) cls);
                     }
                     if (Input.class.isAssignableFrom(cls)) {
-                        INPUTS.put(name, (Class<Input>) cls);
+                        inputs.put(name, (Class<Input>) cls);
                     }
                     if (Codec.class.isAssignableFrom(cls)) {
-                        CODECS.put(name, (Class<Codec>) cls);
+                        codecs.put(name, (Class<Codec>) cls);
                     }
 
                     break;
@@ -79,7 +97,7 @@ private static void discoverPlugins() {
         }
     }
 
-    public static Class<?> getPluginClass(PluginLookup.PluginType pluginType, String pluginName) {
+    public Class<?> getPluginClass(PluginLookup.PluginType pluginType, String pluginName) {
         if (pluginType == PluginLookup.PluginType.FILTER) {
             return getFilterClass(pluginName);
         }
@@ -97,31 +115,31 @@ public static Class<?> getPluginClass(PluginLookup.PluginType pluginType, String
 
     }
 
-    public static Class<Input> getInputClass(String name) {
-        return INPUTS.get(name);
+    public Class<Input> getInputClass(String name) {
+        return inputs.get(name);
     }
 
-    public static Class<Filter> getFilterClass(String name) {
-        return FILTERS.get(name);
+    public Class<Filter> getFilterClass(String name) {
+        return filters.get(name);
     }
 
-    public static Class<Codec> getCodecClass(String name) {
-        return CODECS.get(name);
+    public Class<Codec> getCodecClass(String name) {
+        return codecs.get(name);
     }
 
-    public static Class<Output> getOutputClass(String name) {
-        return OUTPUTS.get(name);
+    public Class<Output> getOutputClass(String name) {
+        return outputs.get(name);
     }
 
-    public static Codec getCodec(String name, Configuration configuration, Context context) {
-        if (name != null && CODECS.containsKey(name)) {
-            return instantiateCodec(CODECS.get(name), configuration, context);
+    public Codec getCodec(String name, Configuration configuration, Context context) {
+        if (name != null && codecs.containsKey(name)) {
+            return instantiateCodec(codecs.get(name), configuration, context);
         }
         return null;
     }
 
     @SuppressWarnings({"unchecked","rawtypes"})
-    private static Codec instantiateCodec(Class clazz, Configuration configuration, Context context) {
+    private Codec instantiateCodec(Class clazz, Configuration configuration, Context context) {
         try {
             Constructor<Codec> constructor = clazz.getConstructor(Configuration.class, Context.class);
             return constructor.newInstance(configuration, context);

logstash-core/src/main/java/org/logstash/plugins/factory/PluginFactoryExt.java

@@ -20,6 +20,7 @@
 import org.logstash.instrument.metrics.MetricKeys;
 import org.logstash.plugins.ConfigVariableExpander;
 import org.logstash.plugins.PluginLookup;
+import org.logstash.plugins.discovery.PluginRegistry;
 
 import java.util.*;
 import java.util.concurrent.ConcurrentHashMap;
@@ -81,7 +82,7 @@ public static IRubyObject filterDelegator(final ThreadContext context,
     }
 
     public PluginFactoryExt(final Ruby runtime, final RubyClass metaClass) {
-        this(runtime, metaClass, PluginLookup::lookup);
+        this(runtime, metaClass, new PluginLookup(PluginRegistry.getInstance()));
     }
 
     PluginFactoryExt(final Ruby runtime, final RubyClass metaClass, PluginResolver pluginResolver) {