Fix a race condition in waiting for EDT thread be idle.

* there are at least two EventQueue's (processed on the same EDT)
* an event from the second queue is being processed and the processing posts
  a new event onto the first queue (for example SwingUtilities.invokeLater()))
* at the moment when the new event is being posted onto the second queue the
  waitForIdle() method has already completed a call to the internal
  waitForIdle(*firstQueue*)

==>
  when the processing of the first event completes waitForIdle exits since it has
  successfully completed waiting for both queues, BUT there is an unprocessed message
  in the first queue.

ALSO: Remove special case handling for the case of a single EventQueue since we always
      have at least two queues and the multi-queue code also works for a single queue.

Bug: 71058346
Test: n/a (Flaky tests failing because of the race condition do not longer fail)
Change-Id: I95a7399942665826ec00542434e6e4cb113c05de

1 files changed, 45 insertions, 62 deletions

diff --git a/fest-swing/src/main/java/org/fest/swing/core/BasicRobot.java b/fest-swing/src/main/java/org/fest/swing/core/BasicRobot.java
index a81d847e..dff8f186 100755
--- a/fest-swing/src/main/java/org/fest/swing/core/BasicRobot.java
+++ b/fest-swing/src/main/java/org/fest/swing/core/BasicRobot.java
@@ -62,9 +62,12 @@ import java.awt.event.KeyEvent;
 import java.awt.event.WindowEvent;
 import java.util.Collection;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
+
 import javax.swing.JComponent;
 import javax.swing.JMenu;
 import javax.swing.JPopupMenu;
@@ -104,11 +107,6 @@ public class BasicRobot implements Robot {
 
   private volatile boolean active;
 
-  private static final Runnable EMPTY_RUNNABLE = new Runnable() {
-    @Override
-    public void run() {}
-  };
-
   private static final int BUTTON_MASK = BUTTON1_MASK | BUTTON2_MASK | BUTTON3_MASK;
 
   private static Toolkit toolkit = ToolkitProvider.instance().defaultToolkit();
@@ -784,79 +782,64 @@ public class BasicRobot implements Robot {
     waitForIdle();
   }
 
-  /** {@inheritDoc} */
+  /** {@inheritDoc}*/
   @RunsInEDT
   @Override
   public void waitForIdle() {
-    waitIfNecessary();
-    Collection<EventQueue> queues = windowMonitor.allEventQueues();
-    if (queues.size() == 1) {
-      waitForIdle(checkNotNull(toolkit.getSystemEventQueue()));
-      return;
-    }
-    // FIXME this resurrects dead event queues
-    for (EventQueue queue : queues) {
-      waitForIdle(checkNotNull(queue));
-    }
-  }
-
-  private void waitIfNecessary() {
-    int delayBetweenEvents = settings.delayBetweenEvents();
-    int eventPostingDelay = settings.eventPostingDelay();
-    if (eventPostingDelay > delayBetweenEvents) {
-      pause(eventPostingDelay - delayBetweenEvents);
-    }
-  }
-
-  private void waitForIdle(@NotNull EventQueue eventQueue) {
     if (EventQueue.isDispatchThread()) {
       throw new IllegalThreadStateException("Cannot call method from the event dispatcher thread");
     }
-    // Abbot: as of Java 1.3.1, robot.waitForIdle only waits for the last event on the queue at the time of this
-    // invocation to be processed. We need better than that. Make sure the given event queue is empty when this method
-    // returns.
-    // We always post at least one idle event to allow any current event dispatch processing to finish.
+    waitIfNecessary();
+    Collection<EventQueue> queues = windowMonitor.allEventQueues();
+    // FIXME this resurrects dead event queues
     long start = currentTimeMillis();
-    int count = 0;
+    int idleTimeout = settings.idleTimeout();
+    Object waitLock = new Object() {
+    };
+    AtomicInteger queuesToProcess = new AtomicInteger();  // Just a wrapper, not used as atomic.
+    AtomicBoolean nonEmptyQueues = new AtomicBoolean();  // Just a wrapper, not used as atomic.
     do {
-      // Timed out waiting for idle
-      int idleTimeout = settings.idleTimeout();
-      if (postInvocationEvent(eventQueue, idleTimeout)) {
-        break;
+      queuesToProcess.set(queues.size());
+      nonEmptyQueues.set(true);
+      for (EventQueue queue : queues) {
+        queue.postEvent(
+          new InvocationEvent(
+            toolkit,
+            () -> {
+              synchronized (waitLock) {
+                if (queuesToProcess.decrementAndGet() == 0) {
+                  // This is the the last queue.
+                  nonEmptyQueues.set(queues.stream().anyMatch(v -> v.peekEvent() != null));
+                }
+                waitLock.notifyAll();
+              }
+            }, waitLock, true));
       }
-      // Timed out waiting for idle event queue
-      if (currentTimeMillis() - start > idleTimeout) {
-        break;
+      synchronized (waitLock) {
+        while (currentTimeMillis() - start < idleTimeout && queuesToProcess.get() > 0) {
+          try {
+            waitLock.wait(idleTimeout - (currentTimeMillis() - start));
+          }
+          catch (InterruptedException e) {
+            break;
+          }
+        }
       }
-      ++count;
       // Force a yield
       pause();
-      // Abbot: this does not detect invocation events (i.e. what gets posted with EventQueue.invokeLater), so if
-      // someone is repeatedly posting one, we might get stuck. Not too worried, since if a Runnable keeps calling
-      // invokeLater on itself, *nothing* else gets much chance to run, so it seems to be a bad programming practice.
-    } while (eventQueue.peekEvent() != null);
-  }
-
-  // Indicates whether we timed out waiting for the invocation to run
-  @RunsInEDT
-  private boolean postInvocationEvent(@NotNull EventQueue eventQueue, long timeout) {
-    Object lock = new RobotIdleLock();
-    synchronized (lock) {
-      eventQueue.postEvent(new InvocationEvent(toolkit, EMPTY_RUNNABLE, lock, true));
-      long start = currentTimeMillis();
-      try {
-        // NOTE: on fast linux systems when showing a dialog, if we don't provide a timeout, we're never notified, and
-        // the test will wait forever (up through 1.5.0_05).
-        lock.wait(timeout);
-        return (currentTimeMillis() - start) >= settings.idleTimeout();
-      } catch (InterruptedException e) {
+      if (currentTimeMillis() - start >= idleTimeout) {
+        // Silently ignore the timed out wait.
+        break;
       }
-      return false;
     }
+    while (nonEmptyQueues.get());
   }
 
-  private static class RobotIdleLock {
-    RobotIdleLock() {
+  private void waitIfNecessary() {
+    int delayBetweenEvents = settings.delayBetweenEvents();
+    int eventPostingDelay = settings.eventPostingDelay();
+    if (eventPostingDelay > delayBetweenEvents) {
+      pause(eventPostingDelay - delayBetweenEvents);
     }
   }