Skip to content

Commit 4a6da86

Browse files
committed
CountDownLatch Synchronization for RCPTestWorkbenchAdvisor
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: 1. Added latch field (line 48): - private static java.util.concurrent.CountDownLatch asyncLatch = null; 2. Initialize latch in preStartup() (line 130): - asyncLatch = new java.util.concurrent.CountDownLatch(4); — tracks 4 operations 3. 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() 4. 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
1 parent e900fff commit 4a6da86

File tree

1 file changed

+40
-5
lines changed

1 file changed

+40
-5
lines changed

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

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,9 @@ public class RCPTestWorkbenchAdvisor extends WorkbenchAdvisor {
4444

4545
private static boolean started = false;
4646

47+
// Use a CountDownLatch to ensure async operations complete before postStartup
48+
private static java.util.concurrent.CountDownLatch asyncLatch = null;
49+
4750
public static boolean isSTARTED() {
4851
synchronized (RCPTestWorkbenchAdvisor.class) {
4952
return started;
@@ -122,6 +125,10 @@ public void eventLoopIdle(final Display display) {
122125
public void preStartup() {
123126
super.preStartup();
124127
final Display display = Display.getCurrent();
128+
129+
// Initialize a latch to track completion of the 4 async/sync operations
130+
asyncLatch = new java.util.concurrent.CountDownLatch(4);
131+
125132
if (display != null) {
126133
display.asyncExec(() -> {
127134
if (isSTARTED())
@@ -169,6 +176,11 @@ public void run() {
169176
// this can happen because we shut down the workbench just
170177
// as soon as we're initialized - ie: when we're trying to
171178
// run this runnable in the deferred case.
179+
} finally {
180+
// Signal that this operation has completed
181+
if (asyncLatch != null) {
182+
asyncLatch.countDown();
183+
}
172184
}
173185
}
174186
};
@@ -183,11 +195,18 @@ public void run() {
183195
if (callDisplayAccess)
184196
DisplayAccess.accessDisplayDuringStartup();
185197
display.asyncExec(() -> {
186-
synchronized (RCPTestWorkbenchAdvisor.class) {
187-
if (callDisplayAccess)
188-
asyncWithDisplayAccess = !isSTARTED() ? Boolean.TRUE : Boolean.FALSE;
189-
else
190-
asyncWithoutDisplayAccess = !isSTARTED() ? Boolean.TRUE : Boolean.FALSE;
198+
try {
199+
synchronized (RCPTestWorkbenchAdvisor.class) {
200+
if (callDisplayAccess)
201+
asyncWithDisplayAccess = !isSTARTED() ? Boolean.TRUE : Boolean.FALSE;
202+
else
203+
asyncWithoutDisplayAccess = !isSTARTED() ? Boolean.TRUE : Boolean.FALSE;
204+
}
205+
} finally {
206+
// Signal that this operation has completed
207+
if (asyncLatch != null) {
208+
asyncLatch.countDown();
209+
}
191210
}
192211
});
193212
}
@@ -199,6 +218,22 @@ public void run() {
199218
@Override
200219
public void postStartup() {
201220
super.postStartup();
221+
222+
// Wait for all async/sync operations to complete before marking as started
223+
// This prevents a race condition where asyncWithDisplayAccess might not be set yet
224+
if (asyncLatch != null) {
225+
try {
226+
// Wait up to 5 seconds for all operations to complete
227+
boolean completed = asyncLatch.await(5, java.util.concurrent.TimeUnit.SECONDS);
228+
if (!completed) {
229+
System.err.println("Warning: Not all async/sync operations completed within timeout");
230+
}
231+
} catch (InterruptedException e) {
232+
Thread.currentThread().interrupt();
233+
System.err.println("Warning: Interrupted while waiting for async/sync operations");
234+
}
235+
}
236+
202237
synchronized (RCPTestWorkbenchAdvisor.class) {
203238
started = true;
204239
}

0 commit comments

Comments
 (0)