[JENKINS-60449] Fix and edge case of OptionalExtension blowing up Jenkins

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

@@ -36,6 +36,8 @@
 import com.google.inject.Provider;
 import com.google.inject.Scope;
 import com.google.inject.Scopes;
+import com.google.inject.TypeLiteral;
+import com.google.inject.internal.Errors;
 import com.google.inject.matcher.Matchers;
 import com.google.inject.name.Names;
 import com.google.inject.spi.ProvisionListener;
@@ -484,24 +486,27 @@ public SezpozModule(List<IndexItem<?,Object>> index) {
              */
             private void resolve(Class c) {
                 try {
-                    c.getGenericSuperclass();
-                    c.getGenericInterfaces();
                     ClassLoader ecl = c.getClassLoader();
-                    Method m = ClassLoader.class.getDeclaredMethod("resolveClass", Class.class);
-                    m.setAccessible(true);
-                    m.invoke(ecl, c);
-                    c.getConstructors();
-                    c.getMethods();
-                    for (Field f : c.getFields()) {
-                        if (f.getAnnotation(javax.inject.Inject.class) != null || f.getAnnotation(com.google.inject.Inject.class) != null) {
-                            resolve(f.getType());
+                    if (ecl != null) { // Not bootstrap classloader
+                        Method m = ClassLoader.class.getDeclaredMethod("resolveClass", Class.class);
+                        m.setAccessible(true);
+                        m.invoke(ecl, c);
+                    }
+                    for (Class cc = c; cc != Object.class; cc = cc.getSuperclass()) {
+                        /**
+                         * See {@link com.google.inject.spi.InjectionPoint#getInjectionPoints(TypeLiteral, boolean, Errors)}
+                         */
+                        cc.getGenericSuperclass();
+                        cc.getGenericInterfaces();
+                        cc.getDeclaredConstructors();
+                        cc.getDeclaredMethods();
+                        for (Field f : cc.getDeclaredFields()) {
+                            if (f.getAnnotation(javax.inject.Inject.class) != null || f.getAnnotation(com.google.inject.Inject.class) != null) {
+                                resolve(f.getType());
+                            }
                         }
                     }
                     LOGGER.log(Level.FINER, "{0} looks OK", c);
-                    while (c != Object.class) {
-                        c.getGenericSuperclass();
-                        c = c.getSuperclass();
-                    }
                 } catch (Exception x) {
                     throw (LinkageError)new LinkageError("Failed to resolve "+c).initCause(x);
                 }

core/src/main/java/hudson/PluginManager.java

@@ -949,23 +949,6 @@ public void dynamicLoad(File arc, boolean removeExisting, @CheckForNull List<Plu
     @Restricted(NoExternalUse.class)
     public void start(List<PluginWrapper> plugins) throws Exception {
       try (ACLContext context = ACL.as(ACL.SYSTEM)) {
-        Jenkins.get().refreshExtensions();
-
-        for (PluginWrapper p : plugins) {
-            p.getPlugin().postInitialize();
-        }
-
-        // run initializers in the added plugins
-        Reactor r = new Reactor(InitMilestone.ordering());
-        Set<ClassLoader> loaders = plugins.stream().map(p -> p.classLoader).collect(Collectors.toSet());
-        r.addAll(new InitializerFinder(uberClassLoader) {
-            @Override
-            protected boolean filter(Method e) {
-                return !loaders.contains(e.getDeclaringClass().getClassLoader()) || super.filter(e);
-            }
-        }.discoverTasks(r));
-        new InitReactorRunner().run(r);
-
         Map<String, PluginWrapper> pluginsByName = plugins.stream().collect(Collectors.toMap(p -> p.getShortName(), p -> p));
 
         // recalculate dependencies of plugins optionally depending the newly deployed ones.
@@ -993,6 +976,20 @@ protected boolean filter(Method e) {
         } catch (ExtensionRefreshException e) {
             throw new IOException("Failed to refresh extensions after installing some plugins", e);
         }
+        for (PluginWrapper p : plugins) {
+          p.getPlugin().postInitialize();
+        }
+
+        // run initializers in the added plugins
+        Reactor r = new Reactor(InitMilestone.ordering());
+        Set<ClassLoader> loaders = plugins.stream().map(p -> p.classLoader).collect(Collectors.toSet());
+        r.addAll(new InitializerFinder(uberClassLoader) {
+          @Override
+          protected boolean filter(Method e) {
+            return !loaders.contains(e.getDeclaringClass().getClassLoader()) || super.filter(e);
+          }
+        }.discoverTasks(r));
+        new InitReactorRunner().run(r);
       }
     }
 

test/src/test/java/hudson/PluginManagerTest.java

@@ -333,6 +333,11 @@ private String callDependerValue() throws Exception {
             fail();
         } catch( ClassNotFoundException ex ){
         }
+        // Extension extending a dependee class can't be loaded either
+        try {
+            r.jenkins.getExtensionList("org.jenkinsci.plugins.dependencytest.depender.DependerExtension");
+            fail();
+        } catch( NoClassDefFoundError ex ){}
 
         // Load dependee.
         {
@@ -342,10 +347,33 @@ private String callDependerValue() throws Exception {
         // (MUST) Not throws an exception
         // (SHOULD) depender successfully accesses to dependee.
         assertEquals("dependee", callDependerValue());
-
-        // No extensions exist.
-        // extensions in depender are loaded.
-        assertFalse(r.jenkins.getExtensionList("org.jenkinsci.plugins.dependencytest.dependee.DependeeExtensionPoint").isEmpty());
+
+        // Extensions in depender are loaded.
+        assertEquals(1, r.jenkins.getExtensionList("org.jenkinsci.plugins.dependencytest.depender.DependerExtension").size());
+    }
+
+    /**
+     * Load "optional-depender" and then load "dependee".
+     * Asserts that "depender" can access to "dependee".
+     *
+     * @throws Exception
+     */
+    @Issue("JENKINS-60449")
+    @WithPlugin("variant.hpi")
+    @Test public void installDependedOptionalPluginWithoutRestart() throws Exception {
+        // Load optional-depender.
+        {
+            dynamicLoad("optional-depender-0.0.2.hpi");
+        }
+        // Extension depending on dependee class isn't loaded
+        assertTrue(r.jenkins.getExtensionList("org.jenkinsci.plugins.dependencytest.optionaldepender.OptionalDependerExtension").isEmpty());
+        // Load dependee.
+        {
+            dynamicLoad("dependee-0.0.2.hpi");
+        }
+
+        // Extensions in depender are loaded.
+        assertEquals(1, r.jenkins.getExtensionList("org.jenkinsci.plugins.dependencytest.optionaldepender.OptionalDependerExtension").size());
     }
 
     @Issue("JENKINS-21486")