From bb0074f6765a4211306f6947ee369d0d65d8996a Mon Sep 17 00:00:00 2001 From: Andrea Selva Date: Tue, 13 Apr 2021 15:50:05 +0200 Subject: [PATCH] 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 --- .../org/logstash/plugins/PluginLookup.java | 14 ++-- .../plugins/discovery/PluginRegistry.java | 72 ++++++++++++------- .../plugins/factory/PluginFactoryExt.java | 3 +- 3 files changed, 56 insertions(+), 33 deletions(-) diff --git a/logstash-core/src/main/java/org/logstash/plugins/PluginLookup.java b/logstash-core/src/main/java/org/logstash/plugins/PluginLookup.java index c61bacc756d..663c1bb867c 100644 --- a/logstash-core/src/main/java/org/logstash/plugins/PluginLookup.java +++ b/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)) { diff --git a/logstash-core/src/main/java/org/logstash/plugins/discovery/PluginRegistry.java b/logstash-core/src/main/java/org/logstash/plugins/discovery/PluginRegistry.java index 7b77283f112..fd2daeb6b9b 100644 --- a/logstash-core/src/main/java/org/logstash/plugins/discovery/PluginRegistry.java +++ b/logstash-core/src/main/java/org/logstash/plugins/discovery/PluginRegistry.java @@ -37,23 +37,41 @@ 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: + * + * */ public final class PluginRegistry { - private static final Map> INPUTS = new HashMap<>(); - private static final Map> FILTERS = new HashMap<>(); - private static final Map> OUTPUTS = new HashMap<>(); - private static final Map> CODECS = new HashMap<>(); + private final Map> inputs = new HashMap<>(); + private final Map> filters = new HashMap<>(); + private final Map> outputs = new HashMap<>(); + private final Map> 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> annotated = reflections.getTypesAnnotatedWith(LogstashPlugin.class); for (final Class cls : annotated) { @@ -61,16 +79,16 @@ private static void discoverPlugins() { if (annotation instanceof LogstashPlugin) { String name = ((LogstashPlugin) annotation).name(); if (Filter.class.isAssignableFrom(cls)) { - FILTERS.put(name, (Class) cls); + filters.put(name, (Class) cls); } if (Output.class.isAssignableFrom(cls)) { - OUTPUTS.put(name, (Class) cls); + outputs.put(name, (Class) cls); } if (Input.class.isAssignableFrom(cls)) { - INPUTS.put(name, (Class) cls); + inputs.put(name, (Class) cls); } if (Codec.class.isAssignableFrom(cls)) { - CODECS.put(name, (Class) cls); + codecs.put(name, (Class) 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 getInputClass(String name) { - return INPUTS.get(name); + public Class getInputClass(String name) { + return inputs.get(name); } - public static Class getFilterClass(String name) { - return FILTERS.get(name); + public Class getFilterClass(String name) { + return filters.get(name); } - public static Class getCodecClass(String name) { - return CODECS.get(name); + public Class getCodecClass(String name) { + return codecs.get(name); } - public static Class getOutputClass(String name) { - return OUTPUTS.get(name); + public Class 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 constructor = clazz.getConstructor(Configuration.class, Context.class); return constructor.newInstance(configuration, context); diff --git a/logstash-core/src/main/java/org/logstash/plugins/factory/PluginFactoryExt.java b/logstash-core/src/main/java/org/logstash/plugins/factory/PluginFactoryExt.java index 3ba787545ca..9846d22fd67 100644 --- a/logstash-core/src/main/java/org/logstash/plugins/factory/PluginFactoryExt.java +++ b/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) {