[cp-patches] FYI: Fix javax.swing.Timer

[Mark Wielaard]

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  <address@hidden>

        * 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);
   }
 }

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