Re: [7u8] Review request for 7177040: Deadlock between PostEventQueue.noEvents, EventQueue.isDispatchThread and SwingUtilities.invokeLater

[Artem Ananiev]

Hi, Peter,

7177040 is about deadlock between EventQueue and PostEventQueue, it's 
not considered to simplify SunToolkit.flushPendingEvents(). Please, 
expect flushLock changes in JDK8 as a part of the fix for 7186109.

Thanks,

Artem

On 7/24/2012 6:32 PM, Peter Levart wrote:
Ok, now to get rid of flushLock in SunToolkit so that flushPendingEvents
is written as plain:

     public static void flushPendingEvents()  {
         AppContext appContext = AppContext.getAppContext();
         PostEventQueue postEventQueue =
             (PostEventQueue)appContext.get(POST_EVENT_QUEUE_KEY);
         if (postEventQueue != null) {
             postEventQueue.flush();
         }
     }

... you could merge the recursion prevention functionality into the
PostEventQueue in a manner similar to this:

class PostEventQueue {
     private EventQueueItem queueHead = null;
     private EventQueueItem queueTail = null;
     private final EventQueue eventQueue;

     // For the recursion prevention and noEvents() status
     private Thread flushingThread = null;

     PostEventQueue(EventQueue eq) {
         eventQueue = eq;
     }

     public synchronized boolean noEvents() {
         return queueHead == null && flushingThread == null;
     }

     public void flush() {
         EventQueueItem tempQueue;
         synchronized (this) {
             while (flushingThread != null && flushingThread !=
Thread.currentThread()) try {
                 wait();
             }
             catch (InterruptedException e) { // what can we do?
                 Thread.currentThread().interrupt();
                 return;
             }

             // prevent recursion
             if (flushingThread == Thread.currentThread()) return;

             tempQueue = queueHead;
             queueHead = queueTail = null;

             // queue was empty
             if (tempQueue == null) return;

             flushingThread = Thread.currentThread();
             notify();
         }
         try {
             while (tempQueue != null) {
                 eventQueue.postEvent(tempQueue.event);
                 tempQueue = tempQueue.next;
             }
         }
         finally {
             synchronized (this) {
                 flushingThread = null;
                 notify();
             }
         }
     }

     /*
      * Enqueue an AWTEvent to be posted to the Java EventQueue.
      */
     void postEvent(AWTEvent event) {
         EventQueueItem item = new EventQueueItem(event);

         synchronized (this) {
             if (queueHead == null) {
                 queueHead = queueTail = item;
             } else {
                 queueTail.next = item;
                 queueTail = item;
             }
         }
         SunToolkit.wakeupEventQueue(eventQueue, event.getSource() ==
AWTAutoShutdown.getInstance());
     }
} // class PostEventQueue


What do you think?

Regards, Peter


2012/7/24 Oleg Pekhovskiy <oleg.pekhovs...@oracle.com
<mailto:oleg.pekhovs...@oracle.com>>

    Hi Peter,

    so obvious, thank you!

    Oleg.


    7/24/2012 3:42 PM, Peter Levart wrote:

        Hi Oleg,

        This is just cosmetics, but:

        SunToolkit:
        public synchronized boolean noEvents() {
                 return queueHead == null && !isFlushing;
             }

        ... a thread calling noEvents could see occasional "spikes" of
        false return even though there is no flushing being performed
        (when other thread is calling flush on an empty PostEventQueue).

        Improved flush method would look like this:

              public void flush() {
                  EventQueueItem tempQueue;
                  synchronized (this) {
                      tempQueue = queueHead;
                      queueHead = queueTail = null;
                      isFlushing =*/(tempQueue != null)/*;

                  }
                  try {
                      while (tempQueue != null) {
                          eventQueue.postEvent(__tempQueue.event);
                          tempQueue = tempQueue.next;
                      }
                  }
                  finally {
                      isFlushing = false;
                  }
              }

        Regards, Peter

        2012/7/23 Oleg Pekhovskiy <oleg.pekhovs...@oracle.com
        <mailto:oleg.pekhovs...@oracle.com>
        <mailto:oleg.pekhovskiy@__oracle.com
        <mailto:oleg.pekhovs...@oracle.com>>>


             Hi!

             Please review this back-port being already pushed to jdk8 but
             deferred for 7u6.

             Bug:
        http://bugs.sun.com/view_bug.__do?bug_id=7177040
        <http://bugs.sun.com/view_bug.do?bug_id=7177040>

             Webrev:
        http://cr.openjdk.java.net/~__bagiras/7u8/7177040.1
        <http://cr.openjdk.java.net/~bagiras/7u8/7177040.1>
        <http://cr.openjdk.java.net/%__7Ebagiras/7u8/7177040.1
        <http://cr.openjdk.java.net/%7Ebagiras/7u8/7177040.1>>


             Review thread for 7u6:
        http://mail.openjdk.java.net/__pipermail/awt-dev/2012-July/__003106.html
        <http://mail.openjdk.java.net/pipermail/awt-dev/2012-July/003106.html>

             Reviewers 7u6 & 8:
             Anthony Petrov, Anton Tarasov

             Thanks,
             Oleg