6809322: javax.management.timer.Timer does not fire all notifications

Summary: Some notifications get dropped due to ConcurrentModificationException thrown in Timer.notifyAlarmClock() method.
Reviewed-by: dholmes, rbackman
Contributed-by: jaroslav.bachorik@oracle.com

src/share/classes/javax/management/timer/Timer.java

@@ -28,8 +28,7 @@ package javax.management.timer;
 import static com.sun.jmx.defaults.JmxProperties.TIMER_LOGGER;
 import java.util.ArrayList;
 import java.util.Date;
-import java.util.Hashtable;
-import java.util.Iterator;
+import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
@@ -128,8 +127,8 @@ public class Timer extends NotificationBroadcasterSupport
      * Table containing all the timer notifications of this timer,
      * with the associated date, period and number of occurrences.
      */
-    private Map<Integer,Object[]> timerTable =
-        new Hashtable<Integer,Object[]>();
+    final private Map<Integer,Object[]> timerTable =
+        new HashMap<>();
 
     /**
      * Past notifications sending on/off flag value.
@@ -163,7 +162,7 @@ public class Timer extends NotificationBroadcasterSupport
      * The notification counter ID.
      * Used to keep the max key value inserted into the timer table.
      */
-    private int counterID = 0;
+    volatile private int counterID = 0;
 
     private java.util.Timer timer;
 
@@ -776,7 +775,7 @@ public class Timer extends NotificationBroadcasterSupport
      *
      * @return The number of timer notifications.
      */
-    public int getNbNotifications() {
+    public synchronized int getNbNotifications() {
         return timerTable.size();
     }
 
@@ -824,7 +823,7 @@ public class Timer extends NotificationBroadcasterSupport
      * @return The timer notification type or null if the identifier is not mapped to any
      * timer notification registered for this timer MBean.
      */
-    public String getNotificationType(Integer id) {
+    public synchronized String getNotificationType(Integer id) {
 
         Object[] obj = timerTable.get(id);
         if (obj != null) {
@@ -841,7 +840,7 @@ public class Timer extends NotificationBroadcasterSupport
      * @return The timer notification detailed message or null if the identifier is not mapped to any
      * timer notification registered for this timer MBean.
      */
-    public String getNotificationMessage(Integer id) {
+    public synchronized String getNotificationMessage(Integer id) {
 
         Object[] obj = timerTable.get(id);
         if (obj != null) {
@@ -862,7 +861,7 @@ public class Timer extends NotificationBroadcasterSupport
     //public Serializable getNotificationUserData(Integer id) {
     // end of NPCTE fix for bugId 4464388
 
-    public Object getNotificationUserData(Integer id) {
+    public synchronized Object getNotificationUserData(Integer id) {
         Object[] obj = timerTable.get(id);
         if (obj != null) {
             return ( ((TimerNotification)obj[TIMER_NOTIF_INDEX]).getUserData() );
@@ -878,7 +877,7 @@ public class Timer extends NotificationBroadcasterSupport
      * @return A copy of the date or null if the identifier is not mapped to any
      * timer notification registered for this timer MBean.
      */
-    public Date getDate(Integer id) {
+    public synchronized Date getDate(Integer id) {
 
         Object[] obj = timerTable.get(id);
         if (obj != null) {
@@ -896,7 +895,7 @@ public class Timer extends NotificationBroadcasterSupport
      * @return A copy of the period or null if the identifier is not mapped to any
      * timer notification registered for this timer MBean.
      */
-    public Long getPeriod(Integer id) {
+    public synchronized Long getPeriod(Integer id) {
 
         Object[] obj = timerTable.get(id);
         if (obj != null) {
@@ -913,7 +912,7 @@ public class Timer extends NotificationBroadcasterSupport
      * @return A copy of the remaining number of occurrences or null if the identifier is not mapped to any
      * timer notification registered for this timer MBean.
      */
-    public Long getNbOccurences(Integer id) {
+    public synchronized Long getNbOccurences(Integer id) {
 
         Object[] obj = timerTable.get(id);
         if (obj != null) {
@@ -931,7 +930,7 @@ public class Timer extends NotificationBroadcasterSupport
      * @return A copy of the flag indicating whether a periodic notification is
      *         executed at <i>fixed-delay</i> or at <i>fixed-rate</i>.
      */
-    public Boolean getFixedRate(Integer id) {
+    public synchronized Boolean getFixedRate(Integer id) {
 
       Object[] obj = timerTable.get(id);
       if (obj != null) {
@@ -982,7 +981,7 @@ public class Timer extends NotificationBroadcasterSupport
      *
      * @return <CODE>true</CODE> if the list of timer notifications is empty, <CODE>false</CODE> otherwise.
      */
-    public boolean isEmpty() {
+    public synchronized boolean isEmpty() {
         return (timerTable.isEmpty());
     }
 
@@ -1184,11 +1183,13 @@ public class Timer extends NotificationBroadcasterSupport
         //
         TimerAlarmClock alarmClock = (TimerAlarmClock)notification.getSource();
 
-        for (Object[] obj : timerTable.values()) {
-            if (obj[ALARM_CLOCK_INDEX] == alarmClock) {
-                timerNotification = (TimerNotification)obj[TIMER_NOTIF_INDEX];
-                timerDate = (Date)obj[TIMER_DATE_INDEX];
-                break;
+        synchronized(Timer.this) {
+            for (Object[] obj : timerTable.values()) {
+                if (obj[ALARM_CLOCK_INDEX] == alarmClock) {
+                    timerNotification = (TimerNotification)obj[TIMER_NOTIF_INDEX];
+                    timerDate = (Date)obj[TIMER_DATE_INDEX];
+                    break;
+                }
             }
         }
 

test/javax/management/timer/MissingNotificationTest.java

+/*
+ * Copyright (c) 2008, 2012, Oracle and/or its affiliates. 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 Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 6809322
+ * @summary Test for missing notifications in a high concurrency environment
+ * @author Jaroslav Bachorik
+ * @run clean MissingNotificationTest
+ * @run build MissingNotificationTest
+ * @run main MissingNotificationTest
+ */
+
+import java.util.Date;
+import java.util.Random;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import javax.management.timer.Timer;
+import javax.management.Notification;
+import javax.management.NotificationListener;
+
+public class MissingNotificationTest {
+    private static int TASK_COUNT = 10000;
+    private static long fixedDelay = 0;// anything bigger than 100 and no alarms remain unfired
+
+    private static class NotifListener implements NotificationListener {
+
+        int count;
+
+        public synchronized void handleNotification(Notification notification, Object handback) {
+            count++;
+        }
+
+        synchronized int getCount() {
+            return count;
+        }
+    }
+
+    public static void main(String[] args) throws Exception {
+        System.out.println(
+            ">>> Test for missing notifications.");
+
+        System.out.println(">>> Create a Timer object.");
+        final Timer timer = new Timer();
+
+        timer.start();
+
+        NotifListener listener = new NotifListener();
+        timer.addNotificationListener(listener, null, null);
+
+        ExecutorService executor = Executors.newFixedThreadPool(100);
+        final Random rand = new Random();
+
+
+        for (int i = 0; i < TASK_COUNT; i++) {
+            executor.execute(new Runnable() {
+                public void run() {
+                    long dateMillis = System.currentTimeMillis() + fixedDelay + rand.nextInt(2000);
+                    Date date = new Date(dateMillis);
+                    timer.addNotification("type", "msg", "userData", date);
+                }
+            });
+
+        }
+
+        executor.shutdown();
+        executor.awaitTermination(20, TimeUnit.SECONDS);
+
+        waitForNotificationsToEnd(listener);
+
+        timer.stop();
+
+        if (listener.count < TASK_COUNT) {
+            throw new RuntimeException("Not fired: " + (TASK_COUNT - listener.count));
+        } else {
+            System.out.println(">>> All notifications handled OK");
+        }
+
+        System.out.println(">>> Bye bye!");
+    }
+
+    /**
+     * Will return when all notifications are handled or after 10sec. of no new
+     * notifications
+     *
+     * @param listener
+     * @throws InterruptedException
+     */
+    private static void waitForNotificationsToEnd(NotifListener listener)
+            throws InterruptedException {
+        int oldCout = listener.getCount();
+        int noChangeCounter = 1;
+        while (listener.getCount() < TASK_COUNT) {
+            Thread.sleep(1000);
+            System.out.print('.');
+            if (oldCout == listener.getCount())//no change
+            {
+                if (++noChangeCounter > 10) {
+                    break;
+                }
+            } else {
+                noChangeCounter = 1;
+            }
+
+            oldCout = listener.getCount();
+        }
+        System.out.println();
+    }
+}