6736611: [Evt Srv] EventSubscriber.unsubscribe removes other listeners

Reviewed-by: emcmanus

src/share/classes/javax/management/event/EventSubscriber.java

@@ -149,10 +149,10 @@ public class EventSubscriber implements EventConsumer {
         if (listener == null)
             throw new IllegalArgumentException("Null listener");
 
-        final ListenerInfo li = new ListenerInfo(listener, filter, handback);
-        List<ListenerInfo> list;
+        final MyListenerInfo li = new MyListenerInfo(listener, filter, handback);
+        List<MyListenerInfo> list;
 
-        Map<ObjectName, List<ListenerInfo>> map;
+        Map<ObjectName, List<MyListenerInfo>> map;
         Set<ObjectName> names;
         if (name.isPattern()) {
             map = patternSubscriptionMap;
@@ -165,7 +165,7 @@ public class EventSubscriber implements EventConsumer {
         synchronized (map) {
             list = map.get(name);
             if (list == null) {
-                list = new ArrayList<ListenerInfo>();
+                list = new ArrayList<MyListenerInfo>();
                 map.put(name, list);
             }
             list.add(li);
@@ -186,7 +186,6 @@ public class EventSubscriber implements EventConsumer {
     public void unsubscribe(ObjectName name,
             NotificationListener listener)
             throws ListenerNotFoundException, IOException {
-
         if (logger.traceOn())
             logger.trace("unsubscribe", "" + name);
 
@@ -196,7 +195,7 @@ public class EventSubscriber implements EventConsumer {
         if (listener == null)
             throw new ListenerNotFoundException();
 
-        Map<ObjectName, List<ListenerInfo>> map;
+        Map<ObjectName, List<MyListenerInfo>> map;
         Set<ObjectName> names;
 
         if (name.isPattern()) {
@@ -207,25 +206,42 @@ public class EventSubscriber implements EventConsumer {
             names = Collections.singleton(name);
         }
 
-        final ListenerInfo li = new ListenerInfo(listener, null, null);
-        List<ListenerInfo> list;
+        List<MyListenerInfo> toRemove = new ArrayList<MyListenerInfo>();
         synchronized (map) {
-            list = map.get(name);
-            if (list == null || !list.remove(li))
+            List<MyListenerInfo> list = map.get(name);
+            if (list == null) {
                 throw new ListenerNotFoundException();
+            }
+
+            for (MyListenerInfo info : list) {
+                if (info.listener == listener) {
+                    toRemove.add(info);
+                }
+            }
+
+            if (toRemove.isEmpty()) {
+                throw new ListenerNotFoundException();
+            }
+
+            for (MyListenerInfo info : toRemove) {
+                list.remove(info);
+            }
 
             if (list.isEmpty())
                 map.remove(name);
         }
 
         for (ObjectName mbeanName : names) {
+            for (MyListenerInfo i : toRemove) {
                 try {
-                mbeanServer.removeNotificationListener(mbeanName, li.listener);
+                    mbeanServer.removeNotificationListener(mbeanName,
+                        i.listener, i.filter, i.handback);
                 } catch (Exception e) {
                     logger.fine("unsubscribe", "removeNotificationListener", e);
                 }
             }
         }
+    }
 
     // ---------------------------------
     // private stuff
@@ -256,12 +272,12 @@ public class EventSubscriber implements EventConsumer {
                 return;
             }
 
-            final List<ListenerInfo> listeners = new ArrayList<ListenerInfo>();
+            final List<MyListenerInfo> listeners = new ArrayList<MyListenerInfo>();
 
             // If there are subscribers for the exact name that has just arrived
             // then add their listeners to the list.
             synchronized (exactSubscriptionMap) {
-                List<ListenerInfo> exactListeners = exactSubscriptionMap.get(name);
+                List<MyListenerInfo> exactListeners = exactSubscriptionMap.get(name);
                 if (exactListeners != null)
                     listeners.addAll(exactListeners);
             }
@@ -277,7 +293,7 @@ public class EventSubscriber implements EventConsumer {
             }
 
             // Add all the listeners just found to the new MBean.
-            for (ListenerInfo li : listeners) {
+            for (MyListenerInfo li : listeners) {
                 try {
                     mbeanServer.addNotificationListener(
                             name,
@@ -292,12 +308,12 @@ public class EventSubscriber implements EventConsumer {
         }
     };
 
-    private static class ListenerInfo {
+    private static class MyListenerInfo {
         public final NotificationListener listener;
         public final NotificationFilter filter;
         public final Object handback;
 
-        public ListenerInfo(NotificationListener listener,
+        public MyListenerInfo(NotificationListener listener,
                 NotificationFilter filter,
                 Object handback) {
 
@@ -308,26 +324,6 @@ public class EventSubscriber implements EventConsumer {
             this.filter = filter;
             this.handback = handback;
         }
-
-        /* Two ListenerInfo instances are equal if they have the same
-         * NotificationListener.  This means that we can use List.remove
-         * to implement the two-argument removeNotificationListener.
-         */
-        @Override
-        public boolean equals(Object o) {
-            if (o == this)
-                return true;
-
-            if (!(o instanceof ListenerInfo))
-                return false;
-
-            return listener.equals(((ListenerInfo)o).listener);
-        }
-
-        @Override
-        public int hashCode() {
-            return listener.hashCode();
-        }
     }
 
     // ---------------------------------
@@ -338,10 +334,10 @@ public class EventSubscriber implements EventConsumer {
     // ---------------------------------
     private final MBeanServer mbeanServer;
 
-    private final Map<ObjectName, List<ListenerInfo>> exactSubscriptionMap =
-            new HashMap<ObjectName, List<ListenerInfo>>();
-    private final Map<ObjectName, List<ListenerInfo>> patternSubscriptionMap =
-            new HashMap<ObjectName, List<ListenerInfo>>();
+    private final Map<ObjectName, List<MyListenerInfo>> exactSubscriptionMap =
+            new HashMap<ObjectName, List<MyListenerInfo>>();
+    private final Map<ObjectName, List<MyListenerInfo>> patternSubscriptionMap =
+            new HashMap<ObjectName, List<MyListenerInfo>>();
 
 
 

test/javax/management/eventService/SubUnsubTest.java

+/*
+ * Copyright 2008 Sun Microsystems, Inc.  All Rights Reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara,
+ * CA 95054 USA or visit www.sun.com if you need additional information or
+ * have any questions.
+ */
+
+/*
+ * @test SubUnsubTest
+ * @bug 6736611
+ * @summary Test not to remove other listeners when calling unsubscribe
+ * @author Shanliang JIANG
+ * @run clean SubUnsubTest
+ * @run build SubUnsubTest
+ * @run main SubUnsubTest
+ */
+
+import java.lang.management.ManagementFactory;
+import javax.management.MBeanServer;
+import javax.management.Notification;
+import javax.management.NotificationFilter;
+import javax.management.NotificationBroadcasterSupport;
+import javax.management.NotificationListener;
+import javax.management.ObjectName;
+import javax.management.event.EventSubscriber;
+import javax.management.event.EventClient;
+public class SubUnsubTest {
+    private static class CountListener implements NotificationListener {
+        volatile int count;
+
+        public void handleNotification(Notification n, Object h) {
+            count++;
+        }
+    }
+
+    public static interface SenderMBean {}
+
+    public static class Sender extends NotificationBroadcasterSupport
+            implements SenderMBean {
+        void send() {
+            Notification n = new Notification("type", this, 1L);
+            sendNotification(n);
+        }
+    }
+
+    public static void main(String[] args) throws Exception {
+        System.out.println("Testing EventSubscriber-unsubscribe method.");
+
+        MBeanServer mbs = ManagementFactory.getPlatformMBeanServer();
+        ObjectName name1 = new ObjectName("d:type=Sender,id=1");
+        ObjectName name2 = new ObjectName("d:type=Sender,id=2");
+        ObjectName pattern = new ObjectName("d:type=Sender,*");
+        Sender sender1 = new Sender();
+        Sender sender2 = new Sender();
+        mbs.registerMBean(sender1, name1);
+        mbs.registerMBean(sender2, name2);
+
+        EventSubscriber sub = EventSubscriber.getEventSubscriber(mbs);
+
+        System.out.println("Single subscribe covering both MBeans");
+        CountListener listener = new CountListener();
+
+        System.out.println("Subscribing and adding listeners ...");
+        sub.subscribe(pattern, listener, null, null);
+        sub.subscribe(name2, listener, null, null);
+        mbs.addNotificationListener(name2, listener, null, null);
+
+        sender1.send();
+        sender2.send();
+        if (listener.count != 4) {
+            throw new RuntimeException("Do not receive all notifications: "+
+                    "Expect 4, got "+listener.count);
+        }
+
+        System.out.println("Unsubscribe the listener with the pattern.");
+        sub.unsubscribe(pattern, listener);
+        listener.count = 0;
+        sender1.send();
+        sender2.send();
+        if (listener.count != 2) {
+            throw new RuntimeException("The method unsubscribe removes wrong listeners.");
+        }
+
+        System.out.println("Unsubscribe the listener with the ObjectName.");
+        sub.unsubscribe(name2, listener);
+        listener.count = 0;
+        sender1.send();
+        sender2.send();
+        if (listener.count != 1) {
+            throw new RuntimeException("The method unsubscribe removes wrong listeners.");
+        }
+
+        System.out.println("Subscribe twice to same MBean with same listener " +
+                "but different handback.");
+        sub.subscribe(name1, listener, null, new Object());
+        sub.subscribe(name1, listener, null, new Object());
+        listener.count = 0;
+
+        sub.unsubscribe(name1, listener);
+        sender1.send();
+        if (listener.count > 0) {
+            throw new RuntimeException("EventSubscriber: the method unsubscribe" +
+                    " does not remove a listener which was subscribed 2 times.");
+        }
+
+        System.out.println("Bye bye!");
+        return;
+    }
+}
\ No newline at end of file