Prefer posix_spawn on OSX

[Copilot]

Description

Uses posix_spawn instead of fork/exec in SystemNative_ForkAndExecProcess on macOS.

• Guard: #if defined(TARGET_OSX), placed as the first code block in the function — restricts the path to macOS only (excludes iOS, tvOS, MacCatalyst, watchOS)
• Credential fallback: When setCredentials is true, falls through to the existing fork/exec path since macOS posix_spawn does not support setuid/setgid
• Error handling: Sets *childPid = -1 early (right after the assert) so all error returns consistently report failure; only overwritten with the spawned PID on the happy path
• Signal handling: Sets POSIX_SPAWN_SETSIGDEF and POSIX_SPAWN_SETSIGMASK for parity with the fork path. The sigdefault set is built by iterating signals 1..NSIG, skipping SIGKILL/SIGSTOP, and only including signals with custom handlers (preserving SIG_IGN and SIG_DFL handlers) — re-uses the existing handler_from_sigaction helper without code duplication. The signal mask captures the parent's current mask via a standalone pthread_sigmask call (with its own error handling block) and passes the result to posix_spawnattr_setsigmask in a compound || conditional alongside posix_spawnattr_setflags, setsigdefault, setsigmask, and file_actions_init with shared error handling
• FD redirection: Uses posix_spawn_file_actions_adddup2 for stdin/stdout/stderr, skipping when fd is -1
• Working directory: Calls posix_spawn_file_actions_addchdir_np directly (always available on supported macOS versions, no HAVE_ constant)
• Scoped out: No POSIX_SPAWN_CLOEXEC_DEFAULT (separate PR), no create_suspended, no create_new_process_group
• Test: Added ChildProcess_InheritsParentSignalMask_CanReceiveSignals test in ProcessTests.Unix.cs that uses RemoteExecutor and PosixSignalRegistration to verify a child process started from a parent with registered signal handlers can still receive signals correctly; skipped on Windows via [SkipOnPlatform(TestPlatforms.Windows)] since SIGCONT is not supported there

fixes #126097

[adamsitnik]

@tmds @wfurt could you please review this PR? The motivation is to later be able to use POSIX_SPAWN_CLOEXEC_DEFAULT to implement #13943.

As a nice bonus we get MASSIVE perf boost for starting processes on macOS (EgorBot/Benchmarks#76):

BenchmarkDotNet v0.15.8, macOS Sequoia 15.4.1 (24E263) [Darwin 24.4.0]
Apple M4, 1 CPU, 10 logical and 10 physical cores
.NET SDK 11.0.100-preview.3.26176.102
  [Host]     : .NET 10.0.5 (10.0.5, 10.0.526.15411), Arm64 RyuJIT armv8.0-a

Method	Toolchain	Mean	Error	Ratio	Allocated
StartAndWaitForExit	/PR_126063/corerun	1,232.2 us	5.24 us	1.00	23.93 KB
StartAndWaitForExit	/main/corerun	11,402.5 us	133.37 us	9.25	23.93 KB
Start	/PR_126063/corerun	119.4 us	2.29 us	1.00	23.98 KB
Start	/main/corerun	13,159.9 us	174.42 us	110.29	23.98 KB

cc @stephentoub I believe it may be worth including in .NET 11 perf blog post ;)

[stephentoub]

@copilot invoke the code-review skill and post the analysis/results as a comment on this PR

[Copilot]

@copilot invoke the code-review skill and post the analysis/results as a comment on this PR

🤖 Copilot Code Review — PR #126063

Note

This review was generated by Copilot using the code-review skill, with cross-model analysis (Claude, Gemini, GPT). Nits found during review were fixed in commit 54f7706.

Holistic Assessment

Motivation: Well-justified. Using posix_spawn on macOS is the recommended and most performant way to spawn processes on Apple targets. The linked issue (#126097) provides clear rationale: it's a prerequisite for POSIX_SPAWN_CLOEXEC_DEFAULT (needed for #13943), and benchmarks show up to 100x improvement on arm64. The motivation is real and the performance data is compelling.

Approach: The approach is sound — adding posix_spawn as a fast path guarded by #if defined(TARGET_OSX) with a fallback to fork/exec when setCredentials is true. The shared preamble (assert, *childPid = -1, access() check) avoids duplication between the two paths.

Summary: ⚠️ Needs Human Review. The native code is well-structured with correct error handling and resource cleanup. The signal handling logic correctly mirrors the fork path. Two findings warrant human reviewer attention: (1) a thread-safety gap in the sigdefault set computation that exists in the fork path too but has a slightly different risk profile here, and (2) the test validates signal delivery but doesn't specifically exercise signal mask inheritance. Neither is blocking — the first is inherent to the design and the second is a test quality observation — but they should be consciously evaluated by the human reviewers.

---

Detailed Findings

✅ Error handling and resource cleanup — Correct

All error paths properly destroy posix_spawnattr_t and posix_spawn_file_actions_t resources. The *childPid = -1 initialization at line 227 ensures consistent error reporting. The pattern of saving result into saved_errno before cleanup and restoring it to errno before return is correct.

✅ Signal default set construction — Correct parity with fork path

The loop at lines 260-275 correctly mirrors the fork path's child-side signal handler reset (lines 435-451): iterates signals 1..NSIG, skips SIGKILL/SIGSTOP, uses handler_from_sigaction to detect custom handlers, and only adds those to the sigdefault set. SIG_IGN and SIG_DFL handlers are preserved, matching fork behavior.

✅ Signal mask capture — Correct

pthread_sigmask(SIG_SETMASK, NULL, &current_mask) at line 279 correctly queries the current mask without modifying it (per POSIX: when set is NULL, how is ignored). This captures the pre-spawn mask, equivalent to the fork path's old_signal_set (which captures the mask before sigfillset blocks everything). The semantics are equivalent because the posix_spawn path doesn't need the temporary signal blocking that the fork path requires.

✅ posix_spawn_file_actions_addchdir_np availability — Correct

The minimum macOS deployment target is 14.0 (set in eng/native/configurecompiler.cmake:758). posix_spawn_file_actions_addchdir_np was introduced in macOS 10.15. No availability guard is needed.

✅ #include <spawn.h> guard — Acceptable

The include is under #ifdef __APPLE__ (line 32) while usage is under #if defined(TARGET_OSX) (line 240). Including the header on extra Apple platforms is harmless and keeps the include block simple.

⚠️ Thread safety of sigdefault set computation — Inherent limitation (advisory, not blocking)

The sigdefault set is computed by walking signal dispositions in the parent process (lines 260-275). Signal dispositions are process-wide, so another thread could register/unregister a handler between this scan and the posix_spawn call. However, this is the same inherent race that exists in the fork path (lines 435-451) — the fork path scans signal dispositions in the child after fork, but before exec, during which time other threads in the parent could also be modifying handlers. The posix_spawn path is no worse. Both GPT and Gemini flagged this; it's real but inherent to the design.

⚠️ Test doesn't exercise mask inheritance specifically — Follow-up improvement (advisory, not blocking)

The test ChildProcess_InheritsParentSignalMask_CanReceiveSignals validates that a child process can receive SIGCONT when the parent has a SIGCONT handler registered. This exercises the POSIX_SPAWN_SETSIGDEF behavior (custom handlers reset to SIG_DFL in child). However, it doesn't explicitly modify the parent's signal mask nor verify the child inherits it, so it wouldn't detect a regression where posix_spawnattr_setsigmask is broken (e.g., always passes an empty mask). Both GPT and the existing Copilot review comment flagged this. The test is still valuable for validating the overall signal cooperation — a stronger mask-inheritance test could be a follow-up.

💡 SignalReceivedMessage is unused in assertions

SignalReceivedMessage is declared at line 1072 and written by the child's signal handler (line 1094), but is never verified by any assertion. The test only asserts ChildReadyMessage via AssertRemoteProcessStandardOutputLine. Consider removing the unused constant, or adding an assertion that verifies it appears in the child's output after the signal is sent.

💡 Double comment typo fixed

Line 306 had // // Change working directory if specified — fixed in commit 54f7706.

💡 Nit: blank line before struct sigaction sa_old;

Per @stephentoub's review comment — fixed in commit 54f7706.

@copilot revert your last commit that introduced a bug!!! Just move the local variable declaration before it's being used for the first time (current_mask)

That was already addressed in commit c0afd23 — the last commit was reverted and sigset_t current_mask; was simply moved to line 278, right before its first use at pthread_sigmask.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• developer.apple.com
  • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node /home/REDACTED/work/_temp/ghcca-node/node/bin/node --enable-source-maps /home/REDACTED/work/_temp/copilot-developer-action-main/dist/index.js (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[github-actions]

🤖 Copilot Code Review — PR #126063

Note

This review was generated by GitHub Copilot.

Holistic Assessment

Motivation: Justified. fork() is deprecated on macOS and posix_spawn is the recommended replacement. Using posix_spawn avoids the overhead of COW page table duplication and is the idiomatic macOS API for process creation. The change is appropriately scoped to macOS (TARGET_OSX) only, and falls back to the existing fork path when credential setting is needed (since posix_spawn does not support setuid/setgid).

Approach: Sound. The implementation correctly mirrors the fork path's signal-handling behavior (resetting only custom handlers, preserving SIG_IGN/SIG_DFL, propagating the parent's signal mask), handles stdio redirection via posix_spawn_file_actions_adddup2, and uses posix_spawn_file_actions_addchdir_np for working directory changes. Error handling properly cleans up posix_spawnattr_t and posix_spawn_file_actions_t on all failure paths. The access() check and *childPid = -1 initialization are lifted above both paths to reduce code duplication.

Summary: ⚠️ Needs Human Review. The code is well-structured and I found no correctness bugs. However, this is a significant change to the process-creation path on macOS, and a few behavioral differences from the fork path (detailed below) warrant human confirmation. A domain expert should verify the exec-error propagation semantics and confirm the fd-handling differences are acceptable.

---

Detailed Findings

✅ Exec-Error Propagation — Correct for macOS

The fork path uses a waitForChildToExecPipe (CLOEXEC pipe) to detect exec failures in the child and propagate the errno back to the parent. The posix_spawn path does not have this mechanism, which initially appears concerning. However, on macOS, posix_spawn() reports exec errors synchronously via its return value — unlike Linux, where posix_spawn may succeed even if exec later fails. This means ENOEXEC, ENOENT, EACCES, etc. are all returned directly from the posix_spawn() call at line 317, which then sets errno and returns -1 at lines 322-326. The managed side's ENOEXEC fallback logic (for UseShellExecute, Process.Unix.cs:501-504) continues to work correctly.

The fork path's pipe also served as a "wait until exec" synchronization barrier. With posix_spawn, this is implicit — posix_spawn() does not return until the exec has been attempted.

(Verified by: tracing the managed call chain from Process.Unix.cs:481 through Interop.ForkAndExecProcess.cs:50-55, and confirming macOS posix_spawn semantics. Also verified by GPT-5.4 model review.)

✅ Signal Handling — Matches Fork Path

The posix_spawn path's signal-reset logic (lines 258-276) closely mirrors the fork path's (lines 436-452):

• Both iterate signals 1..NSIG, skip SIGKILL/SIGSTOP
• Both use handler_from_sigaction() to inspect current disposition
• Both preserve SIG_IGN and SIG_DFL, only resetting custom handlers
• The posix_spawn path captures the parent's current signal mask via pthread_sigmask and passes it to the child via POSIX_SPAWN_SETSIGMASK, matching the fork path's pthread_sigmask(SIG_SETMASK, &old_signal_set, ...) in the child

✅ posix_spawn_file_actions_addchdir_np Availability — Safe

This non-portable extension (_np suffix) is available on macOS 10.15+. The repo's minimum macOS deployment target is 14.0 (eng/native/configurecompiler.cmake:758), well above the availability floor. The #if defined(TARGET_OSX) guard ensures this code only compiles for macOS (not iOS, tvOS, or Mac Catalyst, where HAVE_FORK is unset).

✅ Error Cleanup — Correct

All error paths properly destroy posix_spawnattr_t and posix_spawn_file_actions_t before returning. The posix_spawn_file_actions_init is called inside the compound conditional (line 295), and its corresponding _destroy is only called after initialization succeeds (line 310 in the error path, line 320 in the success path). posix_spawnattr_destroy is called on all paths after posix_spawnattr_init.

💡 dup2 Self-FD Behavioral Difference — Minor

The fork path (line 457) checks stdinFd != STDIN_FILENO before calling dup2, skipping the call when the fd is already the target. The posix_spawn path (line 304) does not perform this check, so posix_spawn_file_actions_adddup2(&file_actions, 0, 0) will be called when stdin is not redirected.

Per POSIX, this is a no-op for dup2. For posix_spawn_file_actions_adddup2, when fildes == fildes2, the action is a no-op except it clears FD_CLOEXEC on the descriptor. Since the standard I/O descriptors (0/1/2) are not expected to have FD_CLOEXEC set, this is benign in practice. The managed side creates these handles via Console.OpenStandardInputHandle() → SafeFileHandle(fd, ownsHandle: false) without setting FD_CLOEXEC.

Not a bug, but worth confirming this difference is intentional.

💡 Test: Unused Assertion Opportunity — Minor

The test defines SignalReceivedMessage (line 1084) and writes it to stdout in the child's signal handler (line 1106), but the parent never reads/asserts this second output line — it only asserts the "Child ready" line (line 1115). Signal receipt is verified indirectly through the ManualResetEvent.WaitOne succeeding (which gates the child's exit code). This is sufficient but reading the second stdout line would make the test more explicit.

💡 Test: Signal Mask Not Exercised — Minor

(Also flagged by GPT-5.4 model review.)

The test validates signal handler inheritance (SIGCONT handler in parent → child can still receive SIGCONT), which exercises POSIX_SPAWN_SETSIGDEF. However, it does not exercise the POSIX_SPAWN_SETSIGMASK behavior — the parent never blocks/alters its signal mask before spawning the child. A separate test that blocks a signal in the parent and verifies the child inherits the expected mask would provide additional confidence in the mask propagation logic. This could be a follow-up.

✅ Preprocessor Guards — Correct

The #if HAVE_FORK || defined(TARGET_OSX) guard at line 223 is technically redundant on macOS (where both are true), but provides correct defensive future-proofing. The structure ensures:

• macOS without credentials → posix_spawn path
• macOS with credentials → fork path
• Non-macOS with HAVE_FORK → fork path
• Platforms without fork or posix_spawn → returns -1 (existing behavior)

Generated by Code Review for issue #126063 · ◷

[adamsitnik]

All the Unix CI legs are green. The perf win is HUGE, please let me know if you need any changes to get it merged.

[wfurt]

LGTM. I left few comments, mostly technicalities. Feel free to update it as separate change if you want to get this in @adamsitnik

[wfurt]

Guard: #if defined(TARGET_OSX), placed as the first code block in the function — restricts the path to macOS only > > (excludes iOS, tvOS, MacCatalyst, watchOS)

why not there?

[adamsitnik]

why not there?

posix_spawn is supported on iOS, but has very strict requirements and we are not sure whether we want to support it on iOS or not (#126097 (comment))

[adamsitnik]

/ba-g failures are unrelated