diff --git a/Sources/Subprocess/Platforms/Subprocess+Darwin.swift b/Sources/Subprocess/Platforms/Subprocess+Darwin.swift index 6762578..cd04d64 100644 --- a/Sources/Subprocess/Platforms/Subprocess+Darwin.swift +++ b/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, ) } } diff --git a/Sources/Subprocess/Platforms/Subprocess+Linux.swift b/Sources/Subprocess/Platforms/Subprocess+Linux.swift index 62eb232..ac1bca9 100644 --- a/Sources/Subprocess/Platforms/Subprocess+Linux.swift +++ b/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,7 +261,7 @@ 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 @@ -269,19 +269,19 @@ public struct PlatformOptions: Sendable { /// 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)) """ } diff --git a/Sources/_SubprocessCShims/include/process_shims.h b/Sources/_SubprocessCShims/include/process_shims.h index 64b7313..085cb77 100644 --- a/Sources/_SubprocessCShims/include/process_shims.h +++ b/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 diff --git a/Sources/_SubprocessCShims/process_shims.c b/Sources/_SubprocessCShims/process_shims.c index 906f70a..376abbd 100644 --- a/Sources/_SubprocessCShims/process_shims.c +++ b/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,12 +290,14 @@ 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( @@ -294,7 +305,7 @@ int _subprocess_spawn( 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; } diff --git a/Tests/SubprocessTests/SubprocessTests+Darwin.swift b/Tests/SubprocessTests/SubprocessTests+Darwin.swift index 2c4acba..9bed5e1 100644 --- a/Tests/SubprocessTests/SubprocessTests+Darwin.swift +++ b/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 diff --git a/Tests/SubprocessTests/SubprocessTests+Linux.swift b/Tests/SubprocessTests/SubprocessTests+Linux.swift index 0fb6718..17de67f 100644 --- a/Tests/SubprocessTests/SubprocessTests+Linux.swift +++ b/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 diff --git a/Tests/SubprocessTests/SubprocessTests+Unix.swift b/Tests/SubprocessTests/SubprocessTests+Unix.swift index e23a246..289bfee 100644 --- a/Tests/SubprocessTests/SubprocessTests+Unix.swift +++ b/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(_ 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 > ) 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*(?[\-]?[0-9]+)\s*(?[\-]?[0-9]+)\s*(?[\-]?[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