<AWT Dev> [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.pekhovskiy at oracle.com
> <mailto:oleg.pekhovskiy at 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.pekhovskiy at oracle.com
>         <mailto:oleg.pekhovskiy at oracle.com>
>         <mailto:oleg.pekhovskiy at __oracle.com
>         <mailto:oleg.pekhovskiy at 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
>
>
>
>