Skip to content

Remove preExecProcessAction #149

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 21, 2025
+10 −172
Merged

Remove preExecProcessAction #149

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 1 addition & 26 deletions Sources/Subprocess/Platforms/Subprocess+Darwin.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,29 +81,6 @@ 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() {}
}
Expand Down Expand Up @@ -161,7 +138,6 @@ 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))
"""
}
Expand Down Expand Up @@ -426,8 +402,7 @@ extension Configuration {
gidPtr,
Int32(supplementaryGroups?.count ?? 0),
sgroups?.baseAddress,
self.platformOptions.createSession ? 1 : 0,
self.platformOptions.preExecProcessAction,
self.platformOptions.createSession ? 1 : 0
)
}
}
Expand Down
20 changes: 2 additions & 18 deletions Sources/Subprocess/Platforms/Subprocess+Linux.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,7 @@ extension Configuration {
processGroupIDPtr,
CInt(supplementaryGroups?.count ?? 0),
sgroups?.baseAddress,
self.platformOptions.createSession ? 1 : 0,
self.platformOptions.preExecProcessAction
self.platformOptions.createSession ? 1 : 0
)
}
}
Expand Down Expand Up @@ -284,20 +283,6 @@ public struct PlatformOptions: Sendable {
/// the child process terminates.
/// Always ends in sending a `.kill` signal at the end.
public var teardownSequence: [TeardownStep] = []
/// 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 Linux, Subprocess uses `fork/exec` as the
/// underlying spawning mechanism. This closure is called
/// after `fork()` but before `exec()`. You may use it to
/// call any necessary process setup functions.
///
/// - 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() {}
}
Expand All @@ -311,8 +296,7 @@ extension PlatformOptions: CustomStringConvertible, CustomDebugStringConvertible
\(indent) groupID: \(String(describing: groupID)),
\(indent) supplementaryGroups: \(String(describing: supplementaryGroups)),
\(indent) processGroupID: \(String(describing: processGroupID)),
\(indent) createSession: \(createSession),
\(indent) preExecProcessAction: \(self.preExecProcessAction == nil ? "not set" : "set")
\(indent) createSession: \(createSession)
\(indent))
"""
}
Expand Down
6 changes: 2 additions & 4 deletions Sources/_SubprocessCShims/include/process_shims.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ int _subprocess_spawn(
uid_t * _Nullable uid,
gid_t * _Nullable gid,
int number_of_sgroups, const gid_t * _Nullable sgroups,
int create_session,
void (* _Nullable configurator)(void)
int create_session
);
#endif // TARGET_OS_MAC

Expand All @@ -60,8 +59,7 @@ int _subprocess_fork_exec(
gid_t * _Nullable gid,
gid_t * _Nullable process_group_id,
int number_of_sgroups, const gid_t * _Nullable sgroups,
int create_session,
void (* _Nullable configurator)(void)
int create_session
);

int _was_process_exited(int status);
Expand Down
23 changes: 5 additions & 18 deletions Sources/_SubprocessCShims/process_shims.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,7 @@ 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,
void (* _Nullable configurator)(void)
int create_session
) {
#define write_error_and_exit int error = errno; \
write(pipefd[1], &error, sizeof(error));\
Expand Down Expand Up @@ -191,11 +190,6 @@ 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
Expand Down Expand Up @@ -244,22 +238,20 @@ int _subprocess_spawn(
uid_t * _Nullable uid,
gid_t * _Nullable gid,
int number_of_sgroups, const gid_t * _Nullable sgroups,
int create_session,
void (* _Nullable configurator)(void)
int create_session
) {
int require_pre_fork = uid != NULL ||
gid != NULL ||
number_of_sgroups > 0 ||
create_session > 0 ||
configurator != NULL;
create_session > 0;

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, configurator
uid, gid, number_of_sgroups, sgroups, create_session
);
return rc;
}
Expand Down Expand Up @@ -486,8 +478,7 @@ int _subprocess_fork_exec(
gid_t * _Nullable gid,
gid_t * _Nullable process_group_id,
int number_of_sgroups, const gid_t * _Nullable sgroups,
int create_session,
void (* _Nullable configurator)(void)
int create_session
) {
#define write_error_and_exit int error = errno; \
write(pipefd[1], &error, sizeof(error));\
Expand Down Expand Up @@ -681,10 +672,6 @@ int _subprocess_fork_exec(
}
}

// Run custom configuratior
if (configurator != NULL) {
configurator();
}
// Finally, exec
execve(exec_path, args, env);
// If we reached this point, something went wrong
Expand Down
72 changes: 0 additions & 72 deletions Tests/SubprocessTests/DarwinTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,78 +26,6 @@ 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(
.disabled("Constantly fails on macOS 26 and Swift 6.2"),
.bug("https://github.com/swiftlang/swift-subprocess/issues/148")
) 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
Expand Down
34 changes: 0 additions & 34 deletions Tests/SubprocessTests/LinuxTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,40 +29,6 @@ import _SubprocessCShims
// MARK: PlatformOption Tests
@Suite(.serialized)
struct SubprocessLinuxTests {
@Test(
.enabled(
if: getgid() == 0,
"This test requires root privileges"
)
)
func testSubprocessPlatformOptionsPreExecProcessAction() async throws {
var platformOptions = PlatformOptions()
platformOptions.preExecProcessAction = {
guard setgid(4321) == 0 else {
// Returns EPERM when:
// The calling process is not privileged (does not have the
// CAP_SETGID capability in its user namespace), and gid does
// not match the real group ID or saved set-group-ID of the
// calling process.
abort()
}
}
let idResult = try await Subprocess.run(
.path("/usr/bin/id"),
arguments: ["-g"],
platformOptions: platformOptions,
output: .string(limit: 32),
error: .string(limit: 32)
)
let error = try #require(idResult.standardError)
try #require(error == "")
#expect(idResult.terminationStatus.isSuccess)
let id = try #require(idResult.standardOutput)
#expect(
id.trimmingCharacters(in: .whitespacesAndNewlines) == "\(4321)"
)
}

@Test func testSuspendResumeProcess() async throws {
func blockAndWaitForStatus(
pid: pid_t,
Expand Down