Prevent memory leaks across builds when using worker API

subprojects/core/src/main/java/org/gradle/api/tasks/JavaExec.java

@@ -29,6 +29,7 @@
 import org.gradle.process.JavaExecSpec;
 import org.gradle.process.JavaForkOptions;
 import org.gradle.process.ProcessForkOptions;
+import org.gradle.process.internal.DslExecActionFactory;
 import org.gradle.process.internal.ExecActionFactory;
 import org.gradle.process.internal.JavaExecAction;
 
@@ -100,14 +101,19 @@ public class JavaExec extends ConventionTask implements JavaExecSpec {
     private final JavaExecAction javaExecHandleBuilder;
 
     public JavaExec() {
-        javaExecHandleBuilder = getExecActionFactory().newJavaExecAction();
+        javaExecHandleBuilder = getDslExecActionFactory().newDecoratedJavaExecAction();
     }
 
     @Inject
     protected ExecActionFactory getExecActionFactory() {
         throw new UnsupportedOperationException();
     }
 
+    @Inject
+    protected DslExecActionFactory getDslExecActionFactory() {
+        throw new UnsupportedOperationException();
+    }
+
     @TaskAction
     public void exec() {
         setMain(getMain()); // make convention mapping work (at least for 'main'...

subprojects/core/src/main/java/org/gradle/process/internal/DefaultExecActionFactory.java

@@ -92,14 +92,21 @@ public ExecAction newExecAction() {
         return new DefaultExecAction(fileResolver, executor, buildCancellationToken);
     }
 
+    @Override
+    public JavaForkOptionsInternal newDecoratedJavaForkOptions() {
+        throw new UnsupportedOperationException();
+    }
+
     @Override
     public JavaForkOptionsInternal newJavaForkOptions() {
         return new DefaultJavaForkOptions(fileResolver, fileCollectionFactory, new DefaultJavaDebugOptions());
     }
 
     @Override
     public JavaForkOptionsInternal immutableCopy(JavaForkOptionsInternal options) {
-        JavaForkOptionsInternal copy = newJavaForkOptions();
+        // NOTE: We do not want/need a decorated version of JavaForkOptions or JavaDebugOptions because
+        // these immutable instances are held across builds and will retain classloaders/services in the decorated object
+        JavaForkOptionsInternal copy = new DefaultJavaForkOptions(fileResolver, new DefaultFileCollectionFactory(fileResolver, null), new DefaultJavaDebugOptions());
         options.copyTo(copy);
         return new ImmutableJavaForkOptions(copy);
     }
@@ -111,7 +118,7 @@ public JavaExecAction newDecoratedJavaExecAction() {
 
     @Override
     public JavaExecAction newJavaExecAction() {
-        return new DefaultJavaExecAction(fileResolver, fileCollectionFactory, executor, buildCancellationToken, this);
+        return new DefaultJavaExecAction(fileResolver, fileCollectionFactory, executor, buildCancellationToken, newJavaForkOptions());
     }
 
     @Override
@@ -121,7 +128,7 @@ public ExecHandleBuilder newExec() {
 
     @Override
     public JavaExecHandleBuilder newJavaExec() {
-        return new JavaExecHandleBuilder(fileResolver, fileCollectionFactory, executor, buildCancellationToken, this);
+        return new JavaExecHandleBuilder(fileResolver, fileCollectionFactory, executor, buildCancellationToken, newJavaForkOptions());
     }
 
     @Override
@@ -166,11 +173,11 @@ public ExecAction newDecoratedExecAction() {
 
         @Override
         public JavaExecAction newDecoratedJavaExecAction() {
-            return instantiator.newInstance(DefaultJavaExecAction.class, fileResolver, fileCollectionFactory, executor, buildCancellationToken, this);
+            return instantiator.newInstance(DefaultJavaExecAction.class, fileResolver, fileCollectionFactory, executor, buildCancellationToken, newDecoratedJavaForkOptions());
         }
 
         @Override
-        public JavaForkOptionsInternal newJavaForkOptions() {
+        public JavaForkOptionsInternal newDecoratedJavaForkOptions() {
             JavaDebugOptions javaDebugOptions = objectFactory.newInstance(DefaultJavaDebugOptions.class, objectFactory);
             return instantiator.newInstance(DefaultJavaForkOptions.class, fileResolver, fileCollectionFactory, javaDebugOptions);
         }

subprojects/core/src/main/java/org/gradle/process/internal/DefaultJavaExecAction.java

@@ -20,15 +20,16 @@
 import org.gradle.api.internal.file.FileResolver;
 import org.gradle.initialization.BuildCancellationToken;
 import org.gradle.process.ExecResult;
+import org.gradle.process.JavaForkOptions;
 
 import java.util.concurrent.Executor;
 
 /**
  * Use {@link ExecActionFactory} or {@link DslExecActionFactory} instead.
  */
 public class DefaultJavaExecAction extends JavaExecHandleBuilder implements JavaExecAction {
-    public DefaultJavaExecAction(FileResolver fileResolver, FileCollectionFactory fileCollectionFactory, Executor executor, BuildCancellationToken buildCancellationToken, JavaForkOptionsFactory javaForkOptionsFactory) {
-        super(fileResolver, fileCollectionFactory, executor, buildCancellationToken, javaForkOptionsFactory);
+    public DefaultJavaExecAction(FileResolver fileResolver, FileCollectionFactory fileCollectionFactory, Executor executor, BuildCancellationToken buildCancellationToken, JavaForkOptions javaOptions) {
+        super(fileResolver, fileCollectionFactory, executor, buildCancellationToken, javaOptions);
     }
 
     @Override

subprojects/core/src/main/java/org/gradle/process/internal/JavaExecHandleBuilder.java

@@ -48,10 +48,10 @@ public class JavaExecHandleBuilder extends AbstractExecHandleBuilder implements
     private final JavaForkOptions javaOptions;
     private final List<CommandLineArgumentProvider> argumentProviders = new ArrayList<CommandLineArgumentProvider>();
 
-    public JavaExecHandleBuilder(FileResolver fileResolver, FileCollectionFactory fileCollectionFactory, Executor executor, BuildCancellationToken buildCancellationToken, JavaForkOptionsFactory javaForkOptionsFactory) {
+    public JavaExecHandleBuilder(FileResolver fileResolver, FileCollectionFactory fileCollectionFactory, Executor executor, BuildCancellationToken buildCancellationToken, JavaForkOptions javaOptions) {
         super(fileResolver, executor, buildCancellationToken);
         this.fileCollectionFactory = fileCollectionFactory;
-        this.javaOptions = javaForkOptionsFactory.newJavaForkOptions();
+        this.javaOptions = javaOptions;
         executable(javaOptions.getExecutable());
     }
 

subprojects/core/src/main/java/org/gradle/process/internal/JavaForkOptionsFactory.java

@@ -17,6 +17,7 @@
 package org.gradle.process.internal;
 
 public interface JavaForkOptionsFactory {
+    JavaForkOptionsInternal newDecoratedJavaForkOptions();
     JavaForkOptionsInternal newJavaForkOptions();
 
     JavaForkOptionsInternal immutableCopy(JavaForkOptionsInternal options);

subprojects/core/src/main/java/org/gradle/process/internal/JvmOptions.java

@@ -251,26 +251,26 @@ public void systemProperty(String name, Object value) {
     }
 
     public FileCollection getBootstrapClasspath() {
-        return internalGetBootstrapCLasspath();
+        return internalGetBootstrapClasspath();
     }
 
-    private ConfigurableFileCollection internalGetBootstrapCLasspath() {
+    private ConfigurableFileCollection internalGetBootstrapClasspath() {
         if (bootstrapClasspath == null) {
             bootstrapClasspath = fileCollectionFactory.configurableFiles("bootstrap classpath");
         }
         return bootstrapClasspath;
     }
 
     public void setBootstrapClasspath(FileCollection classpath) {
-        internalGetBootstrapCLasspath().setFrom(classpath);
+        internalGetBootstrapClasspath().setFrom(classpath);
     }
 
     public void setBootstrapClasspath(Object... classpath) {
-        internalGetBootstrapCLasspath().setFrom(classpath);
+        internalGetBootstrapClasspath().setFrom(classpath);
     }
 
     public void bootstrapClasspath(Object... classpath) {
-        internalGetBootstrapCLasspath().from(classpath);
+        internalGetBootstrapClasspath().from(classpath);
     }
 
     public String getMinHeapSize() {
@@ -322,7 +322,7 @@ public void copyTo(JavaForkOptions target) {
         target.setSystemProperties(mutableSystemProperties);
         target.setMinHeapSize(minHeapSize);
         target.setMaxHeapSize(maxHeapSize);
-        target.setBootstrapClasspath(getBootstrapClasspath());
+        target.bootstrapClasspath(getBootstrapClasspath().getFiles());
         target.setEnableAssertions(assertionsEnabled);
         copyDebugOptionsTo(target.getDebugOptions());
         target.systemProperties(immutableSystemProperties);
@@ -335,7 +335,7 @@ public JvmOptions createCopy() {
         target.setMinHeapSize(minHeapSize);
         target.setMaxHeapSize(maxHeapSize);
         if (bootstrapClasspath != null) {
-            target.setBootstrapClasspath(getBootstrapClasspath());
+            target.setBootstrapClasspath(getBootstrapClasspath().getFiles());
         }
         target.setEnableAssertions(assertionsEnabled);
         copyDebugOptionsTo(target.getDebugOptions());

subprojects/core/src/test/groovy/org/gradle/api/internal/tasks/util/DefaultJavaForkOptionsTest.groovy

@@ -355,7 +355,7 @@ class DefaultJavaForkOptionsTest extends Specification {
         1 * target.setSystemProperties([key: 12])
         1 * target.setMinHeapSize('64m')
         1 * target.setMaxHeapSize('1g')
-        1 * target.setBootstrapClasspath(options.bootstrapClasspath)
+        1 * target.bootstrapClasspath(_)
         1 * target.setEnableAssertions(false)
         1 * target.getDebugOptions() >> new DefaultJavaDebugOptions()
 

subprojects/core/src/test/groovy/org/gradle/process/internal/JavaExecHandleBuilderTest.groovy

@@ -30,7 +30,7 @@ import java.util.concurrent.Executor
 import static java.util.Arrays.asList
 
 class JavaExecHandleBuilderTest extends Specification {
-    JavaExecHandleBuilder builder = new JavaExecHandleBuilder(TestFiles.resolver(), TestFiles.fileCollectionFactory(), Mock(Executor), new DefaultBuildCancellationToken(), TestFiles.execFactory())
+    JavaExecHandleBuilder builder = new JavaExecHandleBuilder(TestFiles.resolver(), TestFiles.fileCollectionFactory(), Mock(Executor), new DefaultBuildCancellationToken(), TestFiles.execFactory().newJavaForkOptions())
 
     FileCollectionFactory fileCollectionFactory = new DefaultFileCollectionFactory()
 

subprojects/testing-jvm/src/main/java/org/gradle/api/tasks/testing/Test.java

@@ -152,7 +152,7 @@ public class Test extends AbstractTestTask implements JavaForkOptions, PatternFi
 
     public Test() {
         patternSet = getFileResolver().getPatternSetFactory().create();
-        forkOptions = getForkOptionsFactory().newJavaForkOptions();
+        forkOptions = getForkOptionsFactory().newDecoratedJavaForkOptions();
         forkOptions.setEnableAssertions(true);
     }
 

subprojects/workers/src/integTest/groovy/org/gradle/workers/internal/WorkerExecutorIntegrationTest.groovy

@@ -190,6 +190,37 @@ class WorkerExecutorIntegrationTest extends AbstractWorkerExecutorIntegrationTes
         assertDifferentDaemonsWereUsed("runInDaemon", "startNewDaemon")
     }
 
+    @Issue("https://github.com/gradle/gradle/issues/10411")
+    def "does not leak project state across multiple builds"() {
+        fixture.withWorkActionClassInBuildSrc()
+        executer.withBuildJvmOpts('-Xms256m', '-Xmx512m').requireIsolatedDaemons().requireDaemon()
+
+        buildFile << """
+            ext.memoryHog = new byte[1024*1024*150] // ~150MB
+
+            tasks.withType(WorkerTask) { task ->
+                isolationMode = IsolationMode.PROCESS
+                // Force a new daemon to be used
+                additionalForkOptions = {
+                    it.systemProperty("foobar", task.name)
+                }
+            }
+            task startDaemon1(type: WorkerTask)
+            task startDaemon2(type: WorkerTask)
+            task startDaemon3(type: WorkerTask)
+        """
+
+        when:
+        succeeds("startDaemon1")
+        succeeds("startDaemon2")
+        succeeds("startDaemon3")
+
+        then:
+        assertDifferentDaemonsWereUsed("startDaemon1", "startDaemon2")
+        assertDifferentDaemonsWereUsed("startDaemon2", "startDaemon3")
+        assertDifferentDaemonsWereUsed("startDaemon1", "startDaemon3")
+    }
+
     def "starts a new worker daemon when there are no idle compatible worker daemons available"() {
         blockingServer.start()
         blockingServer.expectConcurrent("runInDaemon", "startNewDaemon")

subprojects/workers/src/integTest/groovy/org/gradle/workers/internal/WorkerExecutorLegacyApiIntegrationTest.groovy

@@ -217,6 +217,43 @@ class WorkerExecutorLegacyApiIntegrationTest extends AbstractIntegrationSpec {
         isolationMode << ISOLATION_MODES
     }
 
+    @Issue("https://github.com/gradle/gradle/issues/10411")
+    def "does not leak project state across multiple builds"() {
+        executer.withBuildJvmOpts('-Xms256m', '-Xmx512m').requireIsolatedDaemons().requireDaemon()
+
+        buildFile << """
+            ${legacyWorkerTypeAndTask}
+
+            ext.memoryHog = new byte[1024*1024*150] // ~150MB
+
+            tasks.withType(WorkerTask) { task ->
+                isolationMode = IsolationMode.PROCESS
+                displayName = "Test Work"
+
+                text = "foo"
+                arrayOfThings = ["foo", "bar", "baz"]
+                listOfThings = ["foo", "bar", "baz"]
+
+                // Force a new daemon to be used
+                workerConfiguration = {
+                    forkOptions { options ->
+                        options.with {
+                            systemProperty("foobar", task.name)
+                        }
+                    }
+                }
+            }
+            task startDaemon1(type: WorkerTask)
+            task startDaemon2(type: WorkerTask)
+            task startDaemon3(type: WorkerTask)
+        """
+
+        expect:
+        succeeds("startDaemon1")
+        succeeds("startDaemon2")
+        succeeds("startDaemon3")
+    }
+
     @Issue("https://github.com/gradle/gradle/issues/10323")
     def "can use a Properties object as a parameter"() {
         buildFile << """

subprojects/workers/src/main/java/org/gradle/workers/internal/DaemonForkOptionsBuilder.java

@@ -28,7 +28,7 @@ public class DaemonForkOptionsBuilder {
 
     public DaemonForkOptionsBuilder(JavaForkOptionsFactory forkOptionsFactory) {
         this.forkOptionsFactory = forkOptionsFactory;
-        javaForkOptions = forkOptionsFactory.newJavaForkOptions();
+        this.javaForkOptions = forkOptionsFactory.newJavaForkOptions();
     }
 
     public DaemonForkOptionsBuilder keepAliveMode(KeepAliveMode keepAliveMode) {

subprojects/workers/src/main/java/org/gradle/workers/internal/DefaultProcessWorkerSpec.java

@@ -20,7 +20,6 @@
 import org.gradle.api.Action;
 import org.gradle.api.model.ObjectFactory;
 import org.gradle.process.JavaForkOptions;
-import org.gradle.process.internal.JavaForkOptionsFactory;
 import org.gradle.workers.ClassLoaderWorkerSpec;
 import org.gradle.workers.IsolationMode;
 import org.gradle.workers.ProcessWorkerSpec;
@@ -31,9 +30,9 @@ public class DefaultProcessWorkerSpec extends DefaultClassLoaderWorkerSpec imple
     protected final JavaForkOptions forkOptions;
 
     @Inject
-    public DefaultProcessWorkerSpec(JavaForkOptionsFactory javaForkOptionsFactory, ObjectFactory objectFactory) {
+    public DefaultProcessWorkerSpec(JavaForkOptions forkOptions, ObjectFactory objectFactory) {
         super(IsolationMode.PROCESS, objectFactory);
-        this.forkOptions = javaForkOptionsFactory.newJavaForkOptions();
+        this.forkOptions = forkOptions;
         getForkOptions().setEnvironment(Maps.<String, Object>newHashMap());
     }
 

subprojects/workers/src/main/java/org/gradle/workers/internal/DefaultWorkerConfiguration.java

@@ -21,7 +21,7 @@
 import org.gradle.api.ActionConfiguration;
 import org.gradle.api.internal.DefaultActionConfiguration;
 import org.gradle.process.JavaForkOptions;
-import org.gradle.process.internal.JavaForkOptionsFactory;
+import org.gradle.process.internal.JavaForkOptionsInternal;
 import org.gradle.util.GUtil;
 import org.gradle.workers.ClassLoaderWorkerSpec;
 import org.gradle.workers.ForkMode;
@@ -40,8 +40,8 @@ public class DefaultWorkerConfiguration extends DefaultActionConfiguration imple
     private String displayName;
     private List<File> classpath = Lists.newArrayList();
 
-    public DefaultWorkerConfiguration(JavaForkOptionsFactory forkOptionsFactory) {
-        this.forkOptions = forkOptionsFactory.newJavaForkOptions();
+    public DefaultWorkerConfiguration(JavaForkOptionsInternal forkOptions) {
+        this.forkOptions = forkOptions;
     }
 
     @Override

subprojects/workers/src/main/java/org/gradle/workers/internal/DefaultWorkerExecutor.java

@@ -124,7 +124,7 @@ public WorkQueue classLoaderIsolation(Action<ClassLoaderWorkerSpec> action) {
 
     @Override
     public WorkQueue processIsolation(Action<ProcessWorkerSpec> action) {
-        DefaultProcessWorkerSpec spec = instantiator.newInstance(DefaultProcessWorkerSpec.class);
+        DefaultProcessWorkerSpec spec = instantiator.newInstance(DefaultProcessWorkerSpec.class, forkOptionsFactory.newDecoratedJavaForkOptions());
         File defaultWorkingDir = spec.getForkOptions().getWorkingDir();
         File workingDirectory = workerDirectoryProvider.getWorkingDirectory();
         action.execute(spec);
@@ -140,7 +140,7 @@ public WorkQueue processIsolation(Action<ProcessWorkerSpec> action) {
 
     @Override
     public void submit(Class<? extends Runnable> actionClass, Action<? super WorkerConfiguration> configAction) {
-        DefaultWorkerConfiguration configuration = new DefaultWorkerConfiguration(forkOptionsFactory);
+        DefaultWorkerConfiguration configuration = new DefaultWorkerConfiguration(forkOptionsFactory.newDecoratedJavaForkOptions());
         configAction.execute(configuration);
 
         Action<AdapterWorkParameters> parametersAction = new Action<AdapterWorkParameters>() {

subprojects/workers/src/test/groovy/org/gradle/workers/internal/DefaultWorkerConfigurationTest.groovy

@@ -23,7 +23,7 @@ import org.gradle.workers.WorkerConfiguration
 import spock.lang.Specification
 
 class DefaultWorkerConfigurationTest extends Specification {
-    WorkerConfiguration workerConfiguration = new DefaultWorkerConfiguration(TestFiles.execFactory())
+    WorkerConfiguration workerConfiguration = new DefaultWorkerConfiguration(TestFiles.execFactory().newJavaForkOptions())
 
     def "can accurately adapt to/from ForkMode"() {
         when:

subprojects/workers/src/test/groovy/org/gradle/workers/internal/DefaultWorkerExecutorParallelTest.groovy

@@ -32,8 +32,8 @@ import org.gradle.process.internal.worker.child.WorkerDirectoryProvider
 import org.gradle.test.fixtures.concurrent.ConcurrentSpec
 import org.gradle.util.UsesNativeServices
 import org.gradle.workers.WorkAction
-import org.gradle.workers.WorkerExecutionException
 import org.gradle.workers.WorkParameters
+import org.gradle.workers.WorkerExecutionException
 import org.junit.Rule
 import org.junit.rules.TemporaryFolder
 import spock.lang.Unroll
@@ -68,7 +68,7 @@ class DefaultWorkerExecutorParallelTest extends ConcurrentSpec {
         _ * executionQueueFactory.create() >> executionQueue
         _ * instantiator.newInstance(DefaultWorkerSpec) >> { args -> new DefaultWorkerSpec() }
         _ * instantiator.newInstance(DefaultClassLoaderWorkerSpec) >> { args -> new DefaultClassLoaderWorkerSpec(objectFactory) }
-        _ * instantiator.newInstance(DefaultProcessWorkerSpec) >> { args -> new DefaultProcessWorkerSpec(forkOptionsFactory, objectFactory) }
+        _ * instantiator.newInstance(DefaultProcessWorkerSpec, _) >> { args -> new DefaultProcessWorkerSpec(args[1][0], objectFactory) }
         _ * instantiator.newInstance(DefaultWorkerExecutor.DefaultWorkQueue, _, _) >> { args -> new DefaultWorkerExecutor.DefaultWorkQueue(args[1][0], args[1][1]) }
         workerExecutor = new DefaultWorkerExecutor(workerDaemonFactory, workerInProcessFactory, workerNoIsolationFactory, forkOptionsFactory, buildOperationWorkerRegistry, buildOperationExecutor, asyncWorkerTracker, workerDirectoryProvider, executionQueueFactory, classLoaderStructureProvider, actionExecutionSpecFactory, instantiator)
         _ * actionExecutionSpecFactory.newIsolatedSpec(_, _, _, _) >> Mock(IsolatedParametersActionExecutionSpec)
@@ -139,6 +139,11 @@ class DefaultWorkerExecutorParallelTest extends ConcurrentSpec {
             this.delegate = delegate
         }
 
+        @Override
+        JavaForkOptionsInternal newDecoratedJavaForkOptions() {
+            return newJavaForkOptions()
+        }
+
         @Override
         JavaForkOptionsInternal newJavaForkOptions() {
             def forkOptions = delegate.newJavaForkOptions()