Revert changes to ExecHandleRunner (#31283)

gradle · Nov 19, 2024 · 69b756e · 69b756e
2 parents 7b44432 + 994b294
commit 69b756e
Show file tree

Hide file tree

Showing 7 changed files with 67 additions and 20 deletions.
diff --git a/gradle.properties b/gradle.properties
@@ -23,7 +23,7 @@ develocity.internal.testdistribution.writeTraceFile=true
 gradle.internal.testdistribution.queryResponseTimeout=PT20S
 develocity.internal.testdistribution.queryResponseTimeout=PT20S
 # Default performance baseline
-defaultPerformanceBaselines=8.11-commit-4a6676a6310
+defaultPerformanceBaselines=8.11.1-commit-a7cb71b776d
 
 # Skip dependency analysis for tests
 systemProp.dependency.analysis.test.analysis=false
diff --git a/...e/native/src/main/java/org/gradle/internal/nativeintegration/services/NativeServices.java b/...e/native/src/main/java/org/gradle/internal/nativeintegration/services/NativeServices.java
@@ -19,12 +19,14 @@
 import net.rubygrapefruit.platform.NativeException;
 import net.rubygrapefruit.platform.NativeIntegrationUnavailableException;
 import net.rubygrapefruit.platform.Process;
+import net.rubygrapefruit.platform.ProcessLauncher;
 import net.rubygrapefruit.platform.SystemInfo;
 import net.rubygrapefruit.platform.WindowsRegistry;
 import net.rubygrapefruit.platform.file.FileEvents;
 import net.rubygrapefruit.platform.file.FileSystems;
 import net.rubygrapefruit.platform.file.Files;
 import net.rubygrapefruit.platform.file.PosixFiles;
+import net.rubygrapefruit.platform.internal.DefaultProcessLauncher;
 import net.rubygrapefruit.platform.memory.Memory;
 import net.rubygrapefruit.platform.terminal.Terminals;
 import org.gradle.api.JavaVersion;
@@ -440,6 +442,18 @@ protected Memory createMemory(OperatingSystem operatingSystem) {
         return notAvailable(Memory.class, operatingSystem);
     }
 
+    @Provides
+    protected ProcessLauncher createProcessLauncher() {
+        if (useNativeIntegrations) {
+            try {
+                return net.rubygrapefruit.platform.Native.get(ProcessLauncher.class);
+            } catch (NativeIntegrationUnavailableException e) {
+                LOGGER.debug("Native-platform process launcher is not available. Continuing with fallback.");
+            }
+        }
+        return new DefaultProcessLauncher();
+    }
+
     @Provides
     protected PosixFiles createPosixFiles(OperatingSystem operatingSystem) {
         if (useNativeIntegrations) {

diff --git a/.../src/test/groovy/org/gradle/internal/nativeintegration/services/NativeServicesTest.groovy b/.../src/test/groovy/org/gradle/internal/nativeintegration/services/NativeServicesTest.groovy
@@ -15,7 +15,7 @@
  */
 package org.gradle.internal.nativeintegration.services
 
-
+import net.rubygrapefruit.platform.ProcessLauncher
 import net.rubygrapefruit.platform.SystemInfo
 import net.rubygrapefruit.platform.WindowsRegistry
 import org.gradle.api.internal.file.temp.TemporaryFileProvider
@@ -78,6 +78,11 @@ class NativeServicesTest extends Specification {
         services.get(SystemInfo) != null
     }
 
+    def "makes a ProcessLauncher available"() {
+        expect:
+        services.get(ProcessLauncher) != null
+    }
+
     def "makes a TemporaryFileProvider available"() {
         expect:
         services.get(TemporaryFileProvider) != null

diff --git a/...s/jvm/language-java/src/integTest/groovy/org/gradle/integtests/ExecIntegrationTest.groovy b/...s/jvm/language-java/src/integTest/groovy/org/gradle/integtests/ExecIntegrationTest.groovy
@@ -25,6 +25,8 @@ import org.gradle.integtests.fixtures.UnsupportedWithConfigurationCache
 import org.gradle.internal.os.OperatingSystem
 import org.gradle.process.TestJavaMain
 import org.gradle.test.fixtures.dsl.GradleDsl
+import org.gradle.test.precondition.Requires
+import org.gradle.test.preconditions.UnitTestPreconditions
 import org.gradle.util.internal.TextUtil
 import org.junit.Rule
 import spock.lang.Issue
@@ -424,6 +426,37 @@ class ExecIntegrationTest extends AbstractIntegrationSpec {
         "javaexec"         | javaExecSpec() | "Using method javaexec(Closure)"       | "ExecOperations.javaexec(Action) or ProviderFactory.javaexec(Action)"
     }
 
+    @Issue("https://github.com/gradle/gradle/issues/31282")
+    @Requires(UnitTestPreconditions.NotWindows)
+    def "running multiple tasks that fork processes is multi-thread safe"() {
+        def numOfProjects = 1000
+        numOfProjects.times {
+            settingsFile << """
+                include 'project$it'
+            """
+            file("project${it}/build.gradle") << """
+                abstract class MyExec extends DefaultTask {
+                    @Inject
+                    abstract ExecOperations getExecOperations()
+
+                    @TaskAction
+                    void doIt() {
+                        def script = new File(temporaryDir, "script.sh")
+                        script.text = "#!/bin/bash"
+                        script.executable = true
+                        execOperations.exec {
+                            commandLine script.absolutePath
+                        }
+                        script.delete()
+                    }
+                }
+                tasks.register("run", MyExec)
+            """
+        }
+        expect:
+        succeeds("run", "--max-workers=100", "--parallel")
+    }
+
     @UnsupportedWithConfigurationCache(because = "Runs external process at configuration time")
     def "#method in #location is deprecated in Groovy at configuration time"() {
         def initScript = groovyFile("init.gradle", "")

diff --git a/subprojects/core/build.gradle.kts b/subprojects/core/build.gradle.kts
@@ -125,14 +125,14 @@ dependencies {
     api(libs.guava)
     api(libs.inject)
     api(libs.jsr305)
+    api(libs.nativePlatform)
 
     implementation(projects.buildOperationsTrace)
     implementation(projects.io)
     implementation(projects.inputTracking)
     implementation(projects.modelGroovy)
     implementation(projects.serviceRegistryBuilder)
 
-    implementation(libs.nativePlatform)
     implementation(libs.asmCommons)
     implementation(libs.commonsCompress)
     implementation(libs.commonsIo)

diff --git a/subprojects/core/src/main/java/org/gradle/process/internal/DefaultExecHandle.java b/subprojects/core/src/main/java/org/gradle/process/internal/DefaultExecHandle.java
@@ -17,11 +17,13 @@
 package org.gradle.process.internal;
 
 import com.google.common.base.Joiner;
+import net.rubygrapefruit.platform.ProcessLauncher;
 import org.gradle.api.logging.Logger;
 import org.gradle.api.logging.Logging;
 import org.gradle.initialization.BuildCancellationToken;
 import org.gradle.internal.UncheckedException;
 import org.gradle.internal.event.ListenerBroadcast;
+import org.gradle.internal.nativeintegration.services.NativeServices;
 import org.gradle.internal.operations.CurrentBuildOperationRef;
 import org.gradle.process.ExecResult;
 import org.gradle.process.internal.shutdown.ShutdownHooks;
@@ -88,6 +90,7 @@ public class DefaultExecHandle implements ExecHandle, ProcessSettings {
     private final StreamsHandler outputHandler;
     private final StreamsHandler inputHandler;
     private final boolean redirectErrorStream;
+    private final ProcessLauncher processLauncher;
     private int timeoutMillis;
     private boolean daemon;
 
@@ -136,9 +139,9 @@ public class DefaultExecHandle implements ExecHandle, ProcessSettings {
         this.stateChanged = lock.newCondition();
         this.state = ExecHandleState.INIT;
         this.buildCancellationToken = buildCancellationToken;
-        this.shutdownHookAction = new ExecHandleShutdownHookAction(this);
-        this.broadcast = new ListenerBroadcast<ExecHandleListener>(ExecHandleListener.class);
-
+        processLauncher = NativeServices.getInstance().get(ProcessLauncher.class);
+        shutdownHookAction = new ExecHandleShutdownHookAction(this);
+        broadcast = new ListenerBroadcast<ExecHandleListener>(ExecHandleListener.class);
         broadcast.addAll(listeners);
     }
 
@@ -262,7 +265,7 @@ public ExecHandle start() {
 
             broadcast.getSource().beforeExecutionStarted(this);
             execHandleRunner = new ExecHandleRunner(
-                this, new CompositeStreamsHandler(), executor, CurrentBuildOperationRef.instance().get()
+                this, new CompositeStreamsHandler(), processLauncher, executor, CurrentBuildOperationRef.instance().get()
             );
             executor.execute(execHandleRunner);
 

diff --git a/subprojects/core/src/main/java/org/gradle/process/internal/ExecHandleRunner.java b/subprojects/core/src/main/java/org/gradle/process/internal/ExecHandleRunner.java
@@ -16,7 +16,7 @@
 
 package org.gradle.process.internal;
 
-import net.rubygrapefruit.platform.NativeException;
+import net.rubygrapefruit.platform.ProcessLauncher;
 import org.gradle.api.logging.Logger;
 import org.gradle.api.logging.Logging;
 import org.gradle.internal.operations.BuildOperationRef;
@@ -32,6 +32,7 @@ public class ExecHandleRunner implements Runnable {
     private final ProcessBuilderFactory processBuilderFactory;
     private final DefaultExecHandle execHandle;
     private final Lock lock = new ReentrantLock();
+    private final ProcessLauncher processLauncher;
     private final Executor executor;
 
     private Process process;
@@ -40,16 +41,15 @@ public class ExecHandleRunner implements Runnable {
     private volatile BuildOperationRef associatedBuildOperation;
 
     public ExecHandleRunner(
-        DefaultExecHandle execHandle,
-        StreamsHandler streamsHandler,
-        Executor executor,
+        DefaultExecHandle execHandle, StreamsHandler streamsHandler, ProcessLauncher processLauncher, Executor executor,
         BuildOperationRef associatedBuildOperation
     ) {
         if (execHandle == null) {
             throw new IllegalArgumentException("execHandle == null!");
         }
         this.execHandle = execHandle;
         this.streamsHandler = streamsHandler;
+        this.processLauncher = processLauncher;
         this.executor = executor;
         this.associatedBuildOperation = associatedBuildOperation;
         this.processBuilderFactory = new ProcessBuilderFactory();
@@ -119,22 +119,14 @@ private void startProcess() {
                 throw new IllegalStateException("Process has already been aborted");
             }
             ProcessBuilder processBuilder = processBuilderFactory.createProcessBuilder(execHandle);
-            Process process = start(processBuilder);
+            Process process = processLauncher.start(processBuilder);
             streamsHandler.connectStreams(process, execHandle.getDisplayName(), executor);
             this.process = process;
         } finally {
             lock.unlock();
         }
     }
 
-    private Process start(ProcessBuilder processBuilder) {
-        try {
-            return processBuilder.start();
-        } catch (Exception e) {
-            throw new NativeException(String.format("Could not start '%s'", processBuilder.command().get(0)), e);
-        }
-    }
-
     private void completed(int exitValue) {
         if (aborted) {
             execHandle.aborted(exitValue);