7170447: Intermittent DeadListenerTest.java failure

Summary: Due to asynchronous nature of processing server notifications it may happen that an "unregister" notification ha$
Reviewed-by: sjiang

test/javax/management/remote/mandatory/notif/DeadListenerTest.java

@@ -31,6 +31,7 @@
 import com.sun.jmx.remote.internal.ServerNotifForwarder;
 import java.io.IOException;
 import java.lang.management.ManagementFactory;
+import java.lang.ref.WeakReference;
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
@@ -86,20 +87,26 @@ public class DeadListenerTest {
         Map<ObjectName, Set<?>> listenerMap = (Map<ObjectName, Set<?>>) listenerMapF.get(serverNotifForwarder);
         assertTrue("Server listenerMap initially empty", mapWithoutKey(listenerMap, delegateName).isEmpty());
 
-        CountListener count1 = new CountListener();
+        final AtomicInteger count1Val = new AtomicInteger();
+        CountListener count1 = new CountListener(count1Val);
         mbsc.addNotificationListener(name, count1, null, null);
+        WeakReference<CountListener> count1Ref = new WeakReference<>(count1);
+        count1 = null;
 
-        CountListener count2 = new CountListener();
+        final AtomicInteger count2Val = new AtomicInteger();
+        CountListener count2 = new CountListener(count2Val);
         NotificationFilterSupport dummyFilter = new NotificationFilterSupport();
         dummyFilter.enableType("");
         mbsc.addNotificationListener(name, count2, dummyFilter, "noddy");
+        WeakReference<CountListener> count2Ref = new WeakReference<>(count2);
+        count2 = null;
 
         assertTrue("One entry in listenerMap for two listeners on same MBean", mapWithoutKey(listenerMap, delegateName).size() == 1);
         Set<?> set = listenerMap.get(name);
         assertTrue("Set in listenerMap for MBean has two elements", set != null && set.size() == 2);
 
-        assertTrue("Initial value of count1 == 0", count1.count() == 0);
-        assertTrue("Initial value of count2 == 0", count2.count() == 0);
+        assertTrue("Initial value of count1 == 0", count1Val.get() == 0);
+        assertTrue("Initial value of count2 == 0", count2Val.get() == 0);
 
         Notification notif = new Notification("type", name, 0);
 
@@ -107,11 +114,11 @@ public class DeadListenerTest {
 
         // Make sure notifs are working normally.
         long deadline = System.currentTimeMillis() + 2000;
-        while ((count1.count() != 1 || count2.count() != 1) && System.currentTimeMillis() < deadline) {
+        while ((count1Val.get() != 1 || count2Val.get() != 1) && System.currentTimeMillis() < deadline) {
             Thread.sleep(10);
         }
-        assertTrue("New value of count1 == 1", count1.count() == 1);
-        assertTrue("Initial value of count2 == 1", count2.count() == 1);
+        assertTrue("New value of count1 == 1", count1Val.get() == 1);
+        assertTrue("Initial value of count2 == 1", count2Val.get() == 1);
 
         // Make sure that removing a nonexistent listener from an existent MBean produces ListenerNotFoundException
         CountListener count3 = new CountListener();
@@ -136,28 +143,29 @@ public class DeadListenerTest {
         mbs.unregisterMBean(name);
         mbean.sendNotification(notif);
         Thread.sleep(200);
-        assertTrue("New value of count1 == 1", count1.count() == 1);
-        assertTrue("Initial value of count2 == 1", count2.count() == 1);
+
+        assertTrue("New value of count1 == 1", count1Val.get() == 1);
+        assertTrue("Initial value of count2 == 1", count2Val.get() == 1);
+
+        // wait for the listener cleanup to take place upon processing notifications
+        int countdown = 50; // waiting max. 5 secs
+        while (countdown-- > 0 &&
+                (count1Ref.get() != null ||
+                 count2Ref.get() != null)) {
+            System.gc();
+            Thread.sleep(100);
+            System.gc();
+        }
+        // listener has been removed or the wait has timed out
+
+        assertTrue("count1 notification listener has not been cleaned up", count1Ref.get() == null);
+        assertTrue("count2 notification listener has not been cleaned up", count2Ref.get() == null);
 
         // Check that there is no trace of the listeners any more in ServerNotifForwarder.listenerMap.
         // THIS DEPENDS ON JMX IMPLEMENTATION DETAILS.
         // If the JMX implementation changes, the code here may have to change too.
         Set<?> setForUnreg = listenerMap.get(name);
         assertTrue("No trace of unregistered MBean: " + setForUnreg, setForUnreg == null);
-
-        // Remove attempts should fail.
-        try {
-            mbsc.removeNotificationListener(name, count1);
-            assertTrue("Remove of count1 listener should have failed", false);
-        } catch (ListenerNotFoundException e) {
-            // OK: expected
-        }
-        try {
-            mbsc.removeNotificationListener(name, count2, dummyFilter, "noddy");
-            assertTrue("Remove of count2 listener should have failed", false);
-        } catch (ListenerNotFoundException e) {
-            // OK: expected
-        }
     }
 
     private static <K, V> Map<K, V> mapWithoutKey(Map<K, V> map, K key) {
@@ -173,6 +181,10 @@ public class DeadListenerTest {
     public static class CountListener implements NotificationListener {
         final AtomicInteger count;
 
+        public CountListener(AtomicInteger i) {
+            count = i;
+        }
+
         public CountListener() {
             this.count = new AtomicInteger();
         }