6766173: Spec should say that createMBean wraps a constructor RuntimeException...

6766173: Spec should say that createMBean wraps a constructor RuntimeException in a RuntimeMBeanException
Summary: JMX spec clarification
Reviewed-by: dfuchs

src/share/classes/javax/management/MBeanServerConnection.java

@@ -76,7 +76,9 @@ public interface MBeanServerConnection extends NotificationManager {
      * <CODE>preRegister</CODE> (<CODE>MBeanRegistration</CODE>
      * interface) method of the MBean has thrown an exception. The
      * MBean will not be registered.
-     * @exception RuntimeMBeanException If the <CODE>postRegister</CODE>
+     * @exception RuntimeMBeanException If the MBean's constructor or its
+     * {@code preRegister} or {@code postRegister} method threw
+     * a {@code RuntimeException}. If the <CODE>postRegister</CODE>
      * (<CODE>MBeanRegistration</CODE> interface) method of the MBean throws a
      * <CODE>RuntimeException</CODE>, the <CODE>createMBean</CODE> method will
      * throw a <CODE>RuntimeMBeanException</CODE>, although the MBean creation
@@ -148,7 +150,9 @@ public interface MBeanServerConnection extends NotificationManager {
      * <CODE>preRegister</CODE> (<CODE>MBeanRegistration</CODE>
      * interface) method of the MBean has thrown an exception. The
      * MBean will not be registered.
-     * @exception RuntimeMBeanException If the <CODE>postRegister</CODE>
+     * @exception RuntimeMBeanException If the MBean's constructor or its
+     * {@code preRegister} or {@code postRegister} method threw
+     * a {@code RuntimeException}. If the <CODE>postRegister</CODE>
      * (<CODE>MBeanRegistration</CODE> interface) method of the MBean throws a
      * <CODE>RuntimeException</CODE>, the <CODE>createMBean</CODE> method will
      * throw a <CODE>RuntimeMBeanException</CODE>, although the MBean creation
@@ -223,7 +227,9 @@ public interface MBeanServerConnection extends NotificationManager {
      * <CODE>preRegister</CODE> (<CODE>MBeanRegistration</CODE>
      * interface) method of the MBean has thrown an exception. The
      * MBean will not be registered.
-     * @exception RuntimeMBeanException If the <CODE>postRegister</CODE>
+     * @exception RuntimeMBeanException If the MBean's constructor or its
+     * {@code preRegister} or {@code postRegister} method threw
+     * a {@code RuntimeException}. If the <CODE>postRegister</CODE>
      * (<CODE>MBeanRegistration</CODE> interface) method of the MBean throws a
      * <CODE>RuntimeException</CODE>, the <CODE>createMBean</CODE> method will
      * throw a <CODE>RuntimeMBeanException</CODE>, although the MBean creation
@@ -295,7 +301,9 @@ public interface MBeanServerConnection extends NotificationManager {
      * <CODE>preRegister</CODE> (<CODE>MBeanRegistration</CODE>
      * interface) method of the MBean has thrown an exception. The
      * MBean will not be registered.
-     * @exception RuntimeMBeanException If the <CODE>postRegister</CODE>
+     * @exception RuntimeMBeanException The MBean's constructor or its
+     * {@code preRegister} or {@code postRegister} method threw
+     * a {@code RuntimeException}. If the <CODE>postRegister</CODE>
      * (<CODE>MBeanRegistration</CODE> interface) method of the MBean throws a
      * <CODE>RuntimeException</CODE>, the <CODE>createMBean</CODE> method will
      * throw a <CODE>RuntimeMBeanException</CODE>, although the MBean creation

src/share/classes/javax/management/namespace/MBeanServerSupport.java

@@ -457,7 +457,11 @@ public abstract class MBeanServerSupport implements MBeanServer {
      * All the various flavors of {@code MBeanServer.createMBean} methods
      * will eventually call this method. A subclass that wishes to
      * support MBean creation through {@code createMBean} thus only
-     * needs to provide an implementation for this one method.
+     * needs to provide an implementation for this one method.</p>
+     *
+     * <p>A subclass implementation of this method should respect the contract
+     * of the various {@code createMBean} methods in the {@link MBeanServer}
+     * interface, in particular as regards exception wrapping.</p>
      *
      * @param className The class name of the MBean to be instantiated.
      * @param name The object name of the MBean. May be null.
@@ -488,6 +492,17 @@ public abstract class MBeanServerSupport implements MBeanServer {
      * <CODE>preRegister</CODE> (<CODE>MBeanRegistration</CODE>
      * interface) method of the MBean has thrown an exception. The
      * MBean will not be registered.
+     * @exception RuntimeMBeanException If the MBean's constructor or its
+     * {@code preRegister} or {@code postRegister} method threw
+     * a {@code RuntimeException}. If the <CODE>postRegister</CODE>
+     * (<CODE>MBeanRegistration</CODE> interface) method of the MBean throws a
+     * <CODE>RuntimeException</CODE>, the <CODE>createMBean</CODE> method will
+     * throw a <CODE>RuntimeMBeanException</CODE>, although the MBean creation
+     * and registration succeeded. In such a case, the MBean will be actually
+     * registered even though the <CODE>createMBean</CODE> method
+     * threw an exception. Note that <CODE>RuntimeMBeanException</CODE> can
+     * also be thrown by <CODE>preRegister</CODE>, in which case the MBean
+     * will not be registered.
      * @exception MBeanException The constructor of the MBean has
      * thrown an exception
      * @exception NotCompliantMBeanException This class is not a JMX
@@ -1096,7 +1111,7 @@ public abstract class MBeanServerSupport implements MBeanServer {
             MBeanRegistrationException, MBeanException,
             NotCompliantMBeanException {
         try {
-            return safeCreateMBean(className, name, null, params, signature, true);
+            return createMBean(className, name, null, params, signature, true);
         } catch (InstanceNotFoundException ex) {
             // should not happen!
             throw new MBeanException(ex, "Unexpected exception: " + ex);
@@ -1113,7 +1128,7 @@ public abstract class MBeanServerSupport implements MBeanServer {
             throws ReflectionException, InstanceAlreadyExistsException,
             MBeanRegistrationException, MBeanException,
             NotCompliantMBeanException, InstanceNotFoundException {
-        return safeCreateMBean(className, name, loaderName, params, signature, false);
+        return createMBean(className, name, loaderName, params, signature, false);
     }
 
     /**
@@ -1126,7 +1141,7 @@ public abstract class MBeanServerSupport implements MBeanServer {
             MBeanRegistrationException, MBeanException,
             NotCompliantMBeanException {
         try {
-            return safeCreateMBean(className, name, null, null, null, true);
+            return createMBean(className, name, null, null, null, true);
         } catch (InstanceNotFoundException ex) {
             // should not happen!
             throw new MBeanException(ex, "Unexpected exception: " + ex);
@@ -1143,32 +1158,7 @@ public abstract class MBeanServerSupport implements MBeanServer {
             throws ReflectionException, InstanceAlreadyExistsException,
             MBeanRegistrationException, MBeanException,
             NotCompliantMBeanException, InstanceNotFoundException {
-        return safeCreateMBean(className, name, loaderName, null, null, false);
-    }
-
-    // make sure all exceptions are correctly wrapped in a JMXException
-    private ObjectInstance safeCreateMBean(String className,
-            ObjectName name, ObjectName loaderName, Object[] params,
-            String[] signature, boolean useRepository)
-            throws ReflectionException, InstanceAlreadyExistsException,
-            MBeanRegistrationException, MBeanException,
-            NotCompliantMBeanException, InstanceNotFoundException {
-        try {
-            return createMBean(className, name, loaderName, params,
-                               signature, useRepository);
-        } catch (ReflectionException x) { throw x;
-        } catch (InstanceAlreadyExistsException x) { throw x;
-        } catch (MBeanRegistrationException x) { throw x;
-        } catch (MBeanException x) { throw x;
-        } catch (NotCompliantMBeanException x) { throw x;
-        } catch (InstanceNotFoundException x) { throw x;
-        } catch (SecurityException x) { throw x;
-        } catch (JMRuntimeException x) { throw x;
-        } catch (RuntimeException x) {
-            throw new RuntimeOperationsException(x, x.toString());
-        } catch (Exception x) {
-            throw new MBeanException(x, x.toString());
-        }
+        return createMBean(className, name, loaderName, null, null, false);
     }
 
 

test/javax/management/MBeanServer/MBeanExceptionTest.java

@@ -23,16 +23,19 @@
 
 /*
  * @test
- * @bug 5035217
+ * @bug 5035217 6766173
  * @summary Test that MBean's RuntimeException is wrapped in
  * RuntimeMBeanException and (for Standard MBeans) that checked exceptions
  * are wrapped in MBeanException
  * @author Eamonn McManus
- * @compile -source 1.4 MBeanExceptionTest.java
+ * @compile MBeanExceptionTest.java
  * @run main MBeanExceptionTest
  */
 
+import java.util.Collections;
+import java.util.Set;
 import javax.management.*;
+import javax.management.namespace.MBeanServerSupport;
 
 public class MBeanExceptionTest {
     public static void main(String[] args) throws Exception {
@@ -56,6 +59,53 @@ public class MBeanExceptionTest {
         failures += test(mbs, standardName, true);
         failures += test(mbs, standardMBeanName, true);
         failures += test(mbs, dynamicName, false);
+
+        final boolean[] booleans = {false, true};
+
+        for (boolean mbss : booleans) {
+            for (boolean runtimeX : booleans) {
+                Class<? extends Exception> excC =
+                        runtimeX ? RuntimeMBeanException.class : MBeanException.class;
+                String excS =
+                        runtimeX ? "a RuntimeMBeanException" : "an MBeanException";
+                String mbsS =
+                        mbss ? "a conformant MBeanServerSupport" : "a plain MBeanServer";
+                MBeanServer xmbs =
+                        mbss ? new CreateExceptionMBS() : mbs;
+                System.out.println(
+                        "Test that, with " + mbsS + ", " + excS + " is wrapped " +
+                        "in " + excS);
+                // E.g. "Test that, with a plain MBeanServer, an MBeanException
+                // is wrapped in an MBeanException".
+                try {
+                    mbs.createMBean(
+                            Except.class.getName(), new ObjectName(":name=Oops"),
+                            new Object[] {runtimeX},
+                            new String[] {boolean.class.getName()});
+                    System.out.println(
+                            "FAIL: createMBean succeeded but should not have");
+                    failures++;
+                } catch (Exception e) {
+                    if (!excC.isInstance(e)) {
+                        System.out.println(
+                                "FAIL: expected " + excC.getName() + " from " +
+                                "createMBean, got " + e);
+                        failures++;
+                    } else {
+                        Throwable cause = e.getCause();
+                        if (!excC.isInstance(cause)) {
+                            System.out.println(
+                                    "FAIL: expected " + excC.getName() +
+                                    " as cause of " + excC.getName() +
+                                    ", got " + e);
+                            failures++;
+                        } else
+                            System.out.println("...ok");
+                    }
+                }
+            }
+        }
+
         if (failures == 0)
             System.out.println("Test passed");
         else {
@@ -153,6 +203,15 @@ public class MBeanExceptionTest {
     }
 
     public static class Except implements ExceptMBean {
+        public Except() {}
+
+        public Except(boolean runtimeX) throws MBeanException {
+            if (runtimeX)
+                throw new RuntimeMBeanException(new RuntimeException(), "Bang");
+            else
+                throw new MBeanException(new Exception(), "Bang");
+        }
+
         public String getUncheckedException() {
             throw theUncheckedException;
         }
@@ -221,4 +280,28 @@ public class MBeanExceptionTest {
     private static final RuntimeException theUncheckedException =
         new UnsupportedOperationException("The unchecked exception " +
                                           "that should be seen");
+
+    private static class CreateExceptionMBS extends MBeanServerSupport {
+        @Override
+        protected Set<ObjectName> getNames() {
+            return Collections.emptySet();
+        }
+
+        @Override
+        public DynamicMBean getDynamicMBeanFor(ObjectName name)
+                throws InstanceNotFoundException {
+            throw new InstanceNotFoundException(name);
+        }
+
+        @Override
+        public ObjectInstance createMBean(String className,
+                ObjectName name, ObjectName loaderName, Object[] params,
+                String[] signature, boolean useCLR)
+                throws ReflectionException, InstanceAlreadyExistsException,
+                MBeanRegistrationException, MBeanException,
+                NotCompliantMBeanException, InstanceNotFoundException {
+            Exception wrapped = new MBeanException(new Exception(), "Bang");
+            throw new MBeanException(wrapped, "Bang");
+        }
+    }
 }