From 905bfa7f9cb9785689c4ac86aedd8d2978d602ea Mon Sep 17 00:00:00 2001 From: David Pascal Schmalzing Date: Wed, 2 Sep 2026 14:47:15 +0200 Subject: [PATCH 1/2] Hotfix service registry breaking configuration cache --- .../gradle/queue/CachedQueueService.java | 39 +++++++++++++--- .../gradle/queue/ICachedQueueService.java | 6 ++- .../gradle/queue/ICachedQueueTask.java | 7 ++- .../test/java/IsolatedWorkerQueueTest.java | 44 ++++++++++++++++++- .../java/se/rwth/example/ExampleTask.java | 2 +- 5 files changed, 86 insertions(+), 12 deletions(-) diff --git a/se-commons-gradle/src/main/java/de/monticore/gradle/queue/CachedQueueService.java b/se-commons-gradle/src/main/java/de/monticore/gradle/queue/CachedQueueService.java index ff2e5a0..d092ed4 100644 --- a/se-commons-gradle/src/main/java/de/monticore/gradle/queue/CachedQueueService.java +++ b/se-commons-gradle/src/main/java/de/monticore/gradle/queue/CachedQueueService.java @@ -82,14 +82,36 @@ protected void init(ServiceRegistry serviceRegistry) { logger.warn("Possibly overriding the CachedQueueService instance"); INSTANCE = this; } - this.serviceRegistry = Objects.requireNonNull(serviceRegistry); + doInit(Objects.requireNonNull(serviceRegistry)); + } + + /** + * Performs the (re-)initialization of the {@link #serviceRegistry}. + */ + protected synchronized void doInit(ServiceRegistry serviceRegistry) { + this.serviceRegistry = serviceRegistry; this.providerSelf = (Provider) serviceRegistry.get(BuildServiceRegistry.class).getRegistrations().getByName(NAME).getService(); Objects.requireNonNull(serviceRegistry.get(ActionExecutionSpecFactory.class), "ActionExecutionSpecFactory"); Objects.requireNonNull(serviceRegistry.get(IsolatableFactory.class), "isolatableFactory"); - serviceRegistry.get(BuildEventListenerRegistryInternal.class) - .onOperationCompletion(this.providerSelf); + serviceRegistry.get(BuildEventListenerRegistryInternal.class).onOperationCompletion(this.providerSelf); + } + /** + * Ensures that the {@link #serviceRegistry} is set, (re-)initializing it + * lazily from {@code currentServiceRegistry} if it is not. This may be, + * for example, because {@link #init(Gradle)} never ran for this build + * (configuration cache reuse). + */ + protected void ensureInitialized(ServiceRegistry currentServiceRegistry) { + if (this.serviceRegistry != null) { + return; + } + synchronized (this) { + if (this.serviceRegistry == null) { + doInit(Objects.requireNonNull(currentServiceRegistry, "serviceRegistry must not be null")); + } + } } /** @@ -160,7 +182,7 @@ public synchronized void setMaxConcurrentMC(int maxParallelMC) { } - protected ServiceRegistry serviceRegistry; + protected volatile ServiceRegistry serviceRegistry; protected Provider providerSelf; @Override @@ -759,15 +781,18 @@ public void markForErasure() { * Construct a new WorkQueue * * @param workerExecutor the worker executor to use + * @param currentServiceRegistry a {@link ServiceRegistry} freshly injected into the calling + * task, used to lazily (re-)initialize this service if it was + * never initialized for this build (see {@link #ensureInitialized}) * @param extraClasspathElement the classpath elements to use * @return a new {@link WorkQueue} */ - public WorkQueue newWorkQueue(WorkerExecutor workerExecutor, FileCollection extraClasspathElement) { + public WorkQueue newWorkQueue(WorkerExecutor workerExecutor, ServiceRegistry currentServiceRegistry, FileCollection extraClasspathElement) { Objects.requireNonNull(workerExecutor, "worker executor must not be null"); - Objects.requireNonNull(serviceRegistry, "serviceRegistry must not be null"); + ensureInitialized(currentServiceRegistry); return new CachedIsolatedWorkQueue(workerExecutor.noIsolation(), serviceRegistry.get(InstantiatorFactory.class), - Objects.requireNonNull(serviceRegistry, "serviceRegistry"), + serviceRegistry, this.providerSelf, extraClasspathElement); } diff --git a/se-commons-gradle/src/main/java/de/monticore/gradle/queue/ICachedQueueService.java b/se-commons-gradle/src/main/java/de/monticore/gradle/queue/ICachedQueueService.java index 5799a45..f85b697 100644 --- a/se-commons-gradle/src/main/java/de/monticore/gradle/queue/ICachedQueueService.java +++ b/se-commons-gradle/src/main/java/de/monticore/gradle/queue/ICachedQueueService.java @@ -1,6 +1,7 @@ package de.monticore.gradle.queue; import org.gradle.api.file.FileCollection; +import org.gradle.internal.service.ServiceRegistry; import org.gradle.workers.WorkQueue; import org.gradle.workers.WorkerExecutor; @@ -10,15 +11,16 @@ * @since 7.9.0 */ public interface ICachedQueueService { - + /** * Construct a new WorkQueue * * @param workerExecutor the worker executor to use + * @param serviceRegistry a {@link ServiceRegistry} injected fresh into the calling task * @param extraClasspathElement the classpath elements to use * @return a new {@link WorkQueue} */ - WorkQueue newWorkQueue(WorkerExecutor workerExecutor, FileCollection extraClasspathElement); + WorkQueue newWorkQueue(WorkerExecutor workerExecutor, ServiceRegistry serviceRegistry, FileCollection extraClasspathElement); /** * Returns the tracked stats as a serialized JSON string. diff --git a/se-commons-gradle/src/main/java/de/monticore/gradle/queue/ICachedQueueTask.java b/se-commons-gradle/src/main/java/de/monticore/gradle/queue/ICachedQueueTask.java index ba86076..319c398 100644 --- a/se-commons-gradle/src/main/java/de/monticore/gradle/queue/ICachedQueueTask.java +++ b/se-commons-gradle/src/main/java/de/monticore/gradle/queue/ICachedQueueTask.java @@ -4,7 +4,9 @@ import org.gradle.api.Task; import org.gradle.api.provider.Property; import org.gradle.api.tasks.Internal; +import org.gradle.internal.service.ServiceRegistry; +import javax.inject.Inject; import java.lang.reflect.Proxy; /** @@ -12,10 +14,13 @@ * Automatically managed via the {@link CachedQueueServicePlugin} */ public interface ICachedQueueTask extends Task { - + // Type must be object due to https://github.com/gradle/gradle/issues/17559 @Internal Property getSharedQueueServiceProperty(); + + @Inject + ServiceRegistry getServiceRegistry(); @Internal @Deprecated(forRemoval = true) diff --git a/se-commons-gradle/src/test/java/IsolatedWorkerQueueTest.java b/se-commons-gradle/src/test/java/IsolatedWorkerQueueTest.java index 494b02b..ddf9b5c 100644 --- a/se-commons-gradle/src/test/java/IsolatedWorkerQueueTest.java +++ b/se-commons-gradle/src/test/java/IsolatedWorkerQueueTest.java @@ -1,10 +1,13 @@ /* (c) https://github.com/MontiCore/monticore */ +import com.google.common.base.Preconditions; import org.apache.commons.lang3.StringUtils; import org.gradle.testkit.runner.BuildResult; import org.gradle.testkit.runner.GradleRunner; import org.gradle.testkit.runner.TaskOutcome; import org.gradle.testkit.runner.UnexpectedBuildFailure; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.api.parallel.ResourceLock; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -15,6 +18,8 @@ import java.util.List; import java.util.stream.Collectors; +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; import static org.junit.jupiter.api.Assertions.*; /** @@ -25,7 +30,10 @@ */ public class IsolatedWorkerQueueTest { + private static final String GRADLE = "gradle"; + @Test + @ResourceLock(GRADLE) public void testNoIsolation() throws Exception { File projectDir = new File("build/functionalTest/noi"); projectDir.mkdirs(); @@ -59,6 +67,7 @@ public void testNoIsolation() throws Exception { @Test + @ResourceLock(GRADLE) public void testCLIsolation() throws Exception { File projectDir = new File("build/functionalTest/cl"); projectDir.mkdirs(); @@ -94,6 +103,7 @@ public void testCLIsolation() throws Exception { @ParameterizedTest @ValueSource(strings = {"8.5", "8.7", "8.14", "9.3.1","9.5.1", "9.6.1"}) + @ResourceLock(GRADLE) public void testSharedIsolation(String version) throws Exception { File projectDir = new File("build/functionalTest/shared/" + version); projectDir.mkdirs(); @@ -134,8 +144,40 @@ public void testSharedIsolation(String version) throws Exception { assertEquals(4, StringUtils.countMatches(json, "\"START\"")); assertEquals(4, StringUtils.countMatches(json, "\"DONE\"")); } - + + @Test + @ResourceLock(GRADLE) + public void testSharedIsolationWithConfigurationCacheReuse(@TempDir File projectDir) throws Exception { + checkState(new File(projectDir, "settings.gradle").createNewFile()); + write(new File(projectDir, "gradle.properties"), "org.gradle.configuration-cache=true\n"); + write(new File(projectDir, "build.gradle"), + """ + plugins { + id 'se.rwth.example' + } + import se.rwth.example.ExampleTask + tasks.register('A', ExampleTask.class) + """ + ); + + // First run: cold configuration cache, i.e. Settings/plugin apply() are run + BuildResult first = GradleRunner.create() + .withProjectDir(projectDir).withPluginClasspath() + .withArguments("A", "--stacktrace").build(); + assertEquals(TaskOutcome.SUCCESS, checkNotNull(first.task(":A")).getOutcome()); + assertTrue(first.getOutput().contains("Configuration cache entry stored."), first.getOutput()); + + // Second run: force task A to re-execute (as if an input changed) while the configuration + // cache is reused, i.e. Settings/plugin apply() does NOT run again. + BuildResult second = GradleRunner.create() + .withProjectDir(projectDir).withPluginClasspath() + .withArguments("A", "--rerun-tasks", "--stacktrace").build(); + assertTrue(second.getOutput().contains("Reusing configuration cache."), second.getOutput()); + assertEquals(TaskOutcome.SUCCESS, checkNotNull(second.task(":A")).getOutcome()); + } + @Test + @ResourceLock(GRADLE) public void testSharedBuildService( ) throws Exception { File projectDir = new File("build/functionalTest/shared_bs/"); projectDir.mkdirs(); diff --git a/se-commons-gradle/src/testPlugin/java/se/rwth/example/ExampleTask.java b/se-commons-gradle/src/testPlugin/java/se/rwth/example/ExampleTask.java index 74fda27..3cd9f9c 100644 --- a/se-commons-gradle/src/testPlugin/java/se/rwth/example/ExampleTask.java +++ b/se-commons-gradle/src/testPlugin/java/se/rwth/example/ExampleTask.java @@ -49,7 +49,7 @@ public void execute() { queue = getWorkerExecutor().classLoaderIsolation(); break; case SHARED: - queue = doGetSharedQueueService().newWorkQueue(getWorkerExecutor(), getProject().getObjects().fileCollection()); + queue = doGetSharedQueueService().newWorkQueue(getWorkerExecutor(), getServiceRegistry(), getProject().getObjects().fileCollection()); break; default: throw new IllegalStateException("Unknown worker kind: " + getWorkerKind().get()); From 2992d7a9759b5beb19991ba3f9f60561450cc118 Mon Sep 17 00:00:00 2001 From: David Pascal Schmalzing Date: Wed, 2 Sep 2026 15:19:02 +0200 Subject: [PATCH 2/2] Fix breaking api, readd old workqueue --- .../gradle/queue/CachedQueueService.java | 17 +++++++++++++++++ .../gradle/queue/ICachedQueueService.java | 19 +++++++++++++++++-- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/se-commons-gradle/src/main/java/de/monticore/gradle/queue/CachedQueueService.java b/se-commons-gradle/src/main/java/de/monticore/gradle/queue/CachedQueueService.java index d092ed4..8683754 100644 --- a/se-commons-gradle/src/main/java/de/monticore/gradle/queue/CachedQueueService.java +++ b/se-commons-gradle/src/main/java/de/monticore/gradle/queue/CachedQueueService.java @@ -777,6 +777,22 @@ public void markForErasure() { } + /** + * @deprecated does not lazily (re-)initialize this service - see + * {@link ICachedQueueService#newWorkQueue(WorkerExecutor, ServiceRegistry, FileCollection)}. + */ + @Deprecated + @Override + public WorkQueue newWorkQueue(WorkerExecutor workerExecutor, FileCollection extraClasspathElement) { + Objects.requireNonNull(workerExecutor, "worker executor must not be null"); + Objects.requireNonNull(serviceRegistry, "serviceRegistry must not be null"); + return new CachedIsolatedWorkQueue(workerExecutor.noIsolation(), + serviceRegistry.get(InstantiatorFactory.class), + serviceRegistry, + this.providerSelf, + extraClasspathElement); + } + /** * Construct a new WorkQueue * @@ -787,6 +803,7 @@ public void markForErasure() { * @param extraClasspathElement the classpath elements to use * @return a new {@link WorkQueue} */ + @Override public WorkQueue newWorkQueue(WorkerExecutor workerExecutor, ServiceRegistry currentServiceRegistry, FileCollection extraClasspathElement) { Objects.requireNonNull(workerExecutor, "worker executor must not be null"); ensureInitialized(currentServiceRegistry); diff --git a/se-commons-gradle/src/main/java/de/monticore/gradle/queue/ICachedQueueService.java b/se-commons-gradle/src/main/java/de/monticore/gradle/queue/ICachedQueueService.java index f85b697..db27ef4 100644 --- a/se-commons-gradle/src/main/java/de/monticore/gradle/queue/ICachedQueueService.java +++ b/se-commons-gradle/src/main/java/de/monticore/gradle/queue/ICachedQueueService.java @@ -13,7 +13,22 @@ public interface ICachedQueueService { /** - * Construct a new WorkQueue + * Construct a new WorkQueue. + * + * @param workerExecutor the worker executor to use + * @param extraClasspathElement the classpath elements to use + * @return a new {@link WorkQueue} + * @deprecated May break with configuration-cache reuse. Prefer + * {@link #newWorkQueue(WorkerExecutor, ServiceRegistry, FileCollection)}, + * which can (re-)initialize the service registry. + */ + @Deprecated + WorkQueue newWorkQueue(WorkerExecutor workerExecutor, FileCollection extraClasspathElement); + + /** + * Construct a new WorkQueue, (re-)initializing this service from {@code serviceRegistry} first + * if it was not already initialized - e.g. because the configuration cache was reused and the + * one-time settings-plugin initialization did not run for this build. * * @param workerExecutor the worker executor to use * @param serviceRegistry a {@link ServiceRegistry} injected fresh into the calling task @@ -21,7 +36,7 @@ public interface ICachedQueueService { * @return a new {@link WorkQueue} */ WorkQueue newWorkQueue(WorkerExecutor workerExecutor, ServiceRegistry serviceRegistry, FileCollection extraClasspathElement); - + /** * Returns the tracked stats as a serialized JSON string. *