From 33748d2f3aa654d9d1a75a01385b405278183dff Mon Sep 17 00:00:00 2001
From: Lars Vogel <Lars.Vogel@vogella.com>
Date: Mon, 17 Nov 2025 11:27:37 +0100
Subject: [PATCH] Fix race condition in FilteredItemsSelectionDialog initial
 selection
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The flaky ResourceInitialSelectionTest failures were caused by a race
condition in FilteredItemsSelectionDialog.refresh() where setSelection()
was called immediately after tableViewer.refresh(), before the table had
fully updated its items.

Root cause:
- FilteredItemsSelectionDialog.refresh() calls tableViewer.refresh() (line 874)
- This triggers async table updates, especially for virtual tables
- setSelection() was called immediately after (line 883), assuming refresh
  was complete
- On slow systems, the selection would be applied to an incomplete table
  and silently fail, resulting in empty selection: expected:<[...foo.txt]>
  but was:<[]>

The issue manifested as flaky test failures:
- testMultiSelectionAndSomeInitialNonExistingSelectionWithInitialPattern
- testSingleSelectionAndOneInitialSelectionWithInitialPattern
- testMultiSelectionAndTwoInitialSelectionsWithInitialPattern

These tests would intermittently fail with "Two files should be selected
by default" or "One file should be selected by default" assertions.

Solution:
Wrapped both selection application paths in Display.asyncExec() to defer
selection until after the table refresh completes:

1. For preserving previous selection:
   - Changed from: tableViewer.setSelection(new StructuredSelection(...))
   - Changed to: Display.asyncExec(() -> tableViewer.setSelection(...))

2. For default first item selection:
   - Changed from: table.setSelection(0)
   - Changed to: Display.asyncExec(() -> table.setSelection(0))

Both paths now include disposal checks to prevent errors if the dialog
is closed before the async execution runs.

The test's existing waitForDialogRefresh() method processes these async
events through its processUIEvents() calls, ensuring selections are
applied before assertions run. Updated the comment to reflect the
asyncExec fix.

Verified with multiple consecutive test runs - all 13 tests pass
consistently.

Fixes: https://github.com/eclipse-platform/eclipse.platform.ui/issues/294

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
---
 .../dialogs/FilteredItemsSelectionDialog.java | 20 +++++++++++++++----
 .../dialogs/ResourceInitialSelectionTest.java |  5 ++---
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/dialogs/FilteredItemsSelectionDialog.java b/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/dialogs/FilteredItemsSelectionDialog.java
index 5da18768f29..3c271eb86d8 100644
--- a/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/dialogs/FilteredItemsSelectionDialog.java
+++ b/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/dialogs/FilteredItemsSelectionDialog.java
@@ -879,12 +879,24 @@ public void refresh() {
 					lastRefreshSelection = prepareInitialSelection(lastRefreshSelection);
 				}
 				// preserve previous selection
-				if (refreshWithLastSelection && lastRefreshSelection != null && lastRefreshSelection.size() > 0) {
-					tableViewer.setSelection(new StructuredSelection(lastRefreshSelection));
+				// Apply selection asynchronously to ensure table refresh has completed
+				// This fixes race condition where setSelection() is called before
+				// tableViewer.refresh() has fully updated the table items
+				final List<Object> selectionToApply = lastRefreshSelection;
+				if (refreshWithLastSelection && selectionToApply != null && selectionToApply.size() > 0) {
+					tableViewer.getTable().getDisplay().asyncExec(() -> {
+						if (!tableViewer.getTable().isDisposed()) {
+							tableViewer.setSelection(new StructuredSelection(selectionToApply));
+						}
+					});
 				} else {
 					refreshWithLastSelection = true;
-					tableViewer.getTable().setSelection(0);
-					tableViewer.getTable().notifyListeners(SWT.Selection, new Event());
+					tableViewer.getTable().getDisplay().asyncExec(() -> {
+						if (!tableViewer.getTable().isDisposed()) {
+							tableViewer.getTable().setSelection(0);
+							tableViewer.getTable().notifyListeners(SWT.Selection, new Event());
+						}
+					});
 				}
 			} else {
 				tableViewer.setSelection(StructuredSelection.EMPTY);
diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/dialogs/ResourceInitialSelectionTest.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/dialogs/ResourceInitialSelectionTest.java
index 62b6416e60f..d52c83b3f4a 100644
--- a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/dialogs/ResourceInitialSelectionTest.java	
+++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/dialogs/ResourceInitialSelectionTest.java	
@@ -471,9 +471,8 @@ private void waitForDialogRefresh() {
 		});
 
 		// Then wait additional time for selection to be applied
-		// The selection is set asynchronously after table population completes
-		// Previous fix used only 3 × 50ms = 150ms which was insufficient on slow systems
-		// Increased to handle slower machines while minimizing delay on fast ones
+		// FilteredItemsSelectionDialog.refresh() now uses Display.asyncExec() to apply
+		// selection after table refresh completes, so we need to process those async events
 		for (int i = 0; i < 5; i++) {
 			processUIEvents();
 			try {