Skip to content

Commit 44822fe

Browse files
committed
Remove the @convention(c) restriction from preExecProcessAction
This allows it to capture local context, and avoid using a fragile hardcoded file descriptor in the corresponding test. Closes #148
1 parent 6d5ffa5 commit 44822fe

File tree

5 files changed

+46
-18
lines changed

5 files changed

+46
-18
lines changed

Sources/Subprocess/Platforms/Subprocess+Darwin.swift

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ public struct PlatformOptions: Sendable {
103103
/// done so based on the configured `PlatformOptions`.
104104
///
105105
/// - 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."_).
106-
public var preExecProcessAction: (@convention(c) @Sendable () -> Void)? = nil
106+
public var preExecProcessAction: (@Sendable () -> Void)? = nil
107107

108108
public init() {}
109109
}
@@ -412,6 +412,17 @@ extension Configuration {
412412
try spawnConfig(&spawnAttributes, &fileActions)
413413
}
414414

415+
final class Context {
416+
let body: @Sendable () -> ()
417+
init(body: @Sendable @escaping () -> Void) {
418+
self.body = body
419+
}
420+
}
421+
422+
func preExecProcessAction(_ context: UnsafeMutableRawPointer) -> Void {
423+
(Unmanaged<AnyObject>.fromOpaque(context).takeRetainedValue() as! Context).body()
424+
}
425+
415426
// Spawn
416427
let spawnError: CInt = possibleExecutablePath.withCString { exePath in
417428
return supplementaryGroups.withOptionalUnsafeBufferPointer { sgroups in
@@ -427,7 +438,8 @@ extension Configuration {
427438
Int32(supplementaryGroups?.count ?? 0),
428439
sgroups?.baseAddress,
429440
self.platformOptions.createSession ? 1 : 0,
430-
self.platformOptions.preExecProcessAction,
441+
preExecProcessAction,
442+
self.platformOptions.preExecProcessAction.map { Unmanaged.passRetained(Context(body: $0)).toOpaque() }
431443
)
432444
}
433445
}

Sources/Subprocess/Platforms/Subprocess+Linux.swift

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,17 @@ extension Configuration {
131131
errorReadFileDescriptor?.platformDescriptor() ?? -1,
132132
]
133133

134+
final class Context {
135+
let body: @Sendable () -> ()
136+
init(body: @Sendable @escaping () -> Void) {
137+
self.body = body
138+
}
139+
}
140+
141+
func preExecProcessAction(_ context: UnsafeMutableRawPointer) -> Void {
142+
(Unmanaged<AnyObject>.fromOpaque(context).takeRetainedValue() as! Context).body()
143+
}
144+
134145
// Spawn
135146
var pid: pid_t = 0
136147
var processDescriptor: PlatformFileDescriptor = -1
@@ -152,7 +163,8 @@ extension Configuration {
152163
CInt(supplementaryGroups?.count ?? 0),
153164
sgroups?.baseAddress,
154165
self.platformOptions.createSession ? 1 : 0,
155-
self.platformOptions.preExecProcessAction
166+
preExecProcessAction,
167+
self.platformOptions.preExecProcessAction.map { Unmanaged.passRetained(Context(body: $0)).toOpaque() }
156168
)
157169
}
158170
}
@@ -297,7 +309,7 @@ public struct PlatformOptions: Sendable {
297309
/// call any necessary process setup functions.
298310
///
299311
/// - 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."_).
300-
public var preExecProcessAction: (@convention(c) @Sendable () -> Void)? = nil
312+
public var preExecProcessAction: (@Sendable () -> Void)? = nil
301313

302314
public init() {}
303315
}

Sources/_SubprocessCShims/include/process_shims.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ int _subprocess_spawn(
4444
gid_t * _Nullable gid,
4545
int number_of_sgroups, const gid_t * _Nullable sgroups,
4646
int create_session,
47-
void (* _Nullable configurator)(void)
47+
void (* _Nonnull configurator)(void * _Nonnull context),
48+
void * _Nullable configuratorContext
4849
);
4950
#endif // TARGET_OS_MAC
5051

@@ -61,7 +62,8 @@ int _subprocess_fork_exec(
6162
gid_t * _Nullable process_group_id,
6263
int number_of_sgroups, const gid_t * _Nullable sgroups,
6364
int create_session,
64-
void (* _Nullable configurator)(void)
65+
void (* _Nonnull configurator)(void * _Nonnull context),
66+
void * _Nullable configuratorContext
6567
);
6668

6769
int _was_process_exited(int status);

Sources/_SubprocessCShims/process_shims.c

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,8 @@ static int _subprocess_spawn_prefork(
148148
gid_t * _Nullable gid,
149149
int number_of_sgroups, const gid_t * _Nullable sgroups,
150150
int create_session,
151-
void (* _Nullable configurator)(void)
151+
void (* _Nonnull configurator)(void * _Nonnull context),
152+
void * _Nullable configuratorContext
152153
) {
153154
#define write_error_and_exit int error = errno; \
154155
write(pipefd[1], &error, sizeof(error));\
@@ -242,8 +243,8 @@ static int _subprocess_spawn_prefork(
242243
}
243244

244245
// Run custom configuratior
245-
if (configurator != NULL) {
246-
configurator();
246+
if (configuratorContext != NULL) {
247+
configurator(configuratorContext);
247248
}
248249

249250
// Use posix_spawnas exec
@@ -295,21 +296,22 @@ int _subprocess_spawn(
295296
gid_t * _Nullable gid,
296297
int number_of_sgroups, const gid_t * _Nullable sgroups,
297298
int create_session,
298-
void (* _Nullable configurator)(void)
299+
void (* _Nonnull configurator)(void * _Nonnull context),
300+
void * _Nullable configuratorContext
299301
) {
300302
int require_pre_fork = uid != NULL ||
301303
gid != NULL ||
302304
number_of_sgroups > 0 ||
303305
create_session > 0 ||
304-
configurator != NULL;
306+
configuratorContext != NULL;
305307

306308
if (require_pre_fork != 0) {
307309
int rc = _subprocess_spawn_prefork(
308310
pid,
309311
exec_path,
310312
file_actions, spawn_attrs,
311313
args, env,
312-
uid, gid, number_of_sgroups, sgroups, create_session, configurator
314+
uid, gid, number_of_sgroups, sgroups, create_session, configurator, configuratorContext
313315
);
314316
return rc;
315317
}
@@ -387,7 +389,8 @@ int _subprocess_fork_exec(
387389
gid_t * _Nullable process_group_id,
388390
int number_of_sgroups, const gid_t * _Nullable sgroups,
389391
int create_session,
390-
void (* _Nullable configurator)(void)
392+
void (* _Nonnull configurator)(void * _Nonnull context),
393+
void * _Nullable configuratorContext
391394
) {
392395
#define write_error_and_exit int error = errno; \
393396
write(pipefd[1], &error, sizeof(error));\
@@ -573,8 +576,8 @@ int _subprocess_fork_exec(
573576
}
574577

575578
// Run custom configuratior
576-
if (configurator != NULL) {
577-
configurator();
579+
if (configuratorContext != NULL) {
580+
configurator(configuratorContext);
578581
}
579582
// Finally, exec
580583
execve(exec_path, args, env);

Tests/SubprocessTests/SubprocessTests+Darwin.swift

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,7 @@ struct SubprocessDarwinTests {
4343
let (readFD, writeFD) = try FileDescriptor.pipe()
4444
try await readFD.closeAfter {
4545
let childPID = try await writeFD.closeAfter {
46-
// Allocate some constant high-numbered FD that's unlikely to be used.
47-
let specialFD = try writeFD.duplicate(as: FileDescriptor(rawValue: 9000))
46+
let specialFD = try writeFD.duplicate()
4847
return try await specialFD.closeAfter {
4948
// Make the fd non-blocking just to avoid the test hanging if it fails
5049
let opts = fcntl(specialFD.rawValue, F_GETFD)
@@ -54,7 +53,7 @@ struct SubprocessDarwinTests {
5453
var platformOptions = PlatformOptions()
5554
platformOptions.preExecProcessAction = {
5655
var pid: Int32 = getpid()
57-
if write(9000, &pid, 4) != 4 {
56+
if write(specialFD.rawValue, &pid, 4) != 4 {
5857
exit(EXIT_FAILURE)
5958
}
6059
}

0 commit comments

Comments
 (0)