User: ejort
Date: 01/12/17 00:19:07
Modified: src/main/javax/management/timer Timer.java TimerMBean.java
Log:
Reimplement the Timer Service to use one thread per notification.
Fixed the synchronization, still some checks required there.
Fixed a spelling mistake. occurances -> occurences
Added a timer test class.
Revision Changes Path
1.2 +186 -156 jmx/src/main/javax/management/timer/Timer.java
Index: Timer.java
===================================================================
RCS file: /cvsroot/jboss/jmx/src/main/javax/management/timer/Timer.java,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -r1.1 -r1.2
--- Timer.java 2001/12/16 19:38:39 1.1
+++ Timer.java 2001/12/17 08:19:07 1.2
@@ -19,7 +19,7 @@
import javax.management.NotificationBroadcasterSupport;
import javax.management.ObjectName;
-// TODO Fix the synchronization
+// REVIEW: Check synchronization
/**
* The timer service.
@@ -54,7 +54,22 @@
* The number of milliseconds in one week.
*/
public static final long ONE_WEEK = ONE_DAY * 7;
-
+
+ /**
+ * Don't send notifications at initial start up.
+ */
+ private static final int SEND_NO = 0;
+
+ /**
+ * Send all past notifications at initial start up.
+ */
+ private static final int SEND_START = 1;
+
+ /**
+ * Normal operation sending
+ */
+ private static final int SEND_NORMAL = 2;
+
// Attributes ----------------------------------------------------
/**
@@ -68,11 +83,6 @@
long sequenceNumber = 0;
/**
- * The date next run of the notifications, null when nothing to run.
- */
- long nextRunDate = 0;
-
- /**
* The send past events attribute.
*/
boolean sendPastNotifications = false;
@@ -83,15 +93,10 @@
boolean active = false;
/**
- * The notification thread.
- */
- Thread notificationThread = null;
-
- /**
* The registered notifications.
*/
Hashtable notifications = new Hashtable();
-
+
// Static --------------------------------------------------------
// Constructors --------------------------------------------------
@@ -114,8 +119,9 @@
return addNotification(type, message, userData, date, period, 0);
}
- public Integer addNotification(String type, String message, Object userData,
- Date date, long period, long occurances)
+ // REVIEW: Synchronized because of active/thread start
+ public synchronized Integer addNotification(String type, String message,
+ Object userData, Date date, long period, long occurences)
throws IllegalArgumentException
{
// Generate the next id.
@@ -127,16 +133,16 @@
Integer id = new Integer(newId);
// Validate and create the registration.
- RegisteredNotification rn =
+ RegisteredNotification rn =
new RegisteredNotification(id, type, message, userData, date, period,
- occurances);
+ occurences);
// Add the registration.
notifications.put(id, rn);
- // Wake the background thread up
- if (notificationThread != null)
- notificationThread.interrupt();
+ // If we are active, start the thread
+ if (active)
+ rn.start();
return id;
}
@@ -149,7 +155,7 @@
public Date getDate(Integer id)
{
// Make sure there is a registration
- RegisteredNotification rn = (RegisteredNotification) notifications.get(id);
+ RegisteredNotification rn = (RegisteredNotification)notifications.get(id);
if (rn == null)
return null;
@@ -162,15 +168,15 @@
return notifications.size();
}
- public Long getNbOccurances(Integer id)
+ public Long getNbOccurences(Integer id)
{
// Make sure there is a registration
- RegisteredNotification rn = (RegisteredNotification) notifications.get(id);
+ RegisteredNotification rn = (RegisteredNotification)notifications.get(id);
if (rn == null)
return null;
- // Return a copy of the occurances.
- return new Long(rn.occurances);
+ // Return a copy of the occurences.
+ return new Long(rn.occurences);
}
public Vector getNotificationIDs(String type)
@@ -191,7 +197,7 @@
public String getNotificationMessage(Integer id)
{
// Make sure there is a registration
- RegisteredNotification rn = (RegisteredNotification) notifications.get(id);
+ RegisteredNotification rn = (RegisteredNotification)notifications.get(id);
if (rn == null)
return null;
@@ -202,7 +208,7 @@
public String getNotificationType(Integer id)
{
// Make sure there is a registration
- RegisteredNotification rn = (RegisteredNotification) notifications.get(id);
+ RegisteredNotification rn = (RegisteredNotification)notifications.get(id);
if (rn == null)
return null;
@@ -213,7 +219,7 @@
public Object getNotificationUserData(Integer id)
{
// Make sure there is a registration
- RegisteredNotification rn = (RegisteredNotification) notifications.get(id);
+ RegisteredNotification rn = (RegisteredNotification)notifications.get(id);
if (rn == null)
return null;
@@ -224,7 +230,7 @@
public Long getPeriod(Integer id)
{
// Make sure there is a registration
- RegisteredNotification rn = (RegisteredNotification) notifications.get(id);
+ RegisteredNotification rn = (RegisteredNotification)notifications.get(id);
if (rn == null)
return null;
@@ -247,24 +253,48 @@
return notifications.isEmpty();
}
- public void removeAllNotifications()
+ // REVIEW: Synchronized because of active/thread stop and enumeration
+ public synchronized void removeAllNotifications()
{
- notifications.clear();
+ RegisteredNotification rn = null;
+
+ // Loop through the notifications removing everything.
+ for (Enumeration e = notifications.elements(); e.hasMoreElements();)
+ {
+ rn = (RegisteredNotification) e.nextElement();
+ notifications.remove(rn.id);
+
+ // Stop the thread
+ rn.nextDate = 0;
+ if (active)
+ rn.interrupt();
+ }
+
+ // The spec says to reset the identifiers, seems like a bad idea to me
+ nextId = 0;
}
- public void removeNotification(Integer id)
+ // REVIEW: Synchronized because of active/thread stop and enumeration
+ public synchronized void removeNotification(Integer id)
throws InstanceNotFoundException
{
// Check if there is a notification.
- RegisteredNotification rn = (RegisteredNotification) notifications.get(id);
+ RegisteredNotification rn = (RegisteredNotification)notifications.get(id);
if (rn == null)
- throw new InstanceNotFoundException("No notification id : " + id.toString());
+ throw new InstanceNotFoundException("No notification id : " +
+ id.toString());
// Remove the notification
notifications.remove(id);
+
+ // Stop the thread
+ rn.nextDate = 0;
+ if (active)
+ rn.interrupt();
}
- public void removeNotifications(String type)
+ // REVIEW: Synchronized because of active/thread stop and enumeration
+ public synchronized void removeNotifications(String type)
throws InstanceNotFoundException
{
RegisteredNotification rn = null;
@@ -278,12 +308,18 @@
{
notifications.remove(rn.id);
found = true;
+
+ // Stop the thread
+ rn.nextDate = 0;
+ if (active)
+ rn.interrupt();
}
}
// The spec says to through an error when nothing removed.
if (found == false)
- throw new InstanceNotFoundException("Nothing registered for type: " + type);
+ throw new InstanceNotFoundException("Nothing registered for type: " +
+ type);
}
public void setSendPastNotifications(boolean value)
@@ -291,31 +327,39 @@
sendPastNotifications = value;
}
- public void start()
+ // REVIEW: Synchronized because of active/thread stop and enumeration
+ public synchronized void start()
{
// Ignore if already active
if (active)
return;
active = true;
-
- // Do the send past notification processing
- sendNotifications(sendPastNotifications);
- // Start the thread
- notificationThread = new Thread(new PollingThread());
- notificationThread.setDaemon(true);
- notificationThread.start();
+ // Start the threads for each notification
+ for (Enumeration e = notifications.elements(); e.hasMoreElements();)
+ {
+ ((Thread) e.nextElement()).start();
+ }
}
- public void stop()
+ // REVIEW: Synchronized because of active/thread stop and enumeration
+ public synchronized void stop()
{
- // Stop the thread
+ // Ignore if not active
+ if (!active)
+ return;
+
+ // Stop the threads
active = false;
- if (notificationThread != null)
- notificationThread.interrupt();
+
+ // Ask each thread to stop
+ for (Enumeration e = notifications.elements(); e.hasMoreElements();)
+ {
+ ((Thread) e.nextElement()).interrupt();
+ }
}
- // NotificationBroadcasterSupport overrides ----------------------
+ // MBeanRegistrationImplementation overrides ---------------------
public ObjectName preRegister(MBeanServer server, ObjectName objectName)
throws Exception
@@ -346,83 +390,61 @@
// Private -------------------------------------------------------
/**
- * Recalculate the next run.
- */
- private void reschedule()
- {
- // No need to reschedule when not active
- if (!isActive())
- return;
-
- RegisteredNotification rn = null;
- long calcNextRunDate = 0;
-
- // Loop through the registered notifications to find the next run.
- for (Enumeration e = notifications.elements(); e.hasMoreElements();)
- {
- rn = (RegisteredNotification) e.nextElement();
- if (calcNextRunDate == 0 || rn.nextDate < calcNextRunDate)
- calcNextRunDate = rn.nextDate;
- }
-
- // Tell the thread about any changed next run date
- if (nextRunDate != calcNextRunDate)
- {
- nextRunDate = calcNextRunDate;
- }
- }
-
- /**
* Send any outstanding notifications.
*
+ * @param rn the registered notification to send.
* @param doSend whether the send operations should be performed.
- * Pass true to perform the sends.
- * @return true when a notification was sent, false otherwise.
- */
- private void sendNotifications(boolean doSend)
- {
- RegisteredNotification rn = null;
-
- // Keep going until we have done any outstanding notifications
- boolean notificationSent = false;
- do
+ * Pass SEND_NO for startup and not sending past notification,
+ * pass SEND_START for startup and sending past notifications,
+ * pass SEND_NORMAL for normal operation.
+ */
+ private void sendNotifications(RegisteredNotification rn, int doSend)
+ {
+ // Keep going until we have done all outstanding notifications.
+ // The loop ends when not active, or there are no outstanding
+ // notifications.
+ // REVIEW: In practice for normal operation it never loops. We
+ // ignore sends that we have missed. This avoids problems where
+ // the notification takes longer than the period. Correct???
+ while (isActive() && rn.nextDate != 0
+ && rn.nextDate <= System.currentTimeMillis())
{
- notificationSent = false;
-
- // Loop through the registered notifications.
- for (Enumeration e = notifications.elements(); e.hasMoreElements();)
+ // Do we actually send it?
+ // Yes, unless start and not sending past notifications.
+ if (doSend != SEND_NO)
{
- rn = (RegisteredNotification) e.nextElement();
-
- // Notification required
- if (rn.nextDate <= System.currentTimeMillis())
+ long seq = 0;
+ synchronized (this)
{
- long seq = 0;
- synchronized (this)
- {
- seq = ++sequenceNumber;
- }
- sendNotification(new TimerNotification(rn.type, this, seq,
- System.currentTimeMillis(), rn.message, rn.id));
- notificationSent = true;
-
- // If no next run, remove it
- if (!rn.calcNextDate())
- notifications.remove(rn.id);
+ seq = ++sequenceNumber;
}
+ sendNotification(new TimerNotification(rn.type, this, seq,
+ System.currentTimeMillis(), rn.message, rn.id));
}
- } while (notificationSent == true);
- // Reschedule the next run
- reschedule();
+ // Calculate the next date.
+ // Except for we are sending past notifications at start up,
+ // it cannot be in the future.
+ do
+ {
+ // If no next run, remove it sets the next date to zero.
+ if (!rn.calcNextDate())
+ {
+ notifications.remove(rn.id);
+ }
+ }
+ while (isActive() && doSend != SEND_START && rn.nextDate != 0
+ && rn.nextDate < System.currentTimeMillis());
+ }
}
// Inner classes -------------------------------------------------
/**
- * A registered notification.
+ * A registered notification. These run as separate threads.
*/
private class RegisteredNotification
+ extends Thread
{
// Attributes ----------------------------------------------------
@@ -457,15 +479,15 @@
public long period;
/**
- * The maximum number of occurances.
+ * The maximum number of occurences.
*/
- public long occurances;
+ public long occurences;
/**
* The next notification date.
*/
public long nextDate;
-
+
// Constructors --------------------------------------------------
/**
@@ -478,16 +500,16 @@
* @param date the date/time the notification will occur.
* @param period the repeat period in milli-seconds. Passing zero means
* no repeat.
- * @param occurances the maximum number of repeats. When the period is not
+ * @param occurences the maximum number of repeats. When the period is not
* zero and this parameter is zero, it will repeat indefinitely.
* @return the notification id for this notification.
* @exception IllegalArgumentException when the date is before the current
* date, the period is negative or the number of repeats is
* negative.
- */
- public RegisteredNotification(Integer id, String type, String message,
+ */
+ public RegisteredNotification(Integer id, String type, String message,
Object userData, Date startDate,
- long period, long occurances)
+ long period, long occurences)
throws IllegalArgumentException
{
// Basic validation
@@ -495,8 +517,8 @@
throw new IllegalArgumentException("Null Date");
if (period < 0)
throw new IllegalArgumentException("Negative Period");
- if (occurances < 0)
- throw new IllegalArgumentException("Negative Occurances");
+ if (occurences < 0)
+ throw new IllegalArgumentException("Negative Occurences");
// Remember the values
this.id = id;
@@ -505,16 +527,17 @@
this.userData = userData;
this.startDate = startDate.getTime();
this.period = period;
- this.occurances = occurances;
+ this.occurences = occurences;
// Can we get a next date in the future
nextDate = this.startDate;
while (nextDate < System.currentTimeMillis())
{
- // If we have no more occurances its an error
+ // If we have no more occurences its an error
if (!calcNextDate())
throw new IllegalArgumentException("Requested notification(s) " +
"in the past.");
+
}
}
@@ -522,67 +545,74 @@
/**
* Calculate the next notification date. Add on the period until
- * the number of occurances is exhausted.
+ * the number of occurences is exhausted.
*
- * @return false when there are no more occurances, true otherwise.
+ * @return false when there are no more occurences, true otherwise.
*/
synchronized boolean calcNextDate()
{
// No period, we've finished
if (period == 0)
+ {
+ nextDate = 0;
return false;
+ }
// Limited number of repeats have we finished?
- if (occurances !=0 && --occurances == 0)
+ if (occurences != 0 && --occurences == 0)
+ {
+ nextDate = 0;
return false;
+ }
- // Calculate the next occurance
+ // Calculate the next occurence
nextDate += period;
return true;
}
- }
- /**
- * The notification thread polling.
- */
- private class PollingThread
- implements Runnable
- {
- // Attributes ----------------------------------------------------
-
- // Constructors --------------------------------------------------
+ // Thread overrides ----------------------------------------------
- // Public --------------------------------------------------------
-
/**
* Wait until the next run date or until we are interrupted by
- * a reschedule. When active, check the registered notifications
- * to see whether notifications should be sent.
+ * a remove or stop. When active, send any outstanding notifications.
+ *
*/
public void run()
{
+ // Work out the startup send mode
+ int doSend = SEND_NO;
+ if (sendPastNotifications)
+ doSend = SEND_START;
+
// Keep going until we are stopped
- while (isActive())
+ while (isActive() && nextDate != 0)
{
// Send any notifications
- sendNotifications(true);
+ sendNotifications(this, doSend);
- // Wait until the next run or we are interrupted
- try
- {
- if (nextRunDate == 0)
- Thread.currentThread().sleep(30*ONE_MINUTE);
- else
- {
- long waitMillis = nextRunDate - System.currentTimeMillis();
- if (waitMillis > 0)
- Thread.currentThread().sleep(waitMillis);
- }
- }
- catch (InterruptedException ignored)
- {
- }
+ // We have done the initial send, switch to normal mode
+ doSend = SEND_NORMAL;
+
+ // Wait until we the next notification
+ doWait();
+ }
+ }
+
+ /**
+ * Wait until the next run date or until we are interrupted by
+ * remove or stop.
+ */
+ private synchronized void doWait()
+ {
+ try
+ {
+ long waitMillis = nextDate - System.currentTimeMillis();
+ if (waitMillis > 0)
+ wait(waitMillis);
+ }
+ catch (InterruptedException ignored)
+ {
}
}
}
1.2 +7 -7 jmx/src/main/javax/management/timer/TimerMBean.java
Index: TimerMBean.java
===================================================================
RCS file: /cvsroot/jboss/jmx/src/main/javax/management/timer/TimerMBean.java,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -r1.1 -r1.2
--- TimerMBean.java 2001/12/16 19:38:39 1.1
+++ TimerMBean.java 2001/12/17 08:19:07 1.2
@@ -15,7 +15,7 @@
* The timer service MBean interface. <p>
*
* @author <a href="mailto:[EMAIL PROTECTED]";>Adrian Brock</a>
- * @version $Revision: 1.1 $
+ * @version $Revision: 1.2 $
*
*/
@@ -68,11 +68,11 @@
/**
* Creates a new timer notification for a specific date/time, with an
- * optional repeat period and a maximum number of occurances.<p>
+ * optional repeat period and a maximum number of occurences.<p>
*
* If the date and time is before the the current date and time the period
* is repeatedly added until a date after the current date and time is
- * found. If the number of occurances is exceeded before the
+ * found. If the number of occurences is exceeded before the
* current date and time is reached, an IllegalArgumentException is raised.
*
* @param type the notification type.
@@ -81,7 +81,7 @@
* @param date the date/time the notification will occur.
* @param period the repeat period in milli-seconds. Passing zero means
* no repeat.
- * @param occurances the maximum number of repeats. When the period is not
+ * @param occurences the maximum number of repeats. When the period is not
* zero and this parameter is zero, it will repeat indefinitely.
* @return the notification id for this notification.
* @exception IllegalArgumentException when the date is before the current
@@ -89,7 +89,7 @@
* negative.
*/
public Integer addNotification(String type, String message, Object userData,
- Date date, long period, long occurances)
+ Date date, long period, long occurences)
throws IllegalArgumentException;
/**
@@ -117,14 +117,14 @@
public int getNbNotifications();
/**
- * Retrieves a copy of the maximum notification occurances for a passed
+ * Retrieves a copy of the maximum notification occurences for a passed
* notification id.
*
* @param id the notification id.
- * @return a copy of the maximum notification occurances or null when the
+ * @return a copy of the maximum notification occurences or null when the
* notification id is not registered.
*/
- public Long getNbOccurances(Integer id);
+ public Long getNbOccurences(Integer id);
/**
* Retrieves all timer notifications ids of the passed notification type.
_______________________________________________
Jboss-development mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/jboss-development
