Introduce preExecProcessAction on Darwin, rename preSpawnProcessConfigurator to preExecProcessAction on Linux

Sources/Subprocess/Platforms/Subprocess+Darwin.swift

@@ -81,6 +81,29 @@ public struct PlatformOptions: Sendable {
                 inout posix_spawn_file_actions_t?
             ) throws -> Void
         )? = nil
+    /// A closure to configure platform-specific
+    /// spawning constructs. This closure enables direct
+    /// configuration or override of underlying platform-specific
+    /// spawn settings that `Subprocess` utilizes internally,
+    /// in cases where Subprocess does not provide higher-level
+    /// APIs for such modifications.
+    ///
+    /// On Darwin, Subprocess uses `posix_spawn()` as the
+    /// underlying spawning mechanism, but may require an initial `fork()`
+    /// depending on the configured `PlatformOptions`.
+    /// This closure is called after `fork()` but before `posix_spawn()`
+    /// (with the `POSIX_SPAWN_SETEXEC` flag set).
+    /// You may use it to call any necessary process setup functions.
+    ///
+    /// - note: You can set both `preExecProcessAction` and
+    /// `preSpawnProcessConfigurator` and both will be called.
+    /// Setting `preExecProcessAction` will always cause Subprocess
+    /// to pre-`fork()` before calling `posix_spawn()` (with the
+    /// `POSIX_SPAWN_SETEXEC` flag set) even if it would not have otherwise
+    /// done so based on the configured `PlatformOptions`.
+    ///
+    /// - warning: You may ONLY call [async-signal-safe functions](https://pubs.opengroup.org/onlinepubs/9799919799/functions/V2_chap02.html) within this closure (note _"The following table defines a set of functions and function-like macros that shall be async-signal-safe."_).
+    public var preExecProcessAction: (@convention(c) @Sendable () -> Void)? = nil
 
     public init() {}
 }
@@ -138,6 +161,7 @@ extension PlatformOptions: CustomStringConvertible, CustomDebugStringConvertible
             \(indent)    processGroupID: \(String(describing: processGroupID)),
             \(indent)    createSession: \(createSession),
             \(indent)    preSpawnProcessConfigurator: \(self.preSpawnProcessConfigurator == nil ? "not set" : "set")
+            \(indent)    preExecProcessAction: \(self.preExecProcessAction == nil ? "not set" : "set")
             \(indent))
             """
     }
@@ -402,7 +426,8 @@ extension Configuration {
                             gidPtr,
                             Int32(supplementaryGroups?.count ?? 0),
                             sgroups?.baseAddress,
-                            self.platformOptions.createSession ? 1 : 0
+                            self.platformOptions.createSession ? 1 : 0,
+                            self.platformOptions.preExecProcessAction,
                         )
                     }
                 }

Sources/Subprocess/Platforms/Subprocess+Linux.swift

@@ -152,7 +152,7 @@ extension Configuration {
                                     CInt(supplementaryGroups?.count ?? 0),
                                     sgroups?.baseAddress,
                                     self.platformOptions.createSession ? 1 : 0,
-                                    self.platformOptions.preSpawnProcessConfigurator
+                                    self.platformOptions.preExecProcessAction
                                 )
                             }
                         }
@@ -261,27 +261,27 @@ extension ProcessIdentifier: CustomStringConvertible, CustomDebugStringConvertib
 /// The collection of platform-specific settings
 /// to configure the subprocess when running
 public struct PlatformOptions: Sendable {
-    // Set user ID for the subprocess
+    /// Set user ID for the subprocess
     public var userID: uid_t? = nil
     /// Set the real and effective group ID and the saved
     /// set-group-ID of the subprocess, equivalent to calling
     /// `setgid()` on the child process.
     /// Group ID is used to control permissions, particularly
     /// for file access.
     public var groupID: gid_t? = nil
-    // Set list of supplementary group IDs for the subprocess
+    /// Set list of supplementary group IDs for the subprocess
     public var supplementaryGroups: [gid_t]? = nil
     /// Set the process group for the subprocess, equivalent to
     /// calling `setpgid()` on the child process.
     /// Process group ID is used to group related processes for
     /// controlling signals.
     public var processGroupID: pid_t? = nil
-    // Creates a session and sets the process group ID
-    // i.e. Detach from the terminal.
+    /// Creates a session and sets the process group ID
+    /// i.e. Detach from the terminal.
     public var createSession: Bool = false
     /// An ordered list of steps in order to tear down the child
     /// process in case the parent task is cancelled before
-    /// the child proces terminates.
+    /// the child process terminates.
     /// Always ends in sending a `.kill` signal at the end.
     public var teardownSequence: [TeardownStep] = []
     /// A closure to configure platform-specific
@@ -295,7 +295,9 @@ public struct PlatformOptions: Sendable {
     /// underlying spawning mechanism. This closure is called
     /// after `fork()` but before `exec()`. You may use it to
     /// call any necessary process setup functions.
-    public var preSpawnProcessConfigurator: (@convention(c) @Sendable () -> Void)? = nil
+    ///
+    /// - warning: You may ONLY call [async-signal-safe functions](https://pubs.opengroup.org/onlinepubs/9799919799/functions/V2_chap02.html) within this closure (note _"The following table defines a set of functions and function-like macros that shall be async-signal-safe."_).
+    public var preExecProcessAction: (@convention(c) @Sendable () -> Void)? = nil
 
     public init() {}
 }
@@ -310,7 +312,7 @@ extension PlatformOptions: CustomStringConvertible, CustomDebugStringConvertible
             \(indent)    supplementaryGroups: \(String(describing: supplementaryGroups)),
             \(indent)    processGroupID: \(String(describing: processGroupID)),
             \(indent)    createSession: \(createSession),
-            \(indent)    preSpawnProcessConfigurator: \(self.preSpawnProcessConfigurator == nil ? "not set" : "set")
+            \(indent)    preExecProcessAction: \(self.preExecProcessAction == nil ? "not set" : "set")
             \(indent))
             """
     }

Sources/_SubprocessCShims/include/process_shims.h

@@ -43,7 +43,8 @@ int _subprocess_spawn(
     uid_t * _Nullable uid,
     gid_t * _Nullable gid,
     int number_of_sgroups, const gid_t * _Nullable sgroups,
-    int create_session
+    int create_session,
+    void (* _Nullable configurator)(void)
 );
 #endif // TARGET_OS_MAC
 

Sources/_SubprocessCShims/process_shims.c

@@ -143,7 +143,8 @@ static int _subprocess_spawn_prefork(
     uid_t * _Nullable uid,
     gid_t * _Nullable gid,
     int number_of_sgroups, const gid_t * _Nullable sgroups,
-    int create_session
+    int create_session,
+    void (* _Nullable configurator)(void)
 ) {
 #define write_error_and_exit int error = errno; \
     write(pipefd[1], &error, sizeof(error));\
@@ -212,6 +213,9 @@ static int _subprocess_spawn_prefork(
 
         // Perform setups
         if (number_of_sgroups > 0 && sgroups != NULL) {
+            // POSIX doesn't define setgroups (only getgroups) and therefore makes no guarantee of async-signal-safety,
+            // but we'll assume in practice it should be async-signal-safe on any reasonable platform based on the fact
+            // that getgroups is async-signal-safe.
             if (setgroups(number_of_sgroups, sgroups) != 0) {
                 write_error_and_exit;
             }
@@ -233,6 +237,11 @@ static int _subprocess_spawn_prefork(
             (void)setsid();
         }
 
+        // Run custom configuratior
+        if (configurator != NULL) {
+            configurator();
+        }
+
         // Use posix_spawnas exec
         int error = posix_spawn(pid, exec_path, file_actions, spawn_attrs, args, env);
         // If we reached this point, something went wrong
@@ -281,20 +290,22 @@ int _subprocess_spawn(
     uid_t * _Nullable uid,
     gid_t * _Nullable gid,
     int number_of_sgroups, const gid_t * _Nullable sgroups,
-    int create_session
+    int create_session,
+    void (* _Nullable configurator)(void)
 ) {
     int require_pre_fork = uid != NULL ||
         gid != NULL ||
         number_of_sgroups > 0 ||
-        create_session > 0;
+        create_session > 0 ||
+        configurator != NULL;
 
     if (require_pre_fork != 0) {
         int rc = _subprocess_spawn_prefork(
             pid,
             exec_path,
             file_actions, spawn_attrs,
             args, env,
-            uid, gid, number_of_sgroups, sgroups, create_session
+            uid, gid, number_of_sgroups, sgroups, create_session, configurator
         );
         return rc;
     }
@@ -490,6 +501,9 @@ int _subprocess_fork_exec(
         }
 
         if (number_of_sgroups > 0 && sgroups != NULL) {
+            // POSIX doesn't define setgroups (only getgroups) and therefore makes no guarantee of async-signal-safety,
+            // but we'll assume in practice it should be async-signal-safe on any reasonable platform based on the fact
+            // that getgroups is async-signal-safe.
             if (setgroups(number_of_sgroups, sgroups) != 0) {
                 write_error_and_exit;
             }

Tests/SubprocessTests/SubprocessTests+Darwin.swift

@@ -26,6 +26,75 @@ import Testing
 // MARK: PlatformOptions Tests
 @Suite(.serialized)
 struct SubprocessDarwinTests {
+    @Test func testSubprocessPlatformOptionsPreExecProcessAction() async throws {
+        var platformOptions = PlatformOptions()
+        platformOptions.preExecProcessAction = {
+            exit(1234567)
+        }
+        let idResult = try await Subprocess.run(
+            .path("/bin/pwd"),
+            platformOptions: platformOptions,
+            output: .discarded
+        )
+        #expect(idResult.terminationStatus == .exited(1234567))
+    }
+
+    @Test func testSubprocessPlatformOptionsPreExecProcessActionAndProcessConfigurator() async throws {
+        let (readFD, writeFD) = try FileDescriptor.pipe()
+        try await readFD.closeAfter {
+            let childPID = try await writeFD.closeAfter {
+                // Allocate some constant high-numbered FD that's unlikely to be used.
+                let specialFD = try writeFD.duplicate(as: FileDescriptor(rawValue: 9000))
+                return try await specialFD.closeAfter {
+                    // Make the fd non-blocking just to avoid the test hanging if it fails
+                    let opts = fcntl(specialFD.rawValue, F_GETFD)
+                    #expect(opts >= 0)
+                    #expect(fcntl(specialFD.rawValue, F_SETFD, opts | O_NONBLOCK) >= 0)
+
+                    var platformOptions = PlatformOptions()
+                    platformOptions.preExecProcessAction = {
+                        var pid: Int32 = getpid()
+                        if write(9000, &pid, 4) != 4 {
+                            exit(EXIT_FAILURE)
+                        }
+                    }
+                    platformOptions.preSpawnProcessConfigurator = { spawnAttr, _ in
+                        // Set POSIX_SPAWN_SETSID flag, which implies calls
+                        // to setsid
+                        var flags: Int16 = 0
+                        posix_spawnattr_getflags(&spawnAttr, &flags)
+                        posix_spawnattr_setflags(&spawnAttr, flags | Int16(POSIX_SPAWN_SETSID))
+                    }
+                    // Check the process ID (pid), process group ID (pgid), and
+                    // controlling terminal's process group ID (tpgid)
+                    let psResult = try await Subprocess.run(
+                        .path("/bin/bash"),
+                        arguments: ["-c", "ps -o pid,pgid,tpgid -p $$"],
+                        platformOptions: platformOptions,
+                        input: .none,
+                        error: .discarded
+                    ) { execution, standardOutput in
+                        var buffer = Data()
+                        for try await chunk in standardOutput {
+                            let currentChunk = chunk.withUnsafeBytes { Data($0) }
+                            buffer += currentChunk
+                        }
+                        return (pid: execution.processIdentifier.value, standardOutput: buffer)
+                    }
+                    try assertNewSessionCreated(terminationStatus: psResult.terminationStatus, output: String(decoding: psResult.value.standardOutput, as: UTF8.self))
+                    return psResult.value.pid
+                }
+            }
+
+            let bytes = try await readFD.readUntilEOF(upToLength: 4)
+            var pid: Int32 = -1
+            _ = withUnsafeMutableBytes(of: &pid) { ptr in
+                bytes.copyBytes(to: ptr)
+            }
+            #expect(pid == childPID)
+        }
+    }
+
     @Test func testSubprocessPlatformOptionsProcessConfiguratorUpdateSpawnAttr() async throws {
         var platformOptions = PlatformOptions()
         platformOptions.preSpawnProcessConfigurator = { spawnAttr, _ in

Tests/SubprocessTests/SubprocessTests+Linux.swift

@@ -34,9 +34,9 @@ struct SubprocessLinuxTests {
             "This test requires root privileges"
         )
     )
-    func testSubprocessPlatformOptionsPreSpawnProcessConfigurator() async throws {
+    func testSubprocessPlatformOptionsPreExecProcessAction() async throws {
         var platformOptions = PlatformOptions()
-        platformOptions.preSpawnProcessConfigurator = {
+        platformOptions.preExecProcessAction = {
             guard setgid(4321) == 0 else {
                 // Returns EPERM when:
                 //   The calling process is not privileged (does not have the

Tests/SubprocessTests/SubprocessTests+Unix.swift

@@ -955,6 +955,32 @@ extension SubprocessUnixTests {
 }
 
 // MARK: - Utils
+extension FileDescriptor {
+    /// Runs a closure and then closes the FileDescriptor, even if an error occurs.
+    ///
+    /// - Parameter body: The closure to run.
+    ///   If the closure throws an error,
+    ///   this method closes the file descriptor before it rethrows that error.
+    ///
+    /// - Returns: The value returned by the closure.
+    ///
+    /// If `body` throws an error
+    /// or an error occurs while closing the file descriptor,
+    /// this method rethrows that error.
+    public func closeAfter<R>(_ body: () async throws -> R) async throws -> R {
+        // No underscore helper, since the closure's throw isn't necessarily typed.
+        let result: R
+        do {
+            result = try await body()
+        } catch {
+            _ = try? self.close() // Squash close error and throw closure's
+            throw error
+        }
+        try self.close()
+        return result
+    }
+}
+
 extension SubprocessUnixTests {
     private func assertID(
         withArgument argument: String,
@@ -981,10 +1007,18 @@ internal func assertNewSessionCreated<Output: OutputProtocol>(
         Output
     >
 ) throws {
-    #expect(result.terminationStatus.isSuccess)
-    let psValue = try #require(
-        result.standardOutput
+    try assertNewSessionCreated(
+        terminationStatus: result.terminationStatus,
+        output: #require(result.standardOutput)
     )
+}
+
+internal func assertNewSessionCreated(
+    terminationStatus: TerminationStatus,
+    output psValue: String
+) throws {
+    #expect(terminationStatus.isSuccess)
+
     let match = try #require(try #/\s*PID\s*PGID\s*TPGID\s*(?<pid>[\-]?[0-9]+)\s*(?<pgid>[\-]?[0-9]+)\s*(?<tpgid>[\-]?[0-9]+)\s*/#.wholeMatch(in: psValue), "ps output was in an unexpected format:\n\n\(psValue)")
     // If setsid() has been called successfully, we should observe:
     // - pid == pgid