Skip to content

Commit 20be442

Browse files
vogellaclaude
andcommitted
Fix race condition in FilteredItemsSelectionDialog initial selection
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: #294 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 6d06b55 commit 20be442

File tree

2 files changed

+18
-7
lines changed

2 files changed

+18
-7
lines changed

bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/dialogs/FilteredItemsSelectionDialog.java

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -879,12 +879,24 @@ public void refresh() {
879879
lastRefreshSelection = prepareInitialSelection(lastRefreshSelection);
880880
}
881881
// preserve previous selection
882-
if (refreshWithLastSelection && lastRefreshSelection != null && lastRefreshSelection.size() > 0) {
883-
tableViewer.setSelection(new StructuredSelection(lastRefreshSelection));
882+
// Apply selection asynchronously to ensure table refresh has completed
883+
// This fixes race condition where setSelection() is called before
884+
// tableViewer.refresh() has fully updated the table items
885+
final List<Object> selectionToApply = lastRefreshSelection;
886+
if (refreshWithLastSelection && selectionToApply != null && selectionToApply.size() > 0) {
887+
tableViewer.getTable().getDisplay().asyncExec(() -> {
888+
if (!tableViewer.getTable().isDisposed()) {
889+
tableViewer.setSelection(new StructuredSelection(selectionToApply));
890+
}
891+
});
884892
} else {
885893
refreshWithLastSelection = true;
886-
tableViewer.getTable().setSelection(0);
887-
tableViewer.getTable().notifyListeners(SWT.Selection, new Event());
894+
tableViewer.getTable().getDisplay().asyncExec(() -> {
895+
if (!tableViewer.getTable().isDisposed()) {
896+
tableViewer.getTable().setSelection(0);
897+
tableViewer.getTable().notifyListeners(SWT.Selection, new Event());
898+
}
899+
});
888900
}
889901
} else {
890902
tableViewer.setSelection(StructuredSelection.EMPTY);

tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/dialogs/ResourceInitialSelectionTest.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -471,9 +471,8 @@ private void waitForDialogRefresh() {
471471
});
472472

473473
// Then wait additional time for selection to be applied
474-
// The selection is set asynchronously after table population completes
475-
// Previous fix used only 3 × 50ms = 150ms which was insufficient on slow systems
476-
// Increased to handle slower machines while minimizing delay on fast ones
474+
// FilteredItemsSelectionDialog.refresh() now uses Display.asyncExec() to apply
475+
// selection after table refresh completes, so we need to process those async events
477476
for (int i = 0; i < 5; i++) {
478477
processUIEvents();
479478
try {

0 commit comments

Comments
 (0)