# HG changeset patch # User emcmanus # Date 1227797072 -3600 # Node ID eb801ce73ac9ddcfdb864f9268e108805ea937d2 # Parent 8ca7ecc0226d09bee9548105b40e6e242caf2dae 6776225: JMX.isNotificationSource wrong when DynamicWrapperMBean + SendNotification injection Reviewed-by: dfuchs, jfdenise diff -r 8ca7ecc0226d -r eb801ce73ac9 jdk/src/share/classes/com/sun/jmx/mbeanserver/MBeanIntrospector.java --- a/jdk/src/share/classes/com/sun/jmx/mbeanserver/MBeanIntrospector.java Wed Nov 26 15:37:14 2008 +0000 +++ b/jdk/src/share/classes/com/sun/jmx/mbeanserver/MBeanIntrospector.java Thu Nov 27 15:44:32 2008 +0100 @@ -80,7 +80,7 @@ * ancestor with ConvertingMethod. But that would mean an extra object * for every Method in every Standard MBean interface. */ -abstract class MBeanIntrospector { +public abstract class MBeanIntrospector { static final class PerInterfaceMap extends WeakHashMap, WeakReference>> {} @@ -557,7 +557,7 @@ return findNotificationsFromAnnotations(moi.getClass()); } - private static MBeanNotificationInfo[] findNotificationsFromAnnotations( + public static MBeanNotificationInfo[] findNotificationsFromAnnotations( Class mbeanClass) { Class c = getAnnotatedNotificationInfoClass(mbeanClass); if (c == null) diff -r 8ca7ecc0226d -r eb801ce73ac9 jdk/src/share/classes/javax/management/JMX.java --- a/jdk/src/share/classes/javax/management/JMX.java Wed Nov 26 15:37:14 2008 +0000 +++ b/jdk/src/share/classes/javax/management/JMX.java Thu Nov 27 15:44:32 2008 +0100 @@ -51,8 +51,6 @@ * this class. */ static final JMX proof = new JMX(); - private static final ClassLogger logger = - new ClassLogger("javax.management.misc", "JMX"); private JMX() {} @@ -824,11 +822,16 @@ */ public static boolean isNotificationSource(Object mbean) throws NotCompliantMBeanException { - if (mbean instanceof NotificationBroadcaster) - return true; - Object resource = (mbean instanceof DynamicWrapperMBean) ? - ((DynamicWrapperMBean) mbean).getWrappedObject() : mbean; - return (MBeanInjector.injectsSendNotification(resource)); + for (int i = 0; i < 2; i++) { + if (mbean instanceof NotificationBroadcaster || + MBeanInjector.injectsSendNotification(mbean)) + return true; + if (mbean instanceof DynamicWrapperMBean) + mbean = ((DynamicWrapperMBean) mbean).getWrappedObject(); + else + break; + } + return false; } /** diff -r 8ca7ecc0226d -r eb801ce73ac9 jdk/src/share/classes/javax/management/StandardEmitterMBean.java --- a/jdk/src/share/classes/javax/management/StandardEmitterMBean.java Wed Nov 26 15:37:14 2008 +0000 +++ b/jdk/src/share/classes/javax/management/StandardEmitterMBean.java Thu Nov 27 15:44:32 2008 +0100 @@ -26,6 +26,7 @@ package javax.management; import com.sun.jmx.mbeanserver.MBeanInjector; +import com.sun.jmx.mbeanserver.MBeanIntrospector; import static javax.management.JMX.MBeanOptions; /** @@ -195,10 +196,12 @@ MBeanOptions options, NotificationEmitter emitter) { super(implementation, mbeanInterface, options); + MBeanNotificationInfo[] defaultMBNIs = defaultMBNIs(implementation); if (emitter == null) - emitter = defaultEmitter(); + emitter = defaultEmitter(defaultMBNIs); this.emitter = emitter; - this.notificationInfo = emitter.getNotificationInfo(); + this.notificationInfo = + firstNonEmpty(emitter.getNotificationInfo(), defaultMBNIs); injectEmitter(); } @@ -320,15 +323,23 @@ protected StandardEmitterMBean(Class mbeanInterface, MBeanOptions options, NotificationEmitter emitter) { super(mbeanInterface, options); + MBeanNotificationInfo[] defaultMBNIs = defaultMBNIs(this); if (emitter == null) - emitter = defaultEmitter(); + emitter = defaultEmitter(defaultMBNIs); this.emitter = emitter; - this.notificationInfo = emitter.getNotificationInfo(); + this.notificationInfo = + firstNonEmpty(emitter.getNotificationInfo(), defaultMBNIs); injectEmitter(); } - private NotificationEmitter defaultEmitter() { - MBeanNotificationInfo[] mbnis = getNotificationInfo(); + private static MBeanNotificationInfo[] defaultMBNIs(Object mbean) { + return MBeanIntrospector.findNotificationsFromAnnotations( + mbean.getClass()); + } + + private NotificationEmitter defaultEmitter(MBeanNotificationInfo[] defaultMBNIs) { + MBeanNotificationInfo[] mbnis = + firstNonEmpty(getNotificationInfo(), defaultMBNIs); // Will be null unless getNotificationInfo() is overridden, // since the notificationInfo field has not been set at this point. if (mbnis == null) @@ -336,6 +347,14 @@ return new NotificationBroadcasterSupport(mbnis); } + private static T[] firstNonEmpty(T[]... items) { + for (T[] t : items) { + if (t != null && t.length != 0) + return t; + } + return null; + } + private void injectEmitter() { if (emitter instanceof SendNotification) { try { diff -r 8ca7ecc0226d -r eb801ce73ac9 jdk/src/share/classes/javax/management/StandardMBean.java --- a/jdk/src/share/classes/javax/management/StandardMBean.java Wed Nov 26 15:37:14 2008 +0000 +++ b/jdk/src/share/classes/javax/management/StandardMBean.java Thu Nov 27 15:44:32 2008 +0100 @@ -1058,10 +1058,6 @@ cachedMBeanInfo = info; } - private boolean isMXBean() { - return mbean.isMXBean(); - } - private static boolean identicalArrays(T[] a, T[] b) { if (a == b) return true; @@ -1466,7 +1462,7 @@ // Check for "MBeanNotificationInfo[] getNotificationInfo()" // method. // - // This method is only taken into account for the MBeanInfo + // This method is taken into account for the MBeanInfo // immutability checks if and only if the given subclass is // StandardEmitterMBean itself or can be assigned to // StandardEmitterMBean. diff -r 8ca7ecc0226d -r eb801ce73ac9 jdk/test/javax/management/MBeanServer/DynamicWrapperMBeanTest.java --- a/jdk/test/javax/management/MBeanServer/DynamicWrapperMBeanTest.java Wed Nov 26 15:37:14 2008 +0000 +++ b/jdk/test/javax/management/MBeanServer/DynamicWrapperMBeanTest.java Thu Nov 27 15:44:32 2008 +0100 @@ -23,14 +23,27 @@ /* * @test DynamicWrapperMBeanTest - * @bug 6624232 + * @bug 6624232 6776225 * @summary Test the DynamicWrapperMBean interface * @author Eamonn McManus */ import java.lang.management.ManagementFactory; +import javax.annotation.Resource; +import javax.management.JMX; +import javax.management.ListenerNotFoundException; +import javax.management.MBeanNotificationInfo; import javax.management.MBeanServer; +import javax.management.Notification; +import javax.management.NotificationBroadcaster; +import javax.management.NotificationBroadcasterSupport; +import javax.management.NotificationEmitter; +import javax.management.NotificationFilter; +import javax.management.NotificationInfo; +import javax.management.NotificationListener; import javax.management.ObjectName; +import javax.management.SendNotification; +import javax.management.StandardEmitterMBean; import javax.management.StandardMBean; import javax.management.modelmbean.ModelMBeanInfo; import javax.management.modelmbean.ModelMBeanInfoSupport; @@ -39,6 +52,25 @@ import static javax.management.StandardMBean.Options; public class DynamicWrapperMBeanTest { + private static String failure; + + public static void main(String[] args) throws Exception { + wrapTest(); + notifTest(); + + if (failure == null) + System.out.println("TEST PASSED"); + else + throw new Exception("TEST FAILED: " + failure); + } + + private static final Options wrappedVisOpts = new Options(); + private static final Options wrappedInvisOpts = new Options(); + static { + wrappedVisOpts.setWrappedObjectVisible(true); + wrappedInvisOpts.setWrappedObjectVisible(false); + } + public static interface WrappedMBean { public void sayHello(); } @@ -48,19 +80,13 @@ } } - private static String failure; - - public static void main(String[] args) throws Exception { + private static void wrapTest() throws Exception { if (Wrapped.class.getClassLoader() == StandardMBean.class.getClassLoader()) { throw new Exception( "TEST ERROR: Resource and StandardMBean have same ClassLoader"); } - Options wrappedVisOpts = new Options(); - wrappedVisOpts.setWrappedObjectVisible(true); - Options wrappedInvisOpts = new Options(); - wrappedInvisOpts.setWrappedObjectVisible(false); assertEquals("Options withWrappedObjectVisible(false)", new Options(), wrappedInvisOpts); @@ -138,8 +164,223 @@ assertEquals("isInstanceOf(RequiredModelMBean) for invisible resource", true, mbs.isInstanceOf(invisibleName, RequiredModelMBean.class.getName())); - if (failure != null) - throw new Exception("TEST FAILED: " + failure); + mbs.unregisterMBean(visibleName); + mbs.unregisterMBean(invisibleName); + } + + private static enum WrapType { + NBS("NotificationBroadcasterSupport"), + INJ("@Resource SendNotification"), + STD_MBEAN_NBS("StandardMBean delegating to NotificationBroadcasterSupport"), + STD_MBEAN_INJ("StandardMBean delegating to @Resource SendNotification"), + STD_MBEAN_SUB_NBS("StandardMBean subclass implementing NotificationBroadcaster"), + STD_MBEAN_SUB_INJ("StandardMBean subclass with @Resource SendNotification"), + STD_EMIT_MBEAN_NBS("StandardEmitterMBean delegating to NotificationBroadcasterSupport"), + STD_EMIT_MBEAN_INJ("StandardEmitterMBean delegating to @Resource SendNotification"), + STD_EMIT_MBEAN_SUB("StandardEmitterMBean subclass"), + STD_EMIT_MBEAN_SUB_INJ("StandardEmitterMBean subclass with @Resource SendNotification"); + + WrapType(String s) { + this.s = s; + } + + @Override + public String toString() { + return s; + } + + private final String s; + } + + @NotificationInfo( + types = {"foo", "bar"} + ) + public static interface BroadcasterMBean { + public void send(Notification n); + } + + public static class Broadcaster + extends NotificationBroadcasterSupport implements BroadcasterMBean { + public void send(Notification n) { + super.sendNotification(n); + } + } + + public static interface SendNotifMBean extends BroadcasterMBean { + } + + public static class SendNotif implements SendNotifMBean { + @Resource + private volatile SendNotification sendNotif; + + public void send(Notification n) { + sendNotif.sendNotification(n); + } + } + + public static class StdBroadcaster + extends StandardMBean + implements BroadcasterMBean, NotificationBroadcaster { + private final NotificationBroadcasterSupport nbs = + new NotificationBroadcasterSupport(); + + public StdBroadcaster() throws Exception { + super(BroadcasterMBean.class); + } + + public void send(Notification n) { + nbs.sendNotification(n); + } + + public void addNotificationListener(NotificationListener listener, + NotificationFilter filter, Object handback) { + nbs.addNotificationListener(listener, filter, handback); + } + + public MBeanNotificationInfo[] getNotificationInfo() { + return null; + } + + public void removeNotificationListener(NotificationListener listener) + throws ListenerNotFoundException { + nbs.removeNotificationListener(listener); + } + } + + public static class StdSendNotif + extends StandardMBean implements SendNotifMBean { + @Resource + private volatile SendNotification sendNotif; + + public StdSendNotif() throws Exception { + super(SendNotifMBean.class); + } + + public void send(Notification n) { + sendNotif.sendNotification(n); + } + } + + public static class StdEmitterBroadcaster // :-) + extends StandardEmitterMBean + implements BroadcasterMBean { + + public StdEmitterBroadcaster() throws Exception { + super(BroadcasterMBean.class, null); + } + + public void send(Notification n) { + super.sendNotification(n); + } + } + + // This case is unlikely - if you're using @Resource SendNotification + // then there's no point in using StandardEmitterMBean, since + // StandardMBean would suffice. + public static class StdEmitterSendNotif + extends StandardEmitterMBean implements SendNotifMBean { + @Resource + private volatile SendNotification sendNotif; + + public StdEmitterSendNotif() { + super(SendNotifMBean.class, null); + } + + public void send(Notification n) { + sendNotif.sendNotification(n); + } + } + + // Test that JMX.isNotificationSource and + // mbs.isInstanceOf("NotificationBroadcaster") work correctly even when + // the MBean is a broadcaster by virtue of its wrapped resource. + // Test that we find the MBeanNotificationInfo[] from the @NotificationInfo + // annotation on BroadcasterMBean. We cover a large number of different + // MBean types, but all ultimately implement that interface. + private static void notifTest() throws Exception { + System.out.println("===Testing notification senders==="); + + for (WrapType wrapType : WrapType.values()) { + System.out.println("---" + wrapType); + + final Object mbean; + + switch (wrapType) { + case NBS: + // An MBean that extends NotificationBroadcasterSupport + mbean = new Broadcaster(); + break; + case INJ: + // An MBean that injects SendNotification + mbean = new SendNotif(); + break; + case STD_MBEAN_NBS: + // A StandardMBean that delegates to a NotificationBroadcasterSupport + mbean = new StandardMBean( + new Broadcaster(), BroadcasterMBean.class, wrappedVisOpts); + break; + case STD_MBEAN_INJ: + // A StandardMBean that delegates to an object that injects + // SendNotification + mbean = new StandardMBean( + new SendNotif(), BroadcasterMBean.class, wrappedVisOpts); + break; + case STD_EMIT_MBEAN_NBS: { + // A StandardEmitterMBean that delegates to a NotificationBroadcasterSupport + Broadcaster broadcaster = new Broadcaster(); + mbean = new StandardEmitterMBean( + broadcaster, BroadcasterMBean.class, wrappedVisOpts, + broadcaster); + break; + } + case STD_EMIT_MBEAN_INJ: { + // A StandardEmitterMBean that delegates to an object that injects + // SendNotification + SendNotif sendNotif = new SendNotif(); + mbean = new StandardEmitterMBean( + sendNotif, BroadcasterMBean.class, wrappedVisOpts, + null); + break; + } + case STD_MBEAN_SUB_NBS: + // A subclass of StandardMBean that implements NotificationBroadcaster + mbean = new StdBroadcaster(); + break; + case STD_MBEAN_SUB_INJ: + // A subclass of StandardMBean that injects SendNotification + mbean = new StdSendNotif(); + break; + case STD_EMIT_MBEAN_SUB: + // A subclass of StandardEmitterMBean + mbean = new StdEmitterBroadcaster(); + break; + case STD_EMIT_MBEAN_SUB_INJ: + // A subclass of StandardEmitterMBean that injects SendNotification + // (which is a rather strange thing to do and probably a user + // misunderstanding but we should do the right thing anyway). + mbean = new StdEmitterSendNotif(); + break; + default: + throw new AssertionError(); + } + + MBeanServer mbs = ManagementFactory.getPlatformMBeanServer(); + + final ObjectName name = new ObjectName("a:type=Sender"); + mbs.registerMBean(mbean, name); + boolean isBroadcaster = mbs.isInstanceOf( + name, NotificationBroadcaster.class.getName()); + assertEquals("JMX.isNotificationSource(mbean)", + true, JMX.isNotificationSource(mbean)); + assertEquals("isInstanceOf(NotificationBroadcaster)", + true, isBroadcaster); + MBeanNotificationInfo[] mbnis = + mbs.getMBeanInfo(name).getNotifications(); + assertEquals("MBeanNotificationInfo not empty", + true, (mbnis.length > 0)); + + mbs.unregisterMBean(name); + } } private static void assertEquals(String what, Object expect, Object actual) {