CountDownLatch Synchronization for RCPTestWorkbenchAdvisor #3429

vogella · 2025-10-22T15:20:42Z

The flaky test was caused by a race condition in
RCPTestWorkbenchAdvisor.java. The test spawns 4 background threads during preStartup() that schedule async/sync runnables on the display.

However, there was no guarantee these runnables would execute before postStartup() marked the workbench as "started" (by setting started = true).

This meant asyncWithDisplayAccess could still be null when the test assertions ran, causing intermittent failures.

This change adds a CountDownLatch to ensure all 4 async/sync operations complete before marking the workbench as started:

Key changes to
/tests/org.eclipse.ui.tests.harness/src/org/eclipse/ui/tests/harness/util/RCPTestWorkbenchAdvisor.java:

Added latch field (line 48):
- private static java.util.concurrent.CountDownLatch asyncLatch = null;
Initialize latch in preStartup() (line 130):
- asyncLatch = new java.util.concurrent.CountDownLatch(4); — tracks 4 operations
Count down after each operation completes:
- In setupSyncDisplayThread(): Added finally block (lines 179-183) that calls asyncLatch.countDown()
- In setupAsyncDisplayThread(): Added finally block (lines 205-209) that calls asyncLatch.countDown()
Wait for operations in postStartup() (lines 222-235):
- Waits up to 5 seconds for all operations to complete
- Logs warnings if timeout occurs or thread is interrupted
- Only marks started = true after all operations complete

Fixes #1517

eclipse-platform-bot · 2025-10-22T15:27:25Z

This pull request changes some projects for the first time in this development cycle.
Therefore the following files need a version increment:

tests/org.eclipse.ui.tests.harness/META-INF/MANIFEST.MF

An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch.

Git patch

From 8bf477c3eba2a22ebaac6bf63bdf0487305134f7 Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <[email protected]>
Date: Wed, 22 Oct 2025 15:26:58 +0000
Subject: [PATCH] Version bump(s) for 4.38 stream


diff --git a/tests/org.eclipse.ui.tests.harness/META-INF/MANIFEST.MF b/tests/org.eclipse.ui.tests.harness/META-INF/MANIFEST.MF
index 259fee3e99..8cdb2f2c5f 100644
--- a/tests/org.eclipse.ui.tests.harness/META-INF/MANIFEST.MF
+++ b/tests/org.eclipse.ui.tests.harness/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: Harness Plug-in
 Bundle-SymbolicName: org.eclipse.ui.tests.harness;singleton:=true
-Bundle-Version: 1.10.600.qualifier
+Bundle-Version: 1.10.700.qualifier
 Eclipse-BundleShape: dir
 Require-Bundle: org.eclipse.ui,
  org.eclipse.core.runtime,
-- 
2.51.0

Further information are available in Common Build Issues - Missing version increments.

github-actions · 2025-10-22T16:32:50Z

Test Results

3 018 files ±0 3 018 suites ±0 2h 19m 46s ⏱️ + 5m 32s
8 229 tests ±0 7 955 ✅ - 25 249 💤 ±0 2 ❌ +2 23 🔥 +23
23 607 runs ±0 22 740 ✅ - 73 794 💤 ±0 4 ❌ +4 69 🔥 +69

For more details on these failures and errors, see this check.

Results for commit cd9113d. ± Comparison against base commit 3caaf9b.

♻️ This comment has been updated with latest results.

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

fedejeanne · 2025-10-22T18:41:30Z

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

+				if (!completed) {
+					System.err.println("Warning: Not all async/sync operations completed within timeout");
+				}


I would let it fail. I'd rather have a test that fails consistently than one that might fail even if completed == false.

Suggested change

if (!completed) {

System.err.println("Warning: Not all async/sync operations completed within timeout");

}

assertTrue(completed, "Not all async/sync operations completed within timeout");

Hm, it seems I missed an important part: when this fails, the workbench is already created and therefore subsequent attempts to create it will fail too.

After the latest changes, the logs show that once this happens, a bunch of tests fail with the error Workbench already exists and cannot be created again

Here are the first 3 failures.

Stack traces

org.eclipse.ui.tests.rcp.PlatformUITest.testCreateAndRunWorkbench -- Time elapsed: 7.070 s <<< FAILURE! java.lang.AssertionError: Not all async/sync operations completed within timeout at org.eclipse.ui.tests.harness.util.RCPTestWorkbenchAdvisor.postStartup(RCPTestWorkbenchAdvisor.java:221) at org.eclipse.ui.tests.rcp.util.WorkbenchAdvisorObserver.postStartup(WorkbenchAdvisorObserver.java:140) at org.eclipse.ui.internal.Workbench.lambda$18(Workbench.java:2901) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1104) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:339) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1038) at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:153) at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:677) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:339) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:583) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:173) at org.eclipse.ui.tests.rcp.PlatformUITest.testCreateAndRunWorkbench(PlatformUITest.java:57) at java.base/java.lang.reflect.Method.invoke(Method.java:580) at java.base/java.util.ArrayList.forEach(ArrayList.java:1596) at java.base/java.util.ArrayList.forEach(ArrayList.java:1596) org.eclipse.ui.tests.rcp.PlatformUITest.testCreateAndRunWorkbenchWithExceptionOnStartup skipped Running RcpTestSuite WorkbenchAdvisorTest Tests run: 9, Failures: 0, Errors: 9, Skipped: 0, Time elapsed: 0.428 s <<< FAILURE! -- in RcpTestSuite WorkbenchAdvisorTest org.eclipse.ui.tests.rcp.WorkbenchAdvisorTest.testEmptyProgressRegion -- Time elapsed: 0.191 s <<< ERROR! java.lang.IllegalStateException: Workbench already exists and cannot be created again. at org.eclipse.ui.internal.Workbench.<init>(Workbench.java:481) at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:609) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:339) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:583) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:173) at org.eclipse.ui.tests.rcp.WorkbenchAdvisorTest.testEmptyProgressRegion(WorkbenchAdvisorTest.java:293) at java.base/java.lang.reflect.Method.invoke(Method.java:580) at java.base/java.util.ArrayList.forEach(ArrayList.java:1596) at java.base/java.util.ArrayList.forEach(ArrayList.java:1596) org.eclipse.ui.tests.rcp.WorkbenchAdvisorTest.testFillAllActionBar -- Time elapsed: 0.031 s <<< ERROR! java.lang.IllegalStateException: Workbench already exists and cannot be created again. at org.eclipse.ui.internal.Workbench.<init>(Workbench.java:481) at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:609) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:339) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:583) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:173) at org.eclipse.ui.tests.rcp.WorkbenchAdvisorTest.testFillAllActionBar(WorkbenchAdvisorTest.java:279) at java.base/java.lang.reflect.Method.invoke(Method.java:580) at java.base/java.util.ArrayList.forEach(ArrayList.java:1596) at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)

The 1st failure is due to the timeout (Not all async/sync operations completed within timeout) and the workbench is not cleaned-up/shutdown. The following failing tests also fail with Workbench already exists and cannot be created again..

The way I see it, we need 2 things:

See why the timeout occurs (is it "just" a matter of waiting a bit longer since the hardware is old and slow or is it something else?)

Clean-up if the timeout occurs so that the following tests may start the workbench again.

I don't have any novel ideas at the moment :-\ and you @vogella ?

…chronization The flaky test was caused by a race condition in RCPTestWorkbenchAdvisor.java. The test spawns 4 background threads during preStartup() that schedule async/sync runnables on the display. However, there was no guarantee these runnables would execute before postStartup() marked the workbench as "started" (by setting started = true). This meant asyncWithDisplayAccess could still be null when the test assertions ran, causing intermittent failures. This change adds a CountDownLatch to ensure all 4 async/sync operations complete before marking the workbench as started. Key changes to RCPTestWorkbenchAdvisor.java: - Added CountDownLatch field initialized as a constant with count of 4 - Added proper imports for CountDownLatch and TimeUnit - Simplified Boolean assignments to use !isSTARTED() directly - Added finally blocks in setupSyncDisplayThread() and setupAsyncDisplayThread() to call asyncLatch.countDown() - Modified postStartup() to await latch completion with 5-second timeout - Replaced warning logs with AssertionError on timeout and RuntimeException on interruption to ensure test failures are explicit Also bumps version of org.eclipse.ui.tests.harness for 4.38 stream. Fixes eclipse-platform#1517 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

vogella mentioned this pull request Oct 22, 2025

Migrate JUnit 4 tests to JUnit 5 in org.eclipse.jface.text.tests #3410

Closed

fedejeanne reviewed Oct 22, 2025

View reviewed changes

vogella force-pushed the flacky-test-RCP-test-advisor branch 2 times, most recently from 34da034 to bebe15f Compare October 23, 2025 12:22

vogella force-pushed the flacky-test-RCP-test-advisor branch from bebe15f to cd9113d Compare October 23, 2025 14:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

CountDownLatch Synchronization for RCPTestWorkbenchAdvisor #3429

CountDownLatch Synchronization for RCPTestWorkbenchAdvisor #3429

vogella commented Oct 22, 2025

Uh oh!

eclipse-platform-bot commented Oct 22, 2025

Uh oh!

github-actions bot commented Oct 22, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fedejeanne Oct 22, 2025

Uh oh!

fedejeanne Oct 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

CountDownLatch Synchronization for RCPTestWorkbenchAdvisor #3429

Are you sure you want to change the base?

CountDownLatch Synchronization for RCPTestWorkbenchAdvisor #3429

Conversation

vogella commented Oct 22, 2025

Uh oh!

eclipse-platform-bot commented Oct 22, 2025

Uh oh!

github-actions bot commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fedejeanne Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

fedejeanne Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Oct 22, 2025 •

edited

Loading