follow up fix for a race condition

git-svn-id: https://hudson.dev.java.net/svn/hudson/trunk/hudson/main@34300 71c3de6d-444a-0410-be80-ed276b4c234a

core/src/main/java/hudson/DescriptorExtensionList.java

@@ -134,7 +134,20 @@ public class DescriptorExtensionList<T extends Describable<T>, D extends Descrip
                 return d;
         return null;
     }
-    
+
+    /**
+     * {@link #load()} in the descriptor is not a real load activity, so locking against "this" is enough.
+     */
+    @Override
+    protected Object getLoadLock() {
+        return this;
+    }
+
+    @Override
+    protected void scoutLoad() {
+        // no-op, since our load() doesn't by itself do any classloading
+    }
+
     /**
      * Loading the descriptors in this case means filtering the descriptor from the master {@link ExtensionList}.
      */

core/src/main/java/hudson/ExtensionFinder.java

@@ -92,6 +92,36 @@ public abstract class ExtensionFinder implements ExtensionPoint {
         return find(type,hudson);
     }
 
+    /**
+     *
+     * If two threads try to initialize classes in the opposite order, a dead lock will ensue,
+     * and we can get into a similar situation with {@link ExtensionFinder}s.
+     *
+     * <p>
+     * That is, one thread can try to list extensions, which results in {@link ExtensionFinder}
+     * loading and initializing classes. This happens inside a context of a lock, so that
+     * another thread that tries to list the same extensions don't end up creating different
+     * extension instances. So this activity locks extension list first, then class initialization next.
+     *
+     * <p>
+     * In the mean time, another thread can load and initialize a class, and that initialization
+     * can eventually results in listing up extensions, for example through static initializer.
+     * Such activitiy locks class initialization first, then locks extension list.
+     *
+     * <p>
+     * This inconsistent locking order results in a dead lock, you see.
+     *
+     * <p>
+     * So to reduce the likelihood, this method is called in prior to {@link #find(Class, Hudson)} invocation,
+     * but from outside the lock. The implementation is expected to perform all the class initialization activities
+     * from here.
+     *
+     * <p>
+     * See http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6459208 for how to force a class initialization.
+     */
+    public void scout(Class extensionType, Hudson hudson) {
+    }
+
     /**
      * The default implementation that looks for the {@link Extension} marker.
      *
@@ -132,6 +162,35 @@ public abstract class ExtensionFinder implements ExtensionPoint {
 
             return result;
         }
+
+        @Override
+        public void scout(Class extensionType, Hudson hudson) {
+            ClassLoader cl = hudson.getPluginManager().uberClassLoader;
+            for (IndexItem<Extension,Object> item : Index.load(Extension.class, Object.class, cl)) {
+                try {
+                    AnnotatedElement e = item.element();
+                    Class<?> extType;
+                    if (e instanceof Class) {
+                        extType = (Class) e;
+                    } else
+                    if (e instanceof Field) {
+                        extType = ((Field)e).getType();
+                    } else
+                    if (e instanceof Method) {
+                        extType = ((Method)e).getReturnType();
+                    } else
+                        throw new AssertionError();
+                    // accroding to http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6459208
+                    // this appears to be the only way to force a class initialization
+                    Class.forName(extType.getName(),true,extType.getClassLoader());
+                } catch (InstantiationException e) {
+                    LOGGER.log(item.annotation().optional() ? Level.FINE : Level.WARNING,
+                               "Failed to scout "+item.className(), e);
+                } catch (ClassNotFoundException e) {
+                    LOGGER.log(Level.WARNING,"Failed to scout "+item.className(), e);
+                }
+            }
+        }
     }
     
     private static final Logger LOGGER = Logger.getLogger(ExtensionFinder.class.getName());

core/src/main/java/hudson/ExtensionList.java

@@ -209,7 +209,9 @@ public class ExtensionList<T> extends AbstractList<T> {
         if(Hudson.getInstance().getInitLevel().compareTo(InitMilestone.PLUGINS_PREPARED)<0)
             return legacyInstances; // can't perform the auto discovery until all plugins are loaded, so just make the legacy instances visible
 
-        synchronized (hudson.lookup.setIfNull(Lock.class,new Lock())) {
+        scoutLoad();
+
+        synchronized (getLoadLock()) {
             if(extensions==null) {
                 List<ExtensionComponent<T>> r = load();
                 r.addAll(legacyInstances);
@@ -219,6 +221,13 @@ public class ExtensionList<T> extends AbstractList<T> {
         }
     }
 
+    /**
+     * Chooses the object that locks the loading of the extension instances.
+     */
+    protected Object getLoadLock() {
+        return hudson.lookup.setIfNull(Lock.class,new Lock());
+    }
+
     /**
      * Loading an {@link ExtensionList} can result in a nested loading of another {@link ExtensionList}.
      * What that means is that we need a single lock that spans across all the {@link ExtensionList}s,
@@ -226,6 +235,17 @@ public class ExtensionList<T> extends AbstractList<T> {
      */
     private static final class Lock {}
 
+    /**
+     * See {@link ExtensionFinder#scout(Class, Hudson)} for the dead lock issue and what this does.
+     */
+    protected void scoutLoad() {
+        if (LOGGER.isLoggable(Level.FINER))
+            LOGGER.log(Level.FINER,"Scout-loading ExtensionList: "+extensionType, new Throwable());
+        for (ExtensionFinder finder : finders()) {
+            finder.scout(extensionType, hudson);
+        }
+    }
+
     /**
      * Loads all the extensions.
      */