Bound the event queue drain in DisplayHelper

bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/quickaccess/QuickAccessContents.java

@@ -30,6 +30,8 @@
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Objects;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
 import java.util.function.Function;
@@ -56,6 +58,7 @@
 import org.eclipse.jface.viewers.ColumnWeightData;
 import org.eclipse.osgi.util.NLS;
 import org.eclipse.swt.SWT;
+import org.eclipse.swt.SWTException;
 import org.eclipse.swt.events.ControlAdapter;
 import org.eclipse.swt.events.ControlEvent;
 import org.eclipse.swt.events.KeyEvent;
@@ -114,6 +117,9 @@ public abstract class QuickAccessContents {
 	 */
 	public static final Object COMPUTE_JOB_FAMILY = new Object();
 
+	/** How often a compute job re-checks cancellation while waiting for the UI thread. */
+	private static final long UI_ACCESS_POLL_INTERVAL_MS = 100;
+
 	protected Text filterText;
 
 	private final QuickAccessProvider[] providers;
@@ -453,19 +459,52 @@ private List<QuickAccessElement> collectProviderElements(QuickAccessProvider pro
 			return Collections.emptyList();
 		}
 		AtomicReference<List<QuickAccessElement>> result = new AtomicReference<>(Collections.emptyList());
-		table.getDisplay().syncExec(() -> {
-			if (monitor.isCanceled() || table.isDisposed()) {
-				return;
+		CountDownLatch queried = new CountDownLatch(1);
+		try {
+			Display display = table.getDisplay();
+			if (Display.getCurrent() == display) {
+				// Waiting on the latch below would deadlock against our own asyncExec.
+				return queryProvider(provider, filter, monitor);
 			}
-			try {
-				result.set(Arrays.asList(provider.getElementsSorted(filter, monitor)));
-			} catch (RuntimeException e) {
-				WorkbenchPlugin.log(e);
+			display.asyncExec(() -> {
+				try {
+					result.set(queryProvider(provider, filter, monitor));
+				} finally {
+					queried.countDown();
+				}
+			});
+		} catch (SWTException e) { // table or display disposed while the dialog was closing
+			return Collections.emptyList();
+		}
+		// Unlike syncExec, polling lets a cancelled compute give up on a display that
+		// never goes idle.
+		try {
+			while (!queried.await(UI_ACCESS_POLL_INTERVAL_MS, TimeUnit.MILLISECONDS)) {
+				if (monitor.isCanceled() || table.isDisposed()) {
+					return Collections.emptyList();
+				}
 			}
-		});
+		} catch (InterruptedException e) {
+			Thread.currentThread().interrupt();
+			return Collections.emptyList();
+		}
 		return result.get();
 	}
 
+	/** Queries one provider on the display thread. */
+	private List<QuickAccessElement> queryProvider(QuickAccessProvider provider, String filter,
+			IProgressMonitor monitor) {
+		if (monitor.isCanceled() || table.isDisposed()) {
+			return Collections.emptyList();
+		}
+		try {
+			return Arrays.asList(provider.getElementsSorted(filter, monitor));
+		} catch (RuntimeException e) {
+			WorkbenchPlugin.log(e);
+			return Collections.emptyList();
+		}
+	}
+
 	/**
 	 * Queries each provider in turn and streams the matching entries to {@code render}
 	 * after every provider that contributes, so results appear as they are computed

tests/org.eclipse.ui.tests.harness/src/org/eclipse/ui/tests/harness/util/DisplayHelper.java

@@ -33,6 +33,10 @@
  * @since 3.1
  */
 public abstract class DisplayHelper {
+
+	/** Upper bound for one drain of the event queue. */
+	private static final long DRAIN_LIMIT_MS= 500;
+
 	/**
 	 * Creates a new instance.
 	 */
@@ -132,7 +136,8 @@ public boolean condition() {
 	 * <p>
 	 * If <code>timeout &lt; 0</code>, nothing happens and false is returned.
 	 * If <code>timeout == 0</code>, the event loop is driven exactly once,
-	 * but <code>Display.sleep()</code> is never invoked.
+	 * but <code>Display.sleep()</code> is never invoked, and it dispatches for at
+	 * most {@value #DRAIN_LIMIT_MS} ms.
 	 * </p>
 	 *
 	 * @param display the display to run the event loop of
@@ -177,9 +182,15 @@ public static boolean runEventLoop(Display display, long timeout) {
 	 *         <code>true</code> at least once
 	 */
 	private static boolean driveEventQueue(Display display) {
+		// A display that never stops producing events would never let readAndDispatch
+		// return false, making the caller's own timeout unreachable.
+		long deadline= System.nanoTime() + DRAIN_LIMIT_MS * 1_000_000L;
 		boolean events= false;
 		while (display.readAndDispatch()) {
 			events= true;
+			if (System.nanoTime() - deadline > 0) {
+				break;
+			}
 		}
 		return events;
 	}

tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/quickaccess/QuickAccessDialogTest.java

@@ -453,9 +453,8 @@ private static void waitForQuickAccessResults(Display display) {
 		boolean computed = DisplayHelper.waitForCondition(display, COMPUTE_TIMEOUT,
 				() -> Job.getJobManager().find(QuickAccessContents.COMPUTE_JOB_FAMILY).length == 0);
 		assertTrue(computed, "Quick Access computation did not finish");
-		while (display.readAndDispatch()) {
-			// drain the asyncExec that renders the streamed results
-		}
+		// drain the asyncExec that renders the streamed results
+		DisplayHelper.runEventLoop(display, 0);
 	}
 
 	private boolean dialogContains(QuickAccessDialog dialog, String substring) {