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
--- a/jdk/src/share/classes/javax/management/timer/Timer.java Wed Oct 17 13:35:22 2012 +0100
+++ b/jdk/src/share/classes/javax/management/timer/Timer.java Wed Oct 17 18:34:07 2012 +0400
@@ -28,8 +28,7 @@
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 @@
* 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 @@
* 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 @@
*
* @return The number of timer notifications.
*/
- public int getNbNotifications() {
+ public synchronized int getNbNotifications() {
return timerTable.size();
}
@@ -824,7 +823,7 @@
* @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 @@
* @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 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 @@
* @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 @@
* @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 @@
* @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 @@
* @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 @@
*
* @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 @@
//
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;
+ }
}
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/javax/management/timer/MissingNotificationTest.java Wed Oct 17 18:34:07 2012 +0400
@@ -0,0 +1,130 @@
+/*
+ * 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();
+ }
+}