Fix race condition in RCPTestWorkbenchAdvisor test harness

Introduce a Phaser to ensure async/sync operations are scheduled
before postStartup marks the workbench as started. This prevents
timing issues where test variables (asyncWithDisplayAccess,
syncWithDisplayAccess, etc.) might not be set correctly due to
the workbench being marked as started before async runnables execute.

Changes:
- Add Phaser to coordinate async/sync thread operations
- Register/deregister threads when scheduling display operations
- Wait for all operations to be scheduled in postStartup (with 5s timeout)
- Process event loop to ensure async runnables execute before marking started
- Simplify Boolean assignments (remove unnecessary ternary operators)

This ensures deterministic behavior in test harness initialization
and prevents flaky test failures due to race conditions.

Author: vogella

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

@@ -13,6 +13,10 @@
  *******************************************************************************/
 package org.eclipse.ui.tests.harness.util;
 
+import java.util.concurrent.Phaser;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+
 import org.eclipse.jface.preference.IPreferenceStore;
 import org.eclipse.swt.SWTException;
 import org.eclipse.swt.widgets.Display;
@@ -44,6 +48,10 @@ public class RCPTestWorkbenchAdvisor extends WorkbenchAdvisor {
 
 	private static boolean started = false;
 
+	// Use a Phaser to ensure async operations are scheduled before postStartup
+	// Each thread registers itself and arrives when the operation is scheduled
+	private static final Phaser asyncPhaser = new Phaser(1); // 1 for the main thread
+
 	public static boolean isSTARTED() {
 		synchronized (RCPTestWorkbenchAdvisor.class) {
 			return started;
@@ -122,12 +130,10 @@ public void eventLoopIdle(final Display display) {
 	public void preStartup() {
 		super.preStartup();
 		final Display display = Display.getCurrent();
+
 		if (display != null) {
 			display.asyncExec(() -> {
-				if (isSTARTED())
-					asyncDuringStartup = Boolean.FALSE;
-				else
-					asyncDuringStartup = Boolean.TRUE;
+				asyncDuringStartup = !isSTARTED();
 			});
 		}
 
@@ -151,24 +157,28 @@ public void preStartup() {
 	}
 
 	private void setupSyncDisplayThread(final boolean callDisplayAccess, final Display display) {
+		// Register this thread with the phaser
+		asyncPhaser.register();
 		Thread syncThread = new Thread() {
 			@Override
 			public void run() {
 				if (callDisplayAccess)
 					DisplayAccess.accessDisplayDuringStartup();
 				try {
 					display.syncExec(() -> {
-						synchronized (RCPTestWorkbenchAdvisor.class) {
-							if (callDisplayAccess)
-								syncWithDisplayAccess = !isSTARTED() ? Boolean.TRUE : Boolean.FALSE;
-							else
-								syncWithoutDisplayAccess = !isSTARTED() ? Boolean.TRUE : Boolean.FALSE;
+						if (callDisplayAccess) {
+							syncWithDisplayAccess = !isSTARTED();
+						} else {
+							syncWithoutDisplayAccess = !isSTARTED();
 						}
 					});
 				} catch (SWTException e) {
 					// this can happen because we shut down the workbench just
 					// as soon as we're initialized - ie: when we're trying to
 					// run this runnable in the deferred case.
+				} finally {
+					// Signal that this operation has completed
+					asyncPhaser.arriveAndDeregister();
 				}
 			}
 		};
@@ -177,19 +187,26 @@ public void run() {
 	}
 
 	private void setupAsyncDisplayThread(final boolean callDisplayAccess, final Display display) {
+		// Register this thread with the phaser
+		asyncPhaser.register();
 		Thread asyncThread = new Thread() {
 			@Override
 			public void run() {
 				if (callDisplayAccess)
 					DisplayAccess.accessDisplayDuringStartup();
-				display.asyncExec(() -> {
-					synchronized (RCPTestWorkbenchAdvisor.class) {
-						if (callDisplayAccess)
-							asyncWithDisplayAccess = !isSTARTED() ? Boolean.TRUE : Boolean.FALSE;
-						else
-							asyncWithoutDisplayAccess = !isSTARTED() ? Boolean.TRUE : Boolean.FALSE;
-					}
-				});
+				try {
+					display.asyncExec(() -> {
+						if (callDisplayAccess) {
+							asyncWithDisplayAccess = !isSTARTED();
+						} else {
+							asyncWithoutDisplayAccess = !isSTARTED();
+						}
+					});
+				} finally {
+					// Signal that this operation has been scheduled (not necessarily executed yet)
+					// This avoids deadlock since we're not waiting for execution on the UI thread
+					asyncPhaser.arriveAndDeregister();
+				}
 			}
 		};
 		asyncThread.setDaemon(true);
@@ -199,6 +216,29 @@ public void run() {
 	@Override
 	public void postStartup() {
 		super.postStartup();
+
+		// Wait for all async/sync operations to be scheduled (not blocking UI thread)
+		try {
+			// Wait up to 5 seconds for all operations to be scheduled
+			// The main thread arrives and deregisters, waiting for all other registered threads
+			asyncPhaser.awaitAdvanceInterruptibly(asyncPhaser.arrive(), 5, TimeUnit.SECONDS);
+		} catch (TimeoutException e) {
+			// Log warning but don't throw - we need to mark as started to avoid breaking subsequent tests
+			System.err.println("WARNING: Not all async/sync operations were scheduled within timeout");
+			e.printStackTrace();
+		} catch (InterruptedException e) {
+			Thread.currentThread().interrupt();
+			System.err.println("WARNING: Interrupted while waiting for async/sync operations");
+		}
+
+		// Pump the event loop to ensure async runnables execute before marking as started
+		// This prevents the original race condition where async variables might not be set yet
+		// Wait until the variables that should be set during startup are actually set to TRUE
+		UITestUtil.processEventsUntil(() -> Boolean.TRUE.equals(syncWithDisplayAccess) && Boolean.TRUE.equals(asyncWithDisplayAccess), 5000);
+		// Process any remaining events to allow variables that should NOT be set during startup
+		// to accidentally execute (to detect regression)
+		UITestUtil.processEvents();
+
 		synchronized (RCPTestWorkbenchAdvisor.class) {
 			started = true;
 		}