fix #640

Author: skybber

hotswap-agent-core/src/main/java/org/hotswap/agent/config/PluginManager.java

@@ -27,6 +27,7 @@
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.locks.ReentrantLock;
 
 import org.hotswap.agent.command.Scheduler;
 import org.hotswap.agent.command.impl.SchedulerImpl;
@@ -37,7 +38,7 @@
 import org.hotswap.agent.watch.WatcherFactory;
 
 /**
- * The main agent plugin manager, well known singleton controller.
+ * The main agent plugin manager, well-known singleton controller.
  *
  * @author Jiri Bubnik
  */
@@ -46,7 +47,7 @@ public class PluginManager {
 
     public static final String PLUGIN_PACKAGE = "org.hotswap.agent.plugin";
 
-    //////////////////////////   MANAGER SINGLETON /////////////////////////////////////
+    ////////////////////////// MANAGER SINGLETON /////////////////////////////////////
 
     // singleton instance
     private static PluginManager INSTANCE = new PluginManager();
@@ -68,6 +69,7 @@ private PluginManager() {
     private Instrumentation instrumentation;
 
     private Object hotswapLock = new Object();
+    private ReentrantLock initClassLoaderLock = new ReentrantLock();
 
     //////////////////////////   PLUGINS /////////////////////////////////////
 
@@ -113,8 +115,8 @@ public boolean isPluginInitialized(String pluginClassName, ClassLoader classLoad
     /**
      * Initialize the singleton plugin manager.
      * <ul>
-     * <li>Create new resource watcher using WatcherFactory and start it in separate thread.</li>
-     * <li>Create new scheduler and start it in separate thread.</li>
+     * <li>Create a new resource watcher using WatcherFactory and start it in separate thread.</li>
+     * <li>Create a new scheduler and start it in separate thread.</li>
      * <li>Scan for plugins</li>
      * <li>Register HotswapTransformer with the javaagent instrumentation class</li>
      * </ul>
@@ -178,7 +180,8 @@ public void initClassLoader(ClassLoader classLoader, ProtectionDomain protection
 
         // synchronize ClassLoader patching - multiple classloaders may be patched at the same time
         // and they may synchronize loading for security reasons and introduce deadlocks
-        synchronized (this) {
+        try {
+            initClassLoaderLock.lock();
             if (classLoaderConfigurations.containsKey(classLoader))
                 return;
 
@@ -192,6 +195,8 @@ public void initClassLoader(ClassLoader classLoader, ProtectionDomain protection
             PluginConfiguration pluginConfiguration = new PluginConfiguration(getPluginConfiguration(getClass().getClassLoader()), classLoader, false);
             classLoaderConfigurations.put(classLoader, pluginConfiguration);
             pluginConfiguration.init();
+        } finally {
+            initClassLoaderLock.unlock();
         }
 
         // call listeners
@@ -203,7 +208,7 @@ public void initClassLoader(ClassLoader classLoader, ProtectionDomain protection
      * Remove any classloader reference and close all plugin instances associated with classloader.
      * This method is called typically after webapp undeploy.
      *
-     * @param classLoader the classloader to cleanup
+     * @param classLoader the classloader to clean up
      */
     public void closeClassLoader(ClassLoader classLoader) {
         pluginRegistry.closeClassLoader(classLoader);
@@ -325,4 +330,12 @@ public void scheduleHotswap(Map<Class<?>, byte[]> reloadMap, int timeout) {
         getScheduler().scheduleCommand(new ScheduledHotswapCommand(reloadMap), timeout);
     }
 
+    /**
+     * Retrieves the lock used to synchronize initialization of the class loader for the plugin manager.
+     *
+     * @return the lock instance used for class loader initialization synchronization.
+     */
+    public ReentrantLock getInitClassLoaderLock() {
+        return initClassLoaderLock;
+    }
 }

hotswap-agent-core/src/main/java/org/hotswap/agent/util/HotswapTransformer.java

@@ -41,7 +41,7 @@
 /**
  * Java instrumentation transformer.
  * <p/>
- * The is the single instance of transformer registered by HotswapAgent. It will delegate to plugins to
+ * This is the single instance of transformer registered by HotswapAgent. It will delegate to plugins to
  * do the transformer work.
  *
  * @author Jiri Bubnik
@@ -107,7 +107,7 @@ public List<Pattern> getExcludedClassLoaderPatterns() {
      * @param classLoader the classloader to which this transformation is associated
      * @param classNameRegexp regexp to match fully qualified class name.
      *                        Because "." is any character in regexp, this will match / in the transform method as well
-     *                        (diffentence between java/lang/String and java.lang.String).
+     *                        (difference between java/lang/String and java.lang.String).
      * @param transformer     the transformer to be called for each class matching regexp.
      */
     public void registerTransformer(ClassLoader classLoader, String classNameRegexp, HaClassFileTransformer transformer) {
@@ -184,7 +184,7 @@ public void closeClassLoader(ClassLoader classLoader) {
      * <p>It does not do the instrumentation itself, instead iterates registered transformers and compares
      * registration class regexp - if the regexp matches, the classloader is called.
      * <p/>
-     * <p>Note that class bytes may be send to multiple transformers, but the order is not defined.
+     * <p>Note that class bytes may be sent to multiple transformers, but the order is not defined.
      *
      * @see ClassFileTransformer#transform(ClassLoader, String, Class, java.security.ProtectionDomain, byte[])
      */
@@ -204,7 +204,7 @@ public byte[] transform(final ClassLoader classLoader, String className, Class<?
         List<PluginClassFileTransformer> pluginTransformers = new ArrayList<>();
         try {
             // 1. call transform method of defining transformers
-            for (RegisteredTransformersRecord transformerRecord : new ArrayList<RegisteredTransformersRecord>(otherTransformers.values())) {
+            for (RegisteredTransformersRecord transformerRecord : new ArrayList<>(otherTransformers.values())) {
                 if ((className != null && transformerRecord.pattern.matcher(className).matches()) ||
                         (redefiningClass != null && transformerRecord.pattern.matcher(redefiningClass.getName()).matches())) {
                     for (ClassFileTransformer transformer : new ArrayList<ClassFileTransformer>(transformerRecord.transformerList)) {
@@ -221,7 +221,7 @@ public byte[] transform(final ClassLoader classLoader, String className, Class<?
             }
             // 2. call transform method of redefining transformers
             if (redefiningClass != null && className != null) {
-                for (RegisteredTransformersRecord transformerRecord : new ArrayList<RegisteredTransformersRecord>(redefinitionTransformers.values())) {
+                for (RegisteredTransformersRecord transformerRecord : new ArrayList<>(redefinitionTransformers.values())) {
                     if (transformerRecord.pattern.matcher(className).matches()) {
                         for (ClassFileTransformer transformer : new ArrayList<ClassFileTransformer>(transformerRecord.transformerList)) {
                             if(transformer instanceof PluginClassFileTransformer) {
@@ -308,11 +308,11 @@ LinkedList<PluginClassFileTransformer> reduce(final ClassLoader classLoader, Lis
     }
     /**
      * Every classloader should be initialized. Usually if anything interesting happens,
-     * it is initialized during plugin initialization process. However, some plugins (e.g. Hotswapper)
+     * it is initialized during the plugin initialization process. However, some plugins (e.g. Hotswapper)
      * are triggered during classloader initialization process itself (@Init on static method). In this case,
      * the plugin will be never invoked, until the classloader initialization is invoked from here.
      *
-     * Schedule with some timeout to allow standard plugin initialization process to precede.
+     * Schedule with some timeout to allow a standard plugin initialization process to precede.
      *
      * @param classLoader the classloader to which this transformation is associated
      * @param protectionDomain associated protection domain (if any)
@@ -322,7 +322,13 @@ protected boolean ensureClassLoaderInitialized(final ClassLoader classLoader, fi
 
             if (classLoader == null) {
                 // directly init null (bootstrap) classloader
-                PluginManager.getInstance().initClassLoader(null, protectionDomain);
+                if (PluginManager.getInstance().getInitClassLoaderLock().tryLock()) {
+                    try {
+                        PluginManager.getInstance().initClassLoader(null, protectionDomain);
+                    } finally {
+                        PluginManager.getInstance().getInitClassLoaderLock().unlock();
+                    }
+                }
             } else {
                 // ensure the classloader should not be excluded
                 if (shouldScheduleClassLoader(classLoader)) {
@@ -364,10 +370,13 @@ private boolean shouldScheduleClassLoader(final ClassLoader classLoader) {
 
 
     /**
-     * Transform type to ^regexp$ form - match only whole pattern.
+     * Normalizes a given regular expression for matching types by ensuring
+     * it starts with a caret (^) and ends with a dollar sign ($).
      *
-     * @param registeredType type
-     * @return
+     * @param registeredType the input regular expression for a type which may or may not
+     *                       include starting or ending anchors.
+     * @return a string representing the normalized regular expression, guaranteed
+     *         to start with ^ and end with $.
      */
     protected String normalizeTypeRegexp(String registeredType) {
         String regexp = registeredType;