From 7662d51632d536b3b7684aa9cd5e68d75a6a2516 Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Thu, 6 Aug 2026 15:48:25 +0200 Subject: [PATCH] Stop ParallelBuildChainTest failures from cascading through the class TimerBuilder considered its builds done once the number of recorded FINISH events reached the expected number of builds. That count is unreachable whenever fewer builds ran than expected, for example when a test cancels a partly finished build on purpose, so abortCurrentBuilds burned its full 60 second timeout. setExpectedNumberOfBuilds then asserted before replacing the tracked state, so the throw left the unusable state installed and every following test in the class failed with "builds are still running while resetting TimerBuilder", each one hanging for another minute. Completion is now tracked by the number of currently running builders, which the builder itself always decrements, and the execution state is replaced before it is checked, so a leaked build can no longer affect any later test. tearDown resets unconditionally and reports the leak against the test that caused it. The builder also waited by spinning in TestUtil.waitForCondition. With 15 long running builds that kept 15 threads busy for up to 40 seconds and starved the job scheduler on CI machines, so queued builds did not start within the timeout. It now blocks on a monitor instead, and each build binds to the state of its own test so a leaked build cannot contribute events to the next one. Fixes https://github.com/eclipse-platform/eclipse.platform/issues/825 --- .../builders/ParallelBuildChainTest.java | 17 ++- .../tests/internal/builders/TimerBuilder.java | 103 +++++++++++++----- 2 files changed, 88 insertions(+), 32 deletions(-) diff --git a/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/internal/builders/ParallelBuildChainTest.java b/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/internal/builders/ParallelBuildChainTest.java index 1d9c71a616f..ef6088f389b 100644 --- a/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/internal/builders/ParallelBuildChainTest.java +++ b/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/internal/builders/ParallelBuildChainTest.java @@ -22,6 +22,7 @@ import static org.eclipse.core.tests.resources.ResourceTestUtil.updateProjectDescription; import static org.eclipse.core.tests.resources.ResourceTestUtil.waitForBuild; import static org.eclipse.core.tests.resources.TestUtil.waitForCondition; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.ArrayList; @@ -103,10 +104,18 @@ public void setUp() throws Exception { @AfterEach public void tearDown() throws Exception { - // Cleanup workspace first to ensure that auto-build is not started on projects - waitForBuild(); - getWorkspace().getRoot().delete(true, true, createTestMonitor()); - TimerBuilder.abortCurrentBuilds(); + boolean buildsStillRunning; + try { + // Cleanup workspace first to ensure that auto-build is not started on projects + waitForBuild(); + getWorkspace().getRoot().delete(true, true, createTestMonitor()); + } finally { + TimerBuilder.abortCurrentBuilds(); + // Reset unconditionally, so that a build leaked by this test does not affect + // any subsequent test + buildsStillRunning = TimerBuilder.reset(); + } + assertFalse(buildsStillRunning, "builds are still running at the end of the test"); } private void setWorkspaceMaxNumberOfConcurrentBuilds(int maximumNumberOfConcurrentBuilds) throws CoreException { diff --git a/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/internal/builders/TimerBuilder.java b/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/internal/builders/TimerBuilder.java index aacab67bacc..56b3e6a65a6 100644 --- a/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/internal/builders/TimerBuilder.java +++ b/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/internal/builders/TimerBuilder.java @@ -13,14 +13,13 @@ *******************************************************************************/ package org.eclipse.core.tests.internal.builders; -import static org.eclipse.core.tests.resources.TestUtil.waitForCondition; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.function.BooleanSupplier; import org.eclipse.core.resources.IProject; import org.eclipse.core.resources.IncrementalProjectBuilder; import org.eclipse.core.resources.ResourcesPlugin; @@ -33,28 +32,39 @@ public class TimerBuilder extends IncrementalProjectBuilder { public static final String DURATION_ARG = "duration"; public static final String RULE_TYPE_ARG = "ruleType"; - private static final int SHUTDOWN_TIMEOUT_IN_MILLIS = 60_000; + private static final int SHUTDOWN_TIMEOUT_IN_MILLIS = 10_000; - private static BuildExecutionState executionState = new BuildExecutionState(-1); + private static volatile BuildExecutionState executionState = new BuildExecutionState(-1); + /** + * Tracks the builds of a single test. All state is guarded by the instance + * monitor, which is also used to signal running builds to abort. + */ private static class BuildExecutionState { private final int expectedNumberOfBuilds; - private final List events = Collections.synchronizedList(new ArrayList<>()); - private volatile boolean shallAbort = false; - private volatile int maxSimultaneousBuilds = 0; - private volatile int currentlyRunningBuilds = 0; + private final List events = new ArrayList<>(); + private boolean shallAbort; + private int maxSimultaneousBuilds; + private int currentlyRunningBuilds; private BuildExecutionState(int expectedNumberOfBuilds) { this.expectedNumberOfBuilds = expectedNumberOfBuilds; } - private synchronized boolean isExecuting() { - return getProjectBuilds(BuildEventType.FINISH).size() < executionState.expectedNumberOfBuilds; + private synchronized boolean hasRunningBuilds() { + return currentlyRunningBuilds > 0; + } + + private synchronized int getMaxSimultaneousBuilds() { + return maxSimultaneousBuilds; + } + + private synchronized List getEvents() { + return new ArrayList<>(events); } private synchronized List getProjectBuilds(BuildEventType eventType) { - return events.stream().filter(event -> event.eventType == eventType) - .map(event -> event.project).toList(); + return events.stream().filter(event -> event.eventType == eventType).map(event -> event.project).toList(); } private synchronized void startedExecutingProject(IProject project) { @@ -63,22 +73,40 @@ private synchronized void startedExecutingProject(IProject project) { events.add(new BuildEvent(project, BuildEventType.START)); } - private synchronized void endedExcecutingProject(IProject project) { + private synchronized void endedExecutingProject(IProject project) { currentlyRunningBuilds--; events.add(new BuildEvent(project, BuildEventType.FINISH)); notifyAll(); } + /** + * Blocks until the builds shall abort or the given duration has elapsed. + */ + private synchronized void awaitBuildDuration(long durationInMillis) { + awaitWithTimeout(durationInMillis, () -> shallAbort); + } + + /** + * Requests all running builds to abort and waits for their termination. + */ private synchronized void abortAndWaitForAllBuilds() { shallAbort = true; - long durationInMillis = 0; - long waitingStartTimeInMillis = System.currentTimeMillis(); - while (isExecuting() && durationInMillis < SHUTDOWN_TIMEOUT_IN_MILLIS) { + notifyAll(); + awaitWithTimeout(SHUTDOWN_TIMEOUT_IN_MILLIS, () -> currentlyRunningBuilds == 0); + } + + private synchronized void awaitWithTimeout(long timeoutInMillis, BooleanSupplier condition) { + long deadlineInNanos = System.nanoTime() + timeoutInMillis * 1_000_000L; + long remainingMillis = timeoutInMillis; + while (!condition.getAsBoolean() && remainingMillis > 0) { try { - wait(SHUTDOWN_TIMEOUT_IN_MILLIS); + wait(remainingMillis); } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + return; } - durationInMillis = System.currentTimeMillis() - waitingStartTimeInMillis; + // Round up, so that the requested duration is never undercut + remainingMillis = (deadlineInNanos - System.nanoTime() + 999_999) / 1_000_000L; } } @@ -138,15 +166,18 @@ public boolean contains(ISchedulingRule rule) { @Override protected IProject[] build(int kind, Map args, IProgressMonitor monitor) throws CoreException { - assertNotEquals(-1, executionState.expectedNumberOfBuilds, "no expected number of builds has been set"); - executionState.startedExecutingProject(getProject()); + // Bind to the state of the current test, so that a leaked build does not + // contribute events to a subsequent test + BuildExecutionState state = executionState; + assertNotEquals(-1, state.expectedNumberOfBuilds, "no expected number of builds has been set"); + state.startedExecutingProject(getProject()); try { - int durationInMillis = Integer.parseInt(args.get(DURATION_ARG)); - waitForCondition(() -> executionState.shallAbort, durationInMillis); + state.awaitBuildDuration(Integer.parseInt(args.get(DURATION_ARG))); } catch (Exception ex) { ex.printStackTrace(); + } finally { + state.endedExecutingProject(getProject()); } - executionState.endedExcecutingProject(getProject()); return new IProject[] {getProject()}; } @@ -177,20 +208,36 @@ public static List getFinishedProjectBuilds() { } public static int getMaximumNumberOfSimultaneousBuilds() { - return executionState.maxSimultaneousBuilds; + return executionState.getMaxSimultaneousBuilds(); } public static Iterable getBuildEvents() { - return new ArrayList<>(executionState.events); + return executionState.getEvents(); } /** - * Resets the tracked execution states. Asserts that no execution is still - * running. + * Resets the tracked execution states and defines the number of builds expected + * to be executed. Asserts that no execution is still running. */ public static void setExpectedNumberOfBuilds(int expectedNumberOfBuilds) { - assertFalse(executionState.isExecuting(), "builds are still running while resetting TimerBuilder"); + assertFalse(replaceExecutionState(expectedNumberOfBuilds), + "builds are still running while resetting TimerBuilder"); + } + + /** + * Resets the tracked execution states and returns whether builds were still + * running. + */ + public static boolean reset() { + return replaceExecutionState(-1); + } + + private static boolean replaceExecutionState(int expectedNumberOfBuilds) { + BuildExecutionState previousState = executionState; + // Replace the state before checking it, so that a leaked build does not make + // all subsequent tests fail as well executionState = new BuildExecutionState(expectedNumberOfBuilds); + return previousState.hasRunningBuilds(); } public static void abortCurrentBuilds() {