6776225: JMX.isNotificationSource wrong when DynamicWrapperMBean + SendNotification injection
Reviewed-by: dfuchs, jfdenise
--- 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<M> {
+public abstract class MBeanIntrospector<M> {
static final class PerInterfaceMap<M>
extends WeakHashMap<Class<?>, WeakReference<PerInterface<M>>> {}
@@ -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)
--- 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;
}
/**
--- 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> 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 {
--- 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 <T> 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.
--- 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) {