#260 : Slimed down JBoss early detector check a bit & added timeout

  This is also related to #258

agent/core/src/main/java/org/jolokia/detector/AbstractServerDetector.java

@@ -27,7 +27,7 @@
 
 /**
  * Base class for server detectors
- * 
+ *
  * @author roland
  * @since 05.11.10
  */
@@ -164,30 +164,19 @@ public void addMBeanServers(Set<MBeanServerConnection> pServers) {
     }
 
     /**
-     * There is no default early detection mechanism.
-     * 
-     * @return by default there the early detection returns false
-     */
-    public boolean earlyDetect(Instrumentation instrumentation) {
-        return false;
-    }    
-
-    /**
-     * Do nothing by default, leaving the implementation
-     * optional for each specific detector
-     *
-     * @param instrumentation the Instrumentation implementation
+     * By default do nothing during JVM agent startup
      */
-    public void awaitServerInitialization(Instrumentation instrumentation) {
+    public void jvmAgentStartup(Instrumentation instrumentation) {
     }
-
-
+
+
+
     /**
      * Tests if the given class name has been loaded by the JVM. Don't use this method
      * in case you have access to the class loader which will be loading the class
      * because the used approach is not very efficient.
      * @param className the name of the class to check
-     * @param instrumentation 
+     * @param instrumentation
      * @return true if the class has been loaded by the JVM
      * @throws IllegalArgumentException in case instrumentation or the provided class is null
      */
@@ -203,5 +192,5 @@ protected boolean isClassLoaded(String className, Instrumentation instrumentatio
         }
         return false;
     }
-    
+
 }

agent/core/src/main/java/org/jolokia/detector/JBossDetector.java

@@ -36,36 +36,38 @@ public ServerHandle detect(MBeanServerExecutor pMBeanServerExecutor) {
     }
 
     /**
-     * Attempts to return true in case the JVM will start a JBoss modules based application server. Because getting
+     * Attempts to detect a JBoss modules based application server. Because getting
      * access to the main arguments is not possible, it returns true in case the system property
      * {@code jboss.modules.system.pkgs} is set and the {@code org/jboss/modules/Main.class} resource can be found
      * using the class loader of this class.
+     *
+     * If so, it awaits the early initialization of a JBoss modules based application server by polling the system property
+     * {@code java.util.logging.manager} and waiting until the specified class specified by this property has been
+     * loaded by the JVM.
      */
     @Override
-    public boolean earlyDetect(Instrumentation instrumentation) {
-        return earlyDetectForJBossModulesBasedContainer(JBossDetector.class.getClassLoader());
+    public void jvmAgentStartup(Instrumentation instrumentation) {
+        jvmAgentStartup(instrumentation, this.getClass().getClassLoader());
     }
 
-    protected boolean earlyDetectForJBossModulesBasedContainer(ClassLoader classLoader) {
-        URL jbossModulesUrl = classLoader.getResource("org/jboss/modules/Main.class");
-        if (System.getProperty("jboss.modules.system.pkgs") != null && jbossModulesUrl != null) {
-            return true;
+    void jvmAgentStartup(Instrumentation instrumentation, ClassLoader classLoader) {
+        if (earlyDetectForJBossModulesBasedContainer(classLoader)) {
+            awaitServerInitializationForJBossModulesBasedContainer(instrumentation);
         }
-        return false;
     }
 
-    /**
-     * Awaits the early initialization of a JBoss modules based application server by polling the system property
-     * {@code java.util.logging.manager} and waiting until the specified class specified by this property has been
-     * loaded by the JVM.
-     */
-    @Override
-    public void awaitServerInitialization(Instrumentation instrumentation) {
-        awaitServerInitializationForJBossModulesBasedContainer(instrumentation);
+    protected boolean earlyDetectForJBossModulesBasedContainer(ClassLoader classLoader) {
+        return System.getProperty("jboss.modules.system.pkgs") != null &&
+               classLoader.getResource("org/jboss/modules/Main.class") != null;
     }
 
+    // Wait a max 5 Minutes
+    public static final int LOGGING_DETECT_TIMEOUT = 5 * 60 * 1000;
+    public static final int LOGGING_DETECT_INTERVAL = 200;
+
     private void awaitServerInitializationForJBossModulesBasedContainer(Instrumentation instrumentation) {
-        while (true) {
+        int count = 0;
+        while (count * LOGGING_DETECT_INTERVAL < LOGGING_DETECT_TIMEOUT) {
             String loggingManagerClassName = System.getProperty("java.util.logging.manager");
             if (loggingManagerClassName != null) {
                 if (isClassLoaded(loggingManagerClassName, instrumentation)) {
@@ -77,15 +79,17 @@ private void awaitServerInitializationForJBossModulesBasedContainer(Instrumentat
                     // https://github.com/jboss-modules/jboss-modules/blob/1.5.1.Final/src/main/java/org/jboss/modules/Main.java#L482
                     // Therefore the steps 3-6 of the proposal for option 2 don't need to be performed,
                     // see https://github.com/rhuss/jolokia/issues/258 for details.
-                    break;
+                    return;
                 }
             }
             try {
-                Thread.sleep(200);
+                Thread.sleep(LOGGING_DETECT_INTERVAL);
+                count++;
             } catch (InterruptedException e) {
                 throw new RuntimeException(e);
             }
         }
+        throw new IllegalStateException(String.format("Detected JBoss Module loader, but property java.util.logging.manager is not set after %d seconds", LOGGING_DETECT_TIMEOUT / 1000));
     }
 
     private ServerHandle checkFromJSR77(MBeanServerExecutor pMBeanServerExecutor) {

agent/core/src/main/java/org/jolokia/detector/ServerDetector.java

@@ -28,8 +28,8 @@
  * the runtime environment e.g. for the existence of certain classes. If a detector
  * successfully detect 'its' server, it return a {@link ServerHandle} containing type, version
  * and some optional information. For the early detection of a server by the JVM agent,
- * {@link #earlyDetect(Instrumentation)} and {@link #awaitServerInitialization} can  
- * be used. Using these methods is useful in case the server is using its own class 
+ * {@link #jvmAgentStartup(Instrumentation)} and {@link #awaitServerInitialization} can
+ * be used. Using these methods is useful in case the server is using its own class
  * loaders to load components used by Jolokia (e.g jmx, Java logging which is
  * indirectly required by the sun.net.httpserver).
  *
@@ -55,19 +55,10 @@ public interface ServerDetector {
     void addMBeanServers(Set<MBeanServerConnection> pMBeanServers);
 
     /**
-     * Test if it is very likely that the JVM is hosting the particular server. This check is executed 
+     * Notify detector that the JVM is about to start. A detector can, if needed, block and wait for some condition but
+     * should ultimatevely return at some point or throw an exception. This notification is executed
      * in a very early stage (premain of the Jolokia JVM agent) before the main class of the Server is executed.
      * @param instrumentation the Instrumentation implementation
-     * @return true in case the it is very likely that the JVM is running the server
      */
-    boolean earlyDetect(Instrumentation instrumentation);
-
-    /**
-     * Blocks until the server has initialized components which must be available before Jolokia
-     * is running. This operation is normally invoked by the Jolokia JVM agent in a background
-     * thread in case {@link #earlyDetect(Instrumentation)} returns true.
-     * @param instrumentation the Instrumentation implementation
-     */
-    void awaitServerInitialization(Instrumentation instrumentation);
-
+    void jvmAgentStartup(Instrumentation instrumentation);
 }

agent/core/src/test/java/org/jolokia/backend/MBeanServersTest.java

@@ -123,13 +123,9 @@ public void addMBeanServers(Set<MBeanServerConnection> pMBeanServers) {
                 pMBeanServers.add(ownServer);
             }
 
-            public boolean earlyDetect(Instrumentation instrumentation) {
-                return false;
-            }            
-
-            public void awaitServerInitialization(Instrumentation instrumentation) {
+            public void jvmAgentStartup(Instrumentation instrumentation) {
             }
-            
+
         });
         return ret;
     }

agent/core/src/test/java/org/jolokia/backend/TestDetector.java

@@ -88,12 +88,7 @@ public Set<ObjectName> answer() throws Throwable {
         }
     }
 
-    public boolean earlyDetect(Instrumentation instrumentation) {
-        return false;
-    }    
-
-    public void awaitServerInitialization(Instrumentation instrumentation) {
-    }
+    public void jvmAgentStartup(Instrumentation instrumentation) {}
 
     public static void setThrowAddException(boolean b) {
         throwAddException = b;

agent/core/src/test/java/org/jolokia/detector/JBossDetectorTest.java

@@ -174,40 +174,31 @@ public void verifyIsClassLoadedLoaded() {
     }
 
     @Test
-    public void verifyEarlyDetectForJBossModulesBasedContainer() throws MalformedURLException {
+    public void verifyJvmAgentStartup() throws MalformedURLException {
+        Instrumentation inst = createMock(Instrumentation.class);
+        expect(inst.getAllLoadedClasses()).andReturn(new Class[] {}).times(3);
+        expect(inst.getAllLoadedClasses()).andReturn(new Class[] {JBossDetectorTest.class}).atLeastOnce();
         ClassLoader cl = createMock(ClassLoader.class);
         expect(cl.getResource("org/jboss/modules/Main.class")).andReturn(new URL("http", "dummy", "")).once();
-        replay(cl);
-        String prevValue = System.setProperty("jboss.modules.system.pkgs", "blah");
+        String prevPkgValue = System.setProperty("jboss.modules.system.pkgs", "blah");
+        String prevLogValue = System.setProperty("java.util.logging.manager", JBossDetectorTest.class.getName());
+        replay(inst,cl);
+
         try {
-            assertTrue(detector.earlyDetectForJBossModulesBasedContainer(cl));
+            detector.jvmAgentStartup(inst, cl);
         } finally {
-            if (prevValue == null) {
-                System.getProperties().remove("jboss.modules.system.pkgs");
-            } else {
-                System.setProperty("org.jboss.boot.log.file", prevValue);
-            }
+            resetSysProp(prevLogValue, "java.util.logging.manager");
+            resetSysProp(prevPkgValue, "jboss.modules.system.pkgs");
         }
-        verify(cl);
+        verify(inst);
     }
 
-    @Test
-    public void verifyAwaitServerInitialization() throws MalformedURLException {
-        Instrumentation inst = createMock(Instrumentation.class);
-        expect(inst.getAllLoadedClasses()).andReturn(new Class[] {}).times(3);
-        expect(inst.getAllLoadedClasses()).andReturn(new Class[] {JBossDetectorTest.class}).atLeastOnce();
-        replay(inst);
-        String prevValue = System.setProperty("java.util.logging.manager", JBossDetectorTest.class.getName());
-        try {
-            detector.awaitServerInitialization(inst);
-        } finally {
-            if (prevValue == null) {
-                System.getProperties().remove("java.util.logging.manager");
-            } else {
-                System.setProperty("java.util.logging.manager", prevValue);
-            }
+    protected void resetSysProp(String prevValue, String key) {
+        if (prevValue == null) {
+            System.getProperties().remove(key);
+        } else {
+            System.setProperty(key, prevValue);
         }
-        verify(inst);
     }
 
 }

agent/jvm/src/main/java/org/jolokia/jvmagent/JvmAgent.java

@@ -113,22 +113,15 @@ public void run() {
     }
 
     /**
-     * Lookup the server detectors and await the server initialization.
+     * Lookup the server detectors and notify detector about the JVM startup
      *
      * @param instrumentation
-     * @see ServerDetector#earlyDetect(Instrumentation)
-     * @see ServerDetector#awaitServerInitialization(Instrumentation)
+     * @see ServerDetector#jvmAgentStartup(Instrumentation)
      */
     private static void awaitServerInitialization(JvmAgentConfig pConfig, final Instrumentation instrumentation) {
         List<ServerDetector> detectors = MBeanServerHandler.lookupDetectors();
-        for (ServerDetector detector:detectors) {
-            // TODO add feature to JvmAgentConfig to be able to skip earlyDetect & awaitServerInitialization
-            // for a detector.
-            if (detector.earlyDetect(instrumentation)) {
-                System.out.format("Jolokia: await server initialization using %s%n", detector.getClass().getName());
-                detector.awaitServerInitialization(instrumentation);
-                System.out.format("Jolokia: await server initialization done%n");
-            }
+        for (ServerDetector detector : detectors) {
+            detector.jvmAgentStartup(instrumentation);
         }
     };
 

agent/osgi/src/test/java/org/jolokia/osgi/servlet/JolokiaServletTest.java

@@ -145,13 +145,7 @@ public ServerHandle detect(MBeanServerExecutor pMBeanServerExecutor) {
         public void addMBeanServers(Set<MBeanServerConnection> pMBeanServers) {
         }
 
-        public boolean earlyDetect(Instrumentation instrumentation) {
-            return false;
-        }
-
-        public void awaitServerInitialization(Instrumentation instrumentation) {
-        }
-
+        public void jvmAgentStartup(Instrumentation instrumentation) {}
     }
 
 }