Skip to content

Commit 39cf286

Browse files
committed
Fix flaky ResourceInitialSelectionTest via pipeline scheduling rule
FilteredItemsSelectionDialog's background pipeline has three jobs that read and write the same ContentProvider state: FilterHistoryJob mutates it via contentProvider.reset() and contentProvider.addHistoryItems(), FilterJob populates it via contentProvider.add() (inside fillContentProvider), and RefreshCacheJob reads it in contentProvider.reloadCache(). Without a shared scheduling rule they can run concurrently on different worker threads. Under the right timing, FilterHistoryJob.contentProvider.reset() clears the items set on one worker while FilterJob is iterating members() and adding items on another - wiping some or all of the populated items before the table is rendered. This manifested as the long-standing ResourceInitialSelectionTest flake (issue #294): sometimes the table was empty, sometimes a random subset of items survived in the wrong order. The race is reliably triggered by the ModifyListener on the pattern Text firing applyFilter() twice in quick succession during createDialogArea on slow runners, which schedules two FilterHistoryJobs that overlap with the FilterJob. Fix: share a per-dialog ISchedulingRule across filterHistoryJob, filterJob and refreshCacheJob. They now serialize; the race cannot occur. Separate dialogs still run in parallel. Also expose JOB_FAMILY (@noreference) and tag all four pipeline jobs with it, so tests can deterministically wait for the pipeline via Job.getJobManager().find(JOB_FAMILY). The item-count stability probe in ResourceInitialSelectionTest.waitForDialogRefresh() is replaced with that family-based wait - this also gets rid of the unreliable 5 s timeout that was the visible symptom. Fixes #294
1 parent 7f7e4b6 commit 39cf286

3 files changed

Lines changed: 91 additions & 32 deletions

File tree

bundles/org.eclipse.ui.workbench/.settings/.api_filters

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
<?xml version="1.0" encoding="UTF-8" standalone="no"?>
22
<component id="org.eclipse.ui.workbench" version="2">
3+
<resource path="eclipseui/org/eclipse/ui/dialogs/FilteredItemsSelectionDialog.java" type="org.eclipse.ui.dialogs.FilteredItemsSelectionDialog">
4+
<filter id="336658481">
5+
<message_arguments>
6+
<message_argument value="org.eclipse.ui.dialogs.FilteredItemsSelectionDialog"/>
7+
<message_argument value="JOB_FAMILY"/>
8+
</message_arguments>
9+
</filter>
10+
</resource>
311
<resource path="eclipseui/org/eclipse/ui/dialogs/YesNoCancelListSelectionDialog.java" type="org.eclipse.ui.dialogs.YesNoCancelListSelectionDialog">
412
<filter id="576778288">
513
<message_arguments>

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

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
import org.eclipse.core.runtime.ProgressMonitorWrapper;
5151
import org.eclipse.core.runtime.Status;
5252
import org.eclipse.core.runtime.SubMonitor;
53+
import org.eclipse.core.runtime.jobs.ISchedulingRule;
5354
import org.eclipse.core.runtime.jobs.Job;
5455
import org.eclipse.jface.action.Action;
5556
import org.eclipse.jface.action.ActionContributionItem;
@@ -236,6 +237,17 @@ public abstract class FilteredItemsSelectionDialog extends SelectionStatusDialog
236237
*/
237238
private boolean isShownForTheFirstTime = true;
238239

240+
/**
241+
* Job family under which all background filtering and refresh jobs of this
242+
* dialog are grouped. Tests may use this with
243+
* {@link org.eclipse.core.runtime.jobs.IJobManager#join(Object, org.eclipse.core.runtime.IProgressMonitor)}
244+
* to deterministically wait for the filter/refresh pipeline to settle.
245+
*
246+
* @noreference This field is not intended to be referenced by clients.
247+
* @since 3.138
248+
*/
249+
public static final Object JOB_FAMILY = new Object();
250+
239251
/**
240252
* Creates a new instance of the class.
241253
*
@@ -250,10 +262,36 @@ public FilteredItemsSelectionDialog(Shell shell, boolean multi) {
250262
filterJob = new FilterJob();
251263
contentProvider = new ContentProvider();
252264
refreshCacheJob = new RefreshCacheJob();
265+
// All three jobs mutate / read the same ContentProvider state (items,
266+
// duplicates, lastFilteredItems, ...). Without a common scheduling rule
267+
// they can run concurrently on different worker threads, racing
268+
// FilterHistoryJob.contentProvider.reset() against FilterJob's
269+
// fillContentProvider() and silently emptying or corrupting the visible
270+
// items. The rule is per-dialog, so separate dialogs still run in
271+
// parallel.
272+
ISchedulingRule pipelineRule = new ContentProviderSerialRule();
273+
filterHistoryJob.setRule(pipelineRule);
274+
filterJob.setRule(pipelineRule);
275+
refreshCacheJob.setRule(pipelineRule);
253276
itemsListSeparator = new ItemsListSeparator(WorkbenchMessages.FilteredItemsSelectionDialog_separatorLabel);
254277
selectionMode = NONE;
255278
}
256279

280+
/**
281+
* Per-dialog mutex rule used to serialize the filter/refresh pipeline jobs.
282+
*/
283+
private static final class ContentProviderSerialRule implements ISchedulingRule {
284+
@Override
285+
public boolean contains(ISchedulingRule rule) {
286+
return rule == this;
287+
}
288+
289+
@Override
290+
public boolean isConflicting(ISchedulingRule rule) {
291+
return rule == this;
292+
}
293+
}
294+
257295
/**
258296
* Creates a new instance of the class. Created dialog won't allow to select
259297
* more than one item.
@@ -1315,6 +1353,11 @@ public IStatus runInUIThread(IProgressMonitor monitor) {
13151353
return new Status(IStatus.OK, PlatformUI.PLUGIN_ID, IStatus.OK, EMPTY_STRING, null);
13161354
}
13171355

1356+
@Override
1357+
public boolean belongsTo(Object family) {
1358+
return family == JOB_FAMILY;
1359+
}
1360+
13181361
}
13191362

13201363
/**
@@ -1420,6 +1463,11 @@ protected void canceling() {
14201463
contentProvider.stopReloadingCache();
14211464
}
14221465

1466+
@Override
1467+
public boolean belongsTo(Object family) {
1468+
return family == JOB_FAMILY;
1469+
}
1470+
14231471
}
14241472

14251473
private class RemoveHistoryItemAction extends Action {
@@ -1854,6 +1902,11 @@ protected IStatus run(IProgressMonitor monitor) {
18541902
return Status.OK_STATUS;
18551903
}
18561904

1905+
@Override
1906+
public boolean belongsTo(Object family) {
1907+
return family == JOB_FAMILY;
1908+
}
1909+
18571910
}
18581911

18591912
/**
@@ -1977,6 +2030,11 @@ protected void filterContent(GranualProgressMonitor monitor) throws CoreExceptio
19772030

19782031
}
19792032

2033+
@Override
2034+
public boolean belongsTo(Object family) {
2035+
return family == JOB_FAMILY;
2036+
}
2037+
19802038
}
19812039

19822040
/**

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

Lines changed: 25 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import org.eclipse.swt.widgets.Table;
3939
import org.eclipse.swt.widgets.TableItem;
4040
import org.eclipse.ui.PlatformUI;
41+
import org.eclipse.ui.dialogs.FilteredItemsSelectionDialog;
4142
import org.eclipse.ui.dialogs.FilteredResourcesSelectionDialog;
4243
import org.eclipse.ui.internal.decorators.DecoratorManager;
4344
import org.eclipse.ui.tests.harness.util.DisplayHelper;
@@ -125,8 +126,11 @@ public void testSingleSelectionAndOneInitialNonExistingSelectionWithInitialPatte
125126
dialog.open();
126127
dialog.refresh();
127128

128-
// Don't wait for full refresh - this test checks that invalid initial
129-
// selections don't cause a selection before dialog is fully loaded
129+
// Intentionally no waitForDialogRefresh: this asserts that during
130+
// initial load, the dialog does not pre-select anything for an
131+
// invalid initial element. After the refresh pipeline drains the
132+
// dialog falls back to selecting row 0, which is a separate
133+
// behavior not under test here.
130134

131135
List<Object> selected = getSelectedItems(dialog);
132136

@@ -167,8 +171,10 @@ public void testSingleSelectionAndOneFilteredSelection() {
167171
dialog.open();
168172
dialog.refresh();
169173

170-
// Don't wait for full refresh - this test checks that filtered initial
171-
// selections don't cause a selection before dialog is fully loaded
174+
// Intentionally no waitForDialogRefresh: foofoo is filtered out by
175+
// the *.txt pattern, so during initial load nothing is selected.
176+
// After the refresh pipeline drains the dialog falls back to row 0,
177+
// which is a separate behavior not under test here.
172178

173179
List<Object> selected = getSelectedItems(dialog);
174180

@@ -274,8 +280,11 @@ public void testMultiSelectionAndTwoInitialNonExistingSelectionWithInitialPatter
274280
dialog.open();
275281
dialog.refresh();
276282

277-
// Don't wait for full refresh - this test checks that invalid initial
278-
// selections don't cause a selection before dialog is fully loaded
283+
// Intentionally no waitForDialogRefresh: this asserts that during
284+
// initial load, the dialog does not pre-select anything for invalid
285+
// initial elements. After the refresh pipeline drains the dialog
286+
// falls back to selecting row 0, which is a separate behavior not
287+
// under test here.
279288

280289
List<Object> selected = getSelectedItems(dialog);
281290

@@ -448,37 +457,21 @@ private void processUIEvents() {
448457
}
449458

450459
/**
451-
* Wait for dialog refresh jobs to complete and process UI events.
452-
* This ensures background jobs finish before assertions are made.
460+
* Wait for the dialog's background filter/refresh jobs to complete.
461+
* <p>
462+
* The dialog schedules a chain of jobs on open/refresh:
463+
* {@code FilterHistoryJob → FilterJob → RefreshCacheJob → RefreshJob}. All
464+
* four are tagged with {@link FilteredItemsSelectionDialog#JOB_FAMILY}, so
465+
* we wait for that family to drain. The 30 s ceiling is a deadlock guard;
466+
* the pipeline usually completes within a few hundred milliseconds.
453467
*/
454468
private void waitForDialogRefresh() {
455469
Display display = PlatformUI.getWorkbench().getDisplay();
456-
457-
// The dialog performs async operations (FilterHistoryJob → FilterJob →
458-
// RefreshCacheJob → RefreshJob) to filter and populate the table after refresh()
459-
// We need to wait for the table item count to stabilize before checking
460-
// selection state. The count can temporarily be non-zero after the history
461-
// refresh, then change again when FilterJob populates actual results.
462-
// Selection is applied only in the final refresh, so we must wait for
463-
// stability rather than just for any non-zero count.
464-
int[] lastCount = { -1 };
465-
DisplayHelper.waitForCondition(display, 5000, () -> {
470+
DisplayHelper.waitForCondition(display, 30_000L, () -> {
466471
processUIEvents();
467-
try {
468-
Table table = (Table) ((Composite) ((Composite) ((Composite) dialog.getShell().getChildren()[0])
469-
.getChildren()[0]).getChildren()[0]).getChildren()[3];
470-
int count = table.getItemCount();
471-
if (count > 0 && count == lastCount[0]) {
472-
return true; // stable non-zero count: all refreshes have completed
473-
}
474-
lastCount[0] = count;
475-
return false;
476-
} catch (Exception e) {
477-
return false;
478-
}
472+
return Job.getJobManager().find(FilteredItemsSelectionDialog.JOB_FAMILY).length == 0;
479473
});
480-
481-
// Final event loop processing to pick up any trailing async tasks
474+
// Final event loop processing to pick up any trailing asyncExecs.
482475
processUIEvents();
483476
}
484477

0 commit comments

Comments
 (0)