Hi all,

I introduced a regression in Timer just before the release. My fault, I
should have tried to cleanup/fix the Timer code just before releasing. I
forgot to carefully check all mauve results and my last minute cleanup
broke gnu.testlet.javax.swing.Timer.test_23918. Tom, shouldn't the
autotester have caught this?

This fixes it by making sure the variable running is set correctly and
only set/tested when holding the queueLock. Documentation explaining
that also added.

2005-11-06  Mark Wielaard  <[EMAIL PROTECTED]>

        * javax/swing/Timer.java (Waker.run): Test and set running while
        holding queueLock.
        (start): Set running to true.
        (stop): Unconditionally notify queueLock.
        (queueEvent): Only called when queueLock already held.

Committed,

Mark
Index: javax/swing/Timer.java
===================================================================
RCS file: /cvsroot/classpath/classpath/javax/swing/Timer.java,v
retrieving revision 1.23
diff -u -r1.23 Timer.java
--- javax/swing/Timer.java	3 Nov 2005 00:01:08 -0000	1.23
+++ javax/swing/Timer.java	6 Nov 2005 22:51:25 -0000
@@ -65,10 +65,10 @@
      */
     public void run()
     {
-      try
+      synchronized (queueLock)
         {
-          synchronized (queueLock)
-            {
+	  try
+	    {
               try
                 {
                   queueLock.wait(initialDelay);
@@ -106,13 +106,12 @@
                     if (!repeats)
                       break;
                   }
-              running = false;
             }
-	}
-      finally
-        {
-          // The timer is no longer running.
-          running = false;
+	  finally
+	    {
+	      // The timer is no longer running.
+	      running = false;
+	    }
         }
     }
   }
@@ -157,7 +156,7 @@
 
   /**
    * <code>true</code> if the timer is currently active, firing events
-   * as scheduled.
+   * as scheduled. Should only be checked/set with queueLock held.
    */
   boolean running;
 
@@ -417,10 +416,12 @@
   {
     synchronized (queueLock)
       {
-	if (waker != null)
-	  return;
-	waker = new Waker();
-	waker.start();
+	if (!running)
+	  {
+	    running = true;
+	    waker = new Waker();
+	    waker.start();
+	  }
       }
   }
 
@@ -433,8 +434,7 @@
       {
 	running = false;
         queue = 0;
-	if (waker != null)
-	  queueLock.notifyAll();
+	queueLock.notifyAll();
 	waker = null;
       }
   }
@@ -493,14 +493,12 @@
   /**
   * Post a scheduled event to the event queue.
   * Package-private to avoid an accessor method.
+  * Called with queueLock held and running = true from Waker.run().
   */
   void queueEvent()
   {
-    synchronized (queueLock)
-      {
-        queue++;
-        if (queue == 1)
-          SwingUtilities.invokeLater(drainer);
-      }
+      queue++;
+      if (queue == 1)
+	SwingUtilities.invokeLater(drainer);
   }
 }

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
Classpath-patches mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/classpath-patches

Reply via email to