From 9c00085489e6cf6735e61eafc1f0c7eed1a7f16a Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Fri, 28 Aug 2026 13:20:46 +0200 Subject: [PATCH] Bound the event queue drain in DisplayHelper waitForCondition only re-checked its deadline after driveEventQueue returned, and driveEventQueue looped while readAndDispatch was true, so a display that never goes idle made the timeout unreachable. That is how a 60s wait in QuickAccessDialogTest.setUp consumed the whole 2h budget of org.eclipse.ui.tests on macOS. Cap one drain at 500ms. The Quick Access compute job could park for the same reason: its Display.syncExec from a job worker becomes pending work delivered by asyncExec, and SWT runs async messages only when nothing else was dispatched. Poll for the result instead, so a cancelled compute gives up rather than keeping COMPUTE_JOB_FAMILY non-empty forever. Addresses https://github.com/eclipse-platform/eclipse.platform.ui/issues/4289 --- .../quickaccess/QuickAccessContents.java | 55 ++++++++++++++++--- .../ui/tests/harness/util/DisplayHelper.java | 13 ++++- .../quickaccess/QuickAccessDialogTest.java | 5 +- 3 files changed, 61 insertions(+), 12 deletions(-) diff --git a/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/quickaccess/QuickAccessContents.java b/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/quickaccess/QuickAccessContents.java index d01f88c01c9..1fe70540916 100644 --- a/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/quickaccess/QuickAccessContents.java +++ b/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 collectProviderElements(QuickAccessProvider pro return Collections.emptyList(); } AtomicReference> 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 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 diff --git a/tests/org.eclipse.ui.tests.harness/src/org/eclipse/ui/tests/harness/util/DisplayHelper.java b/tests/org.eclipse.ui.tests.harness/src/org/eclipse/ui/tests/harness/util/DisplayHelper.java index c95eceb9663..7d7ef899293 100755 --- a/tests/org.eclipse.ui.tests.harness/src/org/eclipse/ui/tests/harness/util/DisplayHelper.java +++ b/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() { *

* If timeout < 0, nothing happens and false is returned. * If timeout == 0, the event loop is driven exactly once, - * but Display.sleep() is never invoked. + * but Display.sleep() is never invoked, and it dispatches for at + * most {@value #DRAIN_LIMIT_MS} ms. *

* * @param display the display to run the event loop of @@ -177,9 +182,15 @@ public static boolean runEventLoop(Display display, long timeout) { * true 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; } diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/quickaccess/QuickAccessDialogTest.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/quickaccess/QuickAccessDialogTest.java index 26ef0f9b5bc..dc83f2f0b01 100644 --- a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/quickaccess/QuickAccessDialogTest.java +++ b/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) {