Add ProcessStartInfo.StartDetached to start processes detached from parent session/console

[Copilot]

Description

Adds ProcessStartInfo.StartDetached — a cross-platform way to start a process detached from the parent's session and console, such that it outlives the parent.

API surface

// ref/System.Diagnostics.Process.cspublicsealedclassProcessStartInfo{publicboolStartDetached{get;set;}}

Behavior by platform

Platform	Mechanism
Windows	DETACHED_PROCESS creation flag
macOS	POSIX_SPAWN_SETSID via posix_spawn
Other Unix	setsid() in child after fork, before exec

Key semantics

• ThrowsInvalidOperationException when combined with UseShellExecute = true
• Standard handles (stdin/stdout/stderr) not explicitly set are automatically redirected to the null device (NUL on Windows, /dev/null on Unix) — a single File.OpenNullHandle() is opened at most once and disposed after the process starts; override with RedirectStandardInput/RedirectStandardOutput/RedirectStandardError or StandardInputHandle/StandardOutputHandle/StandardErrorHandle

Example

varpsi=newProcessStartInfo("myserver"){StartDetached=true};// stdin/stdout/stderr automatically go to NUL/dev/nullProcess.Start(psi);

Files changed

• ProcessStartInfo.cs / ref — new property with XML docs linking to the Windows SDK DETACHED_PROCESS documentation; remarks note that UseShellExecute = true will throw; StartDetached placed in alphabetical order in the ref assembly (after StandardOutputHandle, before UseCredentialsForNetworkingOnly)
• Strings.resx — resource key StartDetachedNotCompatible; message states the property cannot be used with UseShellExecute set to true
• Process.cs — opens a single null device handle outside the lock region, reuses it for all unset stdio handles; existing finally block handles disposal
• SafeProcessHandle.cs — if/else if/else blocks; single nullDeviceHandle opened at most once; disposed in a finally block after StartCore
• SafeProcessHandle.Windows.cs — DETACHED_PROCESS flag; constant added to Interop.ProcessOptions.cs
• Interop.ForkAndExecProcess.cs / SafeProcessHandle.Unix.cs — thread startDetached through managed interop chain; fixed indentation of ForkAndExecProcess(...) call inside fixed block
• pal_process.c / pal_process.h / pal_process_wasi.c — native implementation: POSIX_SPAWN_SETSID (macOS), if (startDetached && setsid() == -1) check (other Unix)
• ProcessTests.cs — cross-platform StartDetached_GrandchildSurvivesSignalingParent test guarded by IsNotNanoServerAndRemoteExecutorSupported (PlatformDetection.IsNotWindowsNanoServer && RemoteExecutor.IsSupported); child started with StartDetached=true (own process group on Unix) or CreateNewProcessGroup=true (Windows); kills child's process group with SIGKILL on Unix and SIGQUIT (mapped to CTRL_BREAK_EVENT) on Windows via SendSignal(..., entireProcessGroup: true); grandchild-alive check uses deterministic polling (loop with 20 ms sleep up to WaitInMS) instead of a fixed Thread.Sleep; validates that detached grandchild survives while non-detached grandchild is killed; StartDetached_StartsAndExitsSuccessfully smoke test starts a process with StartDetached=true via RemoteExecutor and verifies it completes successfully
• ProcessTests.Unix.cs — SendSignal(..., entireProcessGroup: true) passes the given signal through Interop.Sys.GetPlatformSignalNumber to kill(-pgid, signalNumber), keeping the helper generic (not hardcoded to any particular signal); call site passes PosixSignal.SIGKILL on Unix for reliable process-group termination
• ProcessTests.Windows.cs — SendSignal documents that entireProcessGroup has no distinct effect on Windows (via _ = entireProcessGroup) since GenerateConsoleCtrlEvent is inherently process-group scoped
• ProcessHandlesTests.cs — [Fact] test for UseShellExecute validation

Changes

• Adds a new cross-platform ProcessStartInfo.StartDetached property
• Redirects standard I/O to null device (single handle, properly disposed) when detached and no explicit handle is provided
• Platform-specific implementation: DETACHED_PROCESS (Windows), POSIX_SPAWN_SETSID (macOS), setsid() (Linux/other Unix)
• Validation: throws when combined with UseShellExecute = true only (CreateNoWindow is not rejected — DETACHED_PROCESS and CREATE_NO_WINDOW are compatible per Windows API semantics)
• XML docs link to the Windows SDK DETACHED_PROCESS documentation; only UseShellExecute listed as incompatible
• Tests covering validation (UseShellExecute), cross-platform process-group isolation (detached grandchild survives, non-detached grandchild is killed), and basic smoke test (StartDetached_StartsAndExitsSuccessfully); all skipped on Windows Nano Server and where RemoteExecutor is unsupported
• SendSignal Unix helper kept generic (signal passed via GetPlatformSignalNumber); call site uses SIGKILL on Unix for reliable group termination
• Deterministic polling replaces fixed Thread.Sleep(200) in grandchild-survival assertion
• Windows SendSignal explicitly discards entireProcessGroup with a clarifying comment that GenerateConsoleCtrlEvent is inherently group-scoped
• StartDetached placed in alphabetical order in the ref assembly (after StandardOutputHandle, before UseCredentialsForNetworkingOnly)

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

@copilot address my feedback

[Copilot]

@copilot address my feedback

All feedback addressed in a6e4206. Summary of changes:

• SafeProcessHandle.cs: if/else if/else blocks, single null device handle, disposed in finally after StartCore
• Process.cs: single null device handle opened once after the lock, reused for all stdio
• ProcessStartInfo.cs docs: split into 3 paragraphs with StandardInputHandle/OutputHandle/ErrorHandle refs
• ProcessStartInfo.cs condition: added && string.IsNullOrEmpty(UserName) to the StartDetached branch
• pal_process.c: reduced setsid check nesting
• StartDetachedTests.cs: [Fact] for validation test; removed mobile platform skips; SIGHUP test skips only Windows; added using System.Runtime.InteropServices for PosixSignal

[Copilot]

Pull request overview

This PR introduces a new ProcessStartInfo.StartDetached API to start processes detached from the parent’s session/console across platforms, with additional safety defaults to avoid leaking standard handles and inheritable resources into long-lived detached processes.

Changes:

• Adds ProcessStartInfo.StartDetached to the public surface area (ref + implementation) with validation against UseShellExecute = true.
• Implements platform-specific detaching: DETACHED_PROCESS (Windows), POSIX_SPAWN_SETSID (macOS), and setsid() (other Unix), threading the flag through managed/native interop.
• When detached, defaults stdio to the null device (unless explicitly configured) and restricts handle inheritance by default.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/native/libs/System.Native/pal_process.h	Extends native fork/exec API with startDetached parameter.
src/native/libs/System.Native/pal_process.c	Implements session detachment (POSIX_SPAWN_SETSID / setsid()) based on startDetached.
src/native/libs/System.Native/pal_process_wasi.c	Updates WASI stub signature for startDetached.
src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs	Adds managed interop parameter and forwards startDetached to native.
src/libraries/Common/src/Interop/Windows/Advapi32/Interop.ProcessOptions.cs	Introduces DETACHED_PROCESS constant for process creation flags.
src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs	Adds ProcessStartInfo.StartDetached to the reference assembly API surface.
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs	Adds property + validation + default empty inherited-handles list for detached starts.
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs	Uses a single null-device handle for unset stdio when detached; opens it outside the lock.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs	Applies detached stdio-to-null behavior for SafeProcessHandle.Start.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs	Threads StartDetached through to Interop.Sys.ForkAndExecProcess.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs	Adds DETACHED_PROCESS to creation flags when StartDetached is set.
src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx	Adds error message for StartDetached + UseShellExecute incompatibility.
src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj	Includes new StartDetachedTests.cs in the test project.
src/libraries/System.Diagnostics.Process/tests/StartDetachedTests.cs	Adds validation and behavioral tests for detached starts (including a Unix SIGHUP scenario).

[adamsitnik]

@copilot address my feedback

[Copilot]

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

Note

This review was generated by Copilot AI using three parallel models (Claude Sonnet 4.5, Gemini 2.5 Pro, GPT-4.1) and synthesized into a single report.

🤖 Copilot Code Review — PR #126632

Holistic Assessment

Motivation: Well-justified. A cross-platform API for starting daemon/background processes is a genuine gap — today users must resort to platform-specific flags or shell scripting. Issue #125838 carries the api-approved label and the PR directly addresses it.

Approach: Correct on all three platforms: DETACHED_PROCESS on Windows, POSIX_SPAWN_SETSID on macOS, and setsid() after fork() before exec() on other Unix — all established patterns. Defaulting unset stdio handles to the null device prevents accidental handle inheritance into long-lived detached processes.

Summary: ✅ LGTM. All three models independently raised two concerns that, after careful verification, are false positives:

---

Detailed Findings

✅ Handle disposal in Process.cs — Correctly handled (false positive resolved)

All three models flagged that the nullDeviceHandle in Process.cs might be disposed multiple times (once per childInputHandle, childOutputHandle, childErrorHandle in the finally block) if all three point to the same SafeFileHandle instance. This is safe: SafeHandle.Dispose() uses Interlocked.CompareExchange to set STATE_CLOSED atomically — subsequent calls are no-ops. The handle is properly disposed exactly once (OS-level), regardless of how many managed variables reference it.

✅ POSIX_SPAWN_SETSID availability — No issue

One model raised a concern about POSIX_SPAWN_SETSID not being available on older macOS. The runtime's CMAKE_OSX_DEPLOYMENT_TARGET is 14.0 (Sonoma). POSIX_SPAWN_SETSID was introduced in macOS 10.15, so this is fine.

✅ API Approval — Shape matches api-approved issue #125838

The ref assembly declares public bool StartDetached { get { throw null; } set { } } in alphabetical order (after StandardOutputHandle, before UseCredentialsForNetworkingOnly). This matches the approved shape exactly.

✅ Null device handle lifetime in SafeProcessHandle.cs

The using SafeFileHandle? nullDeviceHandle is disposed at end of scope, after StartCore returns. StartCore passes the handle to native code which immediately dup2s it — the managed-side dispose after return is safe.

✅ setsid() error handling

pal_process.c correctly propagates setsid() failures via ExitChild(waitForChildToExecPipe[WRITE_END_OF_PIPE], errno), so the managed caller receives the error.

✅ Test quality

• StartDetached_GrandchildSurvivesSignalingParent uses deterministic polling (20 ms increments up to WaitInMS) and guards with IsNotNanoServerAndRemoteExecutorSupported.
• StartDetached_StartsAndExitsSuccessfully provides a lightweight smoke test.
• StartDetached_ThrowsOnUseShellExecute covers the validation path.
• Windows SendSignal correctly discards entireProcessGroup with a documented explanation.

💡 Consider a Simpler Test for Basic StartDetached Functionality

The existing test (StartDetached_GrandchildSurvivesSignalingParent) covers the complex grandchild-survival scenario. A si...

The StartDetached_StartsAndExitsSuccessfully smoke test was added in commit 820aef5 and is already present in the current PR.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

[github-actions]

🤖 Copilot Code Review — PR #126632

Note

This review was generated by GitHub Copilot (Claude Opus 4.6). Full source files, git history, API approval issue, and existing review comments were analyzed.

Holistic Assessment

Motivation: Adding ProcessStartInfo.StartDetached addresses a well-known, legitimate need — starting daemon-like processes that are detached from the parent's session/console and survive parent termination. The approved API issue (#125838) demonstrates clear real-world use cases. Justified. ✅

Approach: The implementation correctly uses platform-appropriate mechanisms (DETACHED_PROCESS on Windows, POSIX_SPAWN_SETSID on macOS, setsid() on Linux). The single boolean property is minimal, consistent with existing ProcessStartInfo patterns (CreateNoWindow, CreateNewProcessGroup), and the defaulting of stdio to the null device is the right behavior for detached processes.

Summary: ⚠️Needs Human Review. The implementation is sound — the API matches the approved shape from issue #125838, native code is correct, and tests cover the critical scenarios. I'm flagging for human review because of a handle ownership pattern divergence between Process.Start() and SafeProcessHandle.Start() that, while functionally correct due to SafeHandle.Dispose() idempotency, warrants a human judgment call on whether to align the patterns.

---

Detailed Findings

✅ API Approval — Verified against issue #125838

The api-approved label was applied by @bartonjs on 2025-03-24. The approved API shape includes:

publicsealedpartialclassProcessStartInfo{publicboolStartDetached{get;set;}}

The implementation matches exactly — no extra public surface, correct property signature, correct placement in alphabetical order in the ref assembly (after StandardOutputHandle, before UseCredentialsForNetworkingOnly). ✅

---

✅ Native Implementation — Correct platform mechanisms

• macOS (posix_spawn path): Uses POSIX_SPAWN_SETSID flag (inside #if defined(TARGET_OSX) block). This is an Apple-specific extension available since macOS 10.14; .NET requires macOS 10.15+, so always available. ✅
• Linux (fork path): Calls setsid() after fork, before exec (pal_process.c). The freshly forked child always has PID ≠ PGID, so setsid() will succeed. Placement after dup2 but before credential setup and exec is correct. ✅
• WASI stub: Updated with (void)startDetached to suppress unused parameter warning. ✅

---

✅ Windows Implementation — Correct flag usage

DETACHED_PROCESS = 0x00000008 is correctly OR-ed into creation flags. The decision to allow DETACHED_PROCESS + CREATE_NO_WINDOW together was explicitly reviewed and approved by @jkotas (commit 389dcf7 removed the CreateNoWindow guard, noting the two flags are compatible per Windows API semantics — DETACHED_PROCESS takes precedence). ✅

---

✅ Validation Logic — Correct and complete

• StartDetached + UseShellExecute correctly throws InvalidOperationException ✅
• Error message stored in .resx following conventions ✅
• The StartDetached parameter threads correctly through the entire call chain: ProcessStartInfo → SafeProcessHandle.ForkAndExecProcess → P/Invoke → pal_process.c ✅

---

⚠️ Handle Ownership — Inconsistent pattern between Process.Start() and SafeProcessHandle.Start()

SafeProcessHandle.Start() uses the clean using pattern:

usingSafeFileHandle?nullDeviceHandle=startInfo.StartDetached&&(childInputHandleisnull||childOutputHandleisnull||childErrorHandleisnull)?File.OpenNullHandle():null;

The handle is owned by a single using variable and disposed exactly once.

Process.Start() assigns the same File.OpenNullHandle() result to up to three variables (childInputHandle, childOutputHandle, childErrorHandle), each then independently disposed in the finally block, resulting in up to three Dispose() calls on the same SafeFileHandle.

This is functionally safe — SafeHandle.Dispose() is idempotent by design; subsequent calls are no-ops. The finally block comment even acknowledges non-owning handles. I also see that @adamsitnik specifically requested this pattern in review: "call File.OpenNullHandle() at max once, ensure it gets disposed after process is started."

However, the shared-handle-with-multiple-dispose pattern in Process.Start() is inconsistent with the cleaner SafeProcessHandle.Start() approach. A human reviewer should confirm whether this divergence is intentional or whether a dedicated nullDeviceHandle local with using (matching the other call site) would be preferable.

Not blocking — code is correct as-is.

---

✅ Test Quality — Well-designed coverage

Three tests cover the critical scenarios:

1. StartDetached_ThrowsOnUseShellExecute — Validates the incompatibility check via SafeProcessHandle.Start(). ✅
2. StartDetached_StartsAndExitsSuccessfully — Smoke test verifying a detached process runs and exits normally. ✅
3. StartDetached_GrandchildSurvivesSignalingParent — [Theory] with true/false testing that a detached grandchild survives process-group kill while a non-detached one is killed. Good use of RemoteExecutor, proper cleanup via finally { grandchild.Kill(); }, and appropriate conditional attributes. ✅

The 300ms timeout for the "alive" assertion is reasonable since the grandchild does Thread.Sleep(Timeout.Infinite) — it cannot exit on its own, and setsid() ensures the SIGKILL to the parent's process group doesn't reach it. ✅

---

💡 XML Documentation — Minor clarity opportunity

The XML docs on StartDetached reference StandardInputHandle, StandardOutputHandle, StandardErrorHandle as properties that override null device defaulting. When using Process.Start() (the more common API path), RedirectStandardInput/RedirectStandardOutput/RedirectStandardError also override the null device behavior. The docs already mention these in the first <para>, but the "unless explicitly configured" phrasing in the second <para> could be slightly more inclusive.

Minor — not blocking.

Generated by Code Review for issue #126632 · ◷

[adamsitnik]

/ba-g unrelated

[Copilot]

Description

Adds ProcessStartInfo.StartDetached — a cross-platform way to start a process detached from the parent's session and console, such that it outlives the parent.

API surface

// ref/System.Diagnostics.Process.cspublicsealedclassProcessStartInfo{publicboolStartDetached{get;set;}}

Behavior by platform

Platform	Mechanism
Windows	DETACHED_PROCESS creation flag
macOS	POSIX_SPAWN_SETSID via posix_spawn
Other Unix	setsid() in child after fork, before exec

Key semantics

• ThrowsInvalidOperationException when combined with UseShellExecute = true
• Standard handles (stdin/stdout/stderr) not explicitly set are automatically redirected to the null device (NUL on Windows, /dev/null on Unix) — a single File.OpenNullHandle() is opened at most once and disposed after the process starts; override with RedirectStandardInput/RedirectStandardOutput/RedirectStandardError or StandardInputHandle/StandardOutputHandle/StandardErrorHandle

Example

varpsi=newProcessStartInfo("myserver"){StartDetached=true};// stdin/stdout/stderr automatically go to NUL/dev/nullProcess.Start(psi);

Files changed

• ProcessStartInfo.cs / ref — new property with XML docs linking to the Windows SDK DETACHED_PROCESS documentation; remarks note that UseShellExecute = true will throw; StartDetached placed in alphabetical order in the ref assembly (after StandardOutputHandle, before UseCredentialsForNetworkingOnly)
• Strings.resx — resource key StartDetachedNotCompatible; message states the property cannot be used with UseShellExecute set to true
• Process.cs — opens a single null device handle outside the lock region, reuses it for all unset stdio handles; existing finally block handles disposal
• SafeProcessHandle.cs — if/else if/else blocks; single nullDeviceHandle opened at most once; disposed in a finally block after StartCore
• SafeProcessHandle.Windows.cs — DETACHED_PROCESS flag; constant added to Interop.ProcessOptions.cs
• Interop.ForkAndExecProcess.cs / SafeProcessHandle.Unix.cs — thread startDetached through managed interop chain; fixed indentation of ForkAndExecProcess(...) call inside fixed block
• pal_process.c / pal_process.h / pal_process_wasi.c — native implementation: POSIX_SPAWN_SETSID (macOS), if (startDetached && setsid() == -1) check (other Unix)
• ProcessTests.cs — cross-platform StartDetached_GrandchildSurvivesSignalingParent test guarded by IsNotNanoServerAndRemoteExecutorSupported (PlatformDetection.IsNotWindowsNanoServer && RemoteExecutor.IsSupported); child started with StartDetached=true (own process group on Unix) or CreateNewProcessGroup=true (Windows); kills child's process group with SIGKILL on Unix and SIGQUIT (mapped to CTRL_BREAK_EVENT) on Windows via SendSignal(..., entireProcessGroup: true); grandchild-alive check uses deterministic polling (loop with 20 ms sleep up to WaitInMS) instead of a fixed Thread.Sleep; validates that detached grandchild survives while non-detached grandchild is killed; StartDetached_StartsAndExitsSuccessfully smoke test starts a process with StartDetached=true via RemoteExecutor and verifies it completes successfully
• ProcessTests.Unix.cs — SendSignal(..., entireProcessGroup: true) passes the given signal through Interop.Sys.GetPlatformSignalNumber to kill(-pgid, signalNumber), keeping the helper generic (not hardcoded to any particular signal); call site passes PosixSignal.SIGKILL on Unix for reliable process-group termination
• ProcessTests.Windows.cs — SendSignal documents that entireProcessGroup has no distinct effect on Windows (via _ = entireProcessGroup) since GenerateConsoleCtrlEvent is inherently process-group scoped
• ProcessHandlesTests.cs — [Fact] test for UseShellExecute validation

Changes

• Adds a new cross-platform ProcessStartInfo.StartDetached property
• Redirects standard I/O to null device (single handle, properly disposed) when detached and no explicit handle is provided
• Platform-specific implementation: DETACHED_PROCESS (Windows), POSIX_SPAWN_SETSID (macOS), setsid() (Linux/other Unix)
• Validation: throws when combined with UseShellExecute = true only (CreateNoWindow is not rejected — DETACHED_PROCESS and CREATE_NO_WINDOW are compatible per Windows API semantics)
• XML docs link to the Windows SDK DETACHED_PROCESS documentation; only UseShellExecute listed as incompatible
• Tests covering validation (UseShellExecute), cross-platform process-group isolation (detached grandchild survives, non-detached grandchild is killed), and basic smoke test (StartDetached_StartsAndExitsSuccessfully); all skipped on Windows Nano Server and where RemoteExecutor is unsupported
• SendSignal Unix helper kept generic (signal passed via GetPlatformSignalNumber); call site uses SIGKILL on Unix for reliable group termination
• Deterministic polling replaces fixed Thread.Sleep(200) in grandchild-survival assertion
• Windows SendSignal explicitly discards entireProcessGroup with a clarifying comment that GenerateConsoleCtrlEvent is inherently group-scoped
• StartDetached placed in alphabetical order in the ref assembly (after StandardOutputHandle, before UseCredentialsForNetworkingOnly)

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

@copilot address my feedback

[Copilot]

@copilot address my feedback

All feedback addressed in a6e4206. Summary of changes:

• SafeProcessHandle.cs: if/else if/else blocks, single null device handle, disposed in finally after StartCore
• Process.cs: single null device handle opened once after the lock, reused for all stdio
• ProcessStartInfo.cs docs: split into 3 paragraphs with StandardInputHandle/OutputHandle/ErrorHandle refs
• ProcessStartInfo.cs condition: added && string.IsNullOrEmpty(UserName) to the StartDetached branch
• pal_process.c: reduced setsid check nesting
• StartDetachedTests.cs: [Fact] for validation test; removed mobile platform skips; SIGHUP test skips only Windows; added using System.Runtime.InteropServices for PosixSignal

[Copilot]

Pull request overview

This PR introduces a new ProcessStartInfo.StartDetached API to start processes detached from the parent’s session/console across platforms, with additional safety defaults to avoid leaking standard handles and inheritable resources into long-lived detached processes.

Changes:

• Adds ProcessStartInfo.StartDetached to the public surface area (ref + implementation) with validation against UseShellExecute = true.
• Implements platform-specific detaching: DETACHED_PROCESS (Windows), POSIX_SPAWN_SETSID (macOS), and setsid() (other Unix), threading the flag through managed/native interop.
• When detached, defaults stdio to the null device (unless explicitly configured) and restricts handle inheritance by default.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/native/libs/System.Native/pal_process.h	Extends native fork/exec API with startDetached parameter.
src/native/libs/System.Native/pal_process.c	Implements session detachment (POSIX_SPAWN_SETSID / setsid()) based on startDetached.
src/native/libs/System.Native/pal_process_wasi.c	Updates WASI stub signature for startDetached.
src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs	Adds managed interop parameter and forwards startDetached to native.
src/libraries/Common/src/Interop/Windows/Advapi32/Interop.ProcessOptions.cs	Introduces DETACHED_PROCESS constant for process creation flags.
src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs	Adds ProcessStartInfo.StartDetached to the reference assembly API surface.
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs	Adds property + validation + default empty inherited-handles list for detached starts.
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs	Uses a single null-device handle for unset stdio when detached; opens it outside the lock.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs	Applies detached stdio-to-null behavior for SafeProcessHandle.Start.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs	Threads StartDetached through to Interop.Sys.ForkAndExecProcess.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs	Adds DETACHED_PROCESS to creation flags when StartDetached is set.
src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx	Adds error message for StartDetached + UseShellExecute incompatibility.
src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj	Includes new StartDetachedTests.cs in the test project.
src/libraries/System.Diagnostics.Process/tests/StartDetachedTests.cs	Adds validation and behavioral tests for detached starts (including a Unix SIGHUP scenario).

[adamsitnik]

@copilot address my feedback

[Copilot]

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

Note

This review was generated by Copilot AI using three parallel models (Claude Sonnet 4.5, Gemini 2.5 Pro, GPT-4.1) and synthesized into a single report.

🤖 Copilot Code Review — PR #126632

Holistic Assessment

Motivation: Well-justified. A cross-platform API for starting daemon/background processes is a genuine gap — today users must resort to platform-specific flags or shell scripting. Issue #125838 carries the api-approved label and the PR directly addresses it.

Approach: Correct on all three platforms: DETACHED_PROCESS on Windows, POSIX_SPAWN_SETSID on macOS, and setsid() after fork() before exec() on other Unix — all established patterns. Defaulting unset stdio handles to the null device prevents accidental handle inheritance into long-lived detached processes.

Summary: ✅ LGTM. All three models independently raised two concerns that, after careful verification, are false positives:

---

Detailed Findings

✅ Handle disposal in Process.cs — Correctly handled (false positive resolved)

All three models flagged that the nullDeviceHandle in Process.cs might be disposed multiple times (once per childInputHandle, childOutputHandle, childErrorHandle in the finally block) if all three point to the same SafeFileHandle instance. This is safe: SafeHandle.Dispose() uses Interlocked.CompareExchange to set STATE_CLOSED atomically — subsequent calls are no-ops. The handle is properly disposed exactly once (OS-level), regardless of how many managed variables reference it.

✅ POSIX_SPAWN_SETSID availability — No issue

One model raised a concern about POSIX_SPAWN_SETSID not being available on older macOS. The runtime's CMAKE_OSX_DEPLOYMENT_TARGET is 14.0 (Sonoma). POSIX_SPAWN_SETSID was introduced in macOS 10.15, so this is fine.

✅ API Approval — Shape matches api-approved issue #125838

The ref assembly declares public bool StartDetached { get { throw null; } set { } } in alphabetical order (after StandardOutputHandle, before UseCredentialsForNetworkingOnly). This matches the approved shape exactly.

✅ Null device handle lifetime in SafeProcessHandle.cs

The using SafeFileHandle? nullDeviceHandle is disposed at end of scope, after StartCore returns. StartCore passes the handle to native code which immediately dup2s it — the managed-side dispose after return is safe.

✅ setsid() error handling

pal_process.c correctly propagates setsid() failures via ExitChild(waitForChildToExecPipe[WRITE_END_OF_PIPE], errno), so the managed caller receives the error.

✅ Test quality

• StartDetached_GrandchildSurvivesSignalingParent uses deterministic polling (20 ms increments up to WaitInMS) and guards with IsNotNanoServerAndRemoteExecutorSupported.
• StartDetached_StartsAndExitsSuccessfully provides a lightweight smoke test.
• StartDetached_ThrowsOnUseShellExecute covers the validation path.
• Windows SendSignal correctly discards entireProcessGroup with a documented explanation.

💡 Consider a Simpler Test for Basic StartDetached Functionality

The existing test (StartDetached_GrandchildSurvivesSignalingParent) covers the complex grandchild-survival scenario. A si...

The StartDetached_StartsAndExitsSuccessfully smoke test was added in commit 820aef5 and is already present in the current PR.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

[github-actions]

🤖 Copilot Code Review — PR #126632

Note

This review was generated by GitHub Copilot (Claude Opus 4.6). Full source files, git history, API approval issue, and existing review comments were analyzed.

Holistic Assessment

Motivation: Adding ProcessStartInfo.StartDetached addresses a well-known, legitimate need — starting daemon-like processes that are detached from the parent's session/console and survive parent termination. The approved API issue (#125838) demonstrates clear real-world use cases. Justified. ✅

Approach: The implementation correctly uses platform-appropriate mechanisms (DETACHED_PROCESS on Windows, POSIX_SPAWN_SETSID on macOS, setsid() on Linux). The single boolean property is minimal, consistent with existing ProcessStartInfo patterns (CreateNoWindow, CreateNewProcessGroup), and the defaulting of stdio to the null device is the right behavior for detached processes.

Summary: ⚠️Needs Human Review. The implementation is sound — the API matches the approved shape from issue #125838, native code is correct, and tests cover the critical scenarios. I'm flagging for human review because of a handle ownership pattern divergence between Process.Start() and SafeProcessHandle.Start() that, while functionally correct due to SafeHandle.Dispose() idempotency, warrants a human judgment call on whether to align the patterns.

---

Detailed Findings

✅ API Approval — Verified against issue #125838

The api-approved label was applied by @bartonjs on 2025-03-24. The approved API shape includes:

publicsealedpartialclassProcessStartInfo{publicboolStartDetached{get;set;}}

The implementation matches exactly — no extra public surface, correct property signature, correct placement in alphabetical order in the ref assembly (after StandardOutputHandle, before UseCredentialsForNetworkingOnly). ✅

---

✅ Native Implementation — Correct platform mechanisms

• macOS (posix_spawn path): Uses POSIX_SPAWN_SETSID flag (inside #if defined(TARGET_OSX) block). This is an Apple-specific extension available since macOS 10.14; .NET requires macOS 10.15+, so always available. ✅
• Linux (fork path): Calls setsid() after fork, before exec (pal_process.c). The freshly forked child always has PID ≠ PGID, so setsid() will succeed. Placement after dup2 but before credential setup and exec is correct. ✅
• WASI stub: Updated with (void)startDetached to suppress unused parameter warning. ✅

---

✅ Windows Implementation — Correct flag usage

DETACHED_PROCESS = 0x00000008 is correctly OR-ed into creation flags. The decision to allow DETACHED_PROCESS + CREATE_NO_WINDOW together was explicitly reviewed and approved by @jkotas (commit 389dcf7 removed the CreateNoWindow guard, noting the two flags are compatible per Windows API semantics — DETACHED_PROCESS takes precedence). ✅

---

✅ Validation Logic — Correct and complete

• StartDetached + UseShellExecute correctly throws InvalidOperationException ✅
• Error message stored in .resx following conventions ✅
• The StartDetached parameter threads correctly through the entire call chain: ProcessStartInfo → SafeProcessHandle.ForkAndExecProcess → P/Invoke → pal_process.c ✅

---

⚠️ Handle Ownership — Inconsistent pattern between Process.Start() and SafeProcessHandle.Start()

SafeProcessHandle.Start() uses the clean using pattern:

usingSafeFileHandle?nullDeviceHandle=startInfo.StartDetached&&(childInputHandleisnull||childOutputHandleisnull||childErrorHandleisnull)?File.OpenNullHandle():null;

The handle is owned by a single using variable and disposed exactly once.

Process.Start() assigns the same File.OpenNullHandle() result to up to three variables (childInputHandle, childOutputHandle, childErrorHandle), each then independently disposed in the finally block, resulting in up to three Dispose() calls on the same SafeFileHandle.

This is functionally safe — SafeHandle.Dispose() is idempotent by design; subsequent calls are no-ops. The finally block comment even acknowledges non-owning handles. I also see that @adamsitnik specifically requested this pattern in review: "call File.OpenNullHandle() at max once, ensure it gets disposed after process is started."

However, the shared-handle-with-multiple-dispose pattern in Process.Start() is inconsistent with the cleaner SafeProcessHandle.Start() approach. A human reviewer should confirm whether this divergence is intentional or whether a dedicated nullDeviceHandle local with using (matching the other call site) would be preferable.

Not blocking — code is correct as-is.

---

✅ Test Quality — Well-designed coverage

Three tests cover the critical scenarios:

1. StartDetached_ThrowsOnUseShellExecute — Validates the incompatibility check via SafeProcessHandle.Start(). ✅
2. StartDetached_StartsAndExitsSuccessfully — Smoke test verifying a detached process runs and exits normally. ✅
3. StartDetached_GrandchildSurvivesSignalingParent — [Theory] with true/false testing that a detached grandchild survives process-group kill while a non-detached one is killed. Good use of RemoteExecutor, proper cleanup via finally { grandchild.Kill(); }, and appropriate conditional attributes. ✅

The 300ms timeout for the "alive" assertion is reasonable since the grandchild does Thread.Sleep(Timeout.Infinite) — it cannot exit on its own, and setsid() ensures the SIGKILL to the parent's process group doesn't reach it. ✅

---

💡 XML Documentation — Minor clarity opportunity

The XML docs on StartDetached reference StandardInputHandle, StandardOutputHandle, StandardErrorHandle as properties that override null device defaulting. When using Process.Start() (the more common API path), RedirectStandardInput/RedirectStandardOutput/RedirectStandardError also override the null device behavior. The docs already mention these in the first <para>, but the "unless explicitly configured" phrasing in the second <para> could be slightly more inclusive.

Minor — not blocking.

Generated by Code Review for issue #126632 · ◷

[adamsitnik]

/ba-g unrelated

[Copilot]

Description

Adds ProcessStartInfo.StartDetached — a cross-platform way to start a process detached from the parent's session and console, such that it outlives the parent.

API surface

// ref/System.Diagnostics.Process.cspublicsealedclassProcessStartInfo{publicboolStartDetached{get;set;}}

Behavior by platform

Platform	Mechanism
Windows	DETACHED_PROCESS creation flag
macOS	POSIX_SPAWN_SETSID via posix_spawn
Other Unix	setsid() in child after fork, before exec

Key semantics

• ThrowsInvalidOperationException when combined with UseShellExecute = true
• Standard handles (stdin/stdout/stderr) not explicitly set are automatically redirected to the null device (NUL on Windows, /dev/null on Unix) — a single File.OpenNullHandle() is opened at most once and disposed after the process starts; override with RedirectStandardInput/RedirectStandardOutput/RedirectStandardError or StandardInputHandle/StandardOutputHandle/StandardErrorHandle

Example

varpsi=newProcessStartInfo("myserver"){StartDetached=true};// stdin/stdout/stderr automatically go to NUL/dev/nullProcess.Start(psi);

Files changed

• ProcessStartInfo.cs / ref — new property with XML docs linking to the Windows SDK DETACHED_PROCESS documentation; remarks note that UseShellExecute = true will throw; StartDetached placed in alphabetical order in the ref assembly (after StandardOutputHandle, before UseCredentialsForNetworkingOnly)
• Strings.resx — resource key StartDetachedNotCompatible; message states the property cannot be used with UseShellExecute set to true
• Process.cs — opens a single null device handle outside the lock region, reuses it for all unset stdio handles; existing finally block handles disposal
• SafeProcessHandle.cs — if/else if/else blocks; single nullDeviceHandle opened at most once; disposed in a finally block after StartCore
• SafeProcessHandle.Windows.cs — DETACHED_PROCESS flag; constant added to Interop.ProcessOptions.cs
• Interop.ForkAndExecProcess.cs / SafeProcessHandle.Unix.cs — thread startDetached through managed interop chain; fixed indentation of ForkAndExecProcess(...) call inside fixed block
• pal_process.c / pal_process.h / pal_process_wasi.c — native implementation: POSIX_SPAWN_SETSID (macOS), if (startDetached && setsid() == -1) check (other Unix)
• ProcessTests.cs — cross-platform StartDetached_GrandchildSurvivesSignalingParent test guarded by IsNotNanoServerAndRemoteExecutorSupported (PlatformDetection.IsNotWindowsNanoServer && RemoteExecutor.IsSupported); child started with StartDetached=true (own process group on Unix) or CreateNewProcessGroup=true (Windows); kills child's process group with SIGKILL on Unix and SIGQUIT (mapped to CTRL_BREAK_EVENT) on Windows via SendSignal(..., entireProcessGroup: true); grandchild-alive check uses deterministic polling (loop with 20 ms sleep up to WaitInMS) instead of a fixed Thread.Sleep; validates that detached grandchild survives while non-detached grandchild is killed; StartDetached_StartsAndExitsSuccessfully smoke test starts a process with StartDetached=true via RemoteExecutor and verifies it completes successfully
• ProcessTests.Unix.cs — SendSignal(..., entireProcessGroup: true) passes the given signal through Interop.Sys.GetPlatformSignalNumber to kill(-pgid, signalNumber), keeping the helper generic (not hardcoded to any particular signal); call site passes PosixSignal.SIGKILL on Unix for reliable process-group termination
• ProcessTests.Windows.cs — SendSignal documents that entireProcessGroup has no distinct effect on Windows (via _ = entireProcessGroup) since GenerateConsoleCtrlEvent is inherently process-group scoped
• ProcessHandlesTests.cs — [Fact] test for UseShellExecute validation

Changes

• Adds a new cross-platform ProcessStartInfo.StartDetached property
• Redirects standard I/O to null device (single handle, properly disposed) when detached and no explicit handle is provided
• Platform-specific implementation: DETACHED_PROCESS (Windows), POSIX_SPAWN_SETSID (macOS), setsid() (Linux/other Unix)
• Validation: throws when combined with UseShellExecute = true only (CreateNoWindow is not rejected — DETACHED_PROCESS and CREATE_NO_WINDOW are compatible per Windows API semantics)
• XML docs link to the Windows SDK DETACHED_PROCESS documentation; only UseShellExecute listed as incompatible
• Tests covering validation (UseShellExecute), cross-platform process-group isolation (detached grandchild survives, non-detached grandchild is killed), and basic smoke test (StartDetached_StartsAndExitsSuccessfully); all skipped on Windows Nano Server and where RemoteExecutor is unsupported
• SendSignal Unix helper kept generic (signal passed via GetPlatformSignalNumber); call site uses SIGKILL on Unix for reliable group termination
• Deterministic polling replaces fixed Thread.Sleep(200) in grandchild-survival assertion
• Windows SendSignal explicitly discards entireProcessGroup with a clarifying comment that GenerateConsoleCtrlEvent is inherently group-scoped
• StartDetached placed in alphabetical order in the ref assembly (after StandardOutputHandle, before UseCredentialsForNetworkingOnly)

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

@copilot address my feedback

[Copilot]

@copilot address my feedback

All feedback addressed in a6e4206. Summary of changes:

• SafeProcessHandle.cs: if/else if/else blocks, single null device handle, disposed in finally after StartCore
• Process.cs: single null device handle opened once after the lock, reused for all stdio
• ProcessStartInfo.cs docs: split into 3 paragraphs with StandardInputHandle/OutputHandle/ErrorHandle refs
• ProcessStartInfo.cs condition: added && string.IsNullOrEmpty(UserName) to the StartDetached branch
• pal_process.c: reduced setsid check nesting
• StartDetachedTests.cs: [Fact] for validation test; removed mobile platform skips; SIGHUP test skips only Windows; added using System.Runtime.InteropServices for PosixSignal

[Copilot]

Pull request overview

This PR introduces a new ProcessStartInfo.StartDetached API to start processes detached from the parent’s session/console across platforms, with additional safety defaults to avoid leaking standard handles and inheritable resources into long-lived detached processes.

Changes:

• Adds ProcessStartInfo.StartDetached to the public surface area (ref + implementation) with validation against UseShellExecute = true.
• Implements platform-specific detaching: DETACHED_PROCESS (Windows), POSIX_SPAWN_SETSID (macOS), and setsid() (other Unix), threading the flag through managed/native interop.
• When detached, defaults stdio to the null device (unless explicitly configured) and restricts handle inheritance by default.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/native/libs/System.Native/pal_process.h	Extends native fork/exec API with startDetached parameter.
src/native/libs/System.Native/pal_process.c	Implements session detachment (POSIX_SPAWN_SETSID / setsid()) based on startDetached.
src/native/libs/System.Native/pal_process_wasi.c	Updates WASI stub signature for startDetached.
src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs	Adds managed interop parameter and forwards startDetached to native.
src/libraries/Common/src/Interop/Windows/Advapi32/Interop.ProcessOptions.cs	Introduces DETACHED_PROCESS constant for process creation flags.
src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs	Adds ProcessStartInfo.StartDetached to the reference assembly API surface.
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs	Adds property + validation + default empty inherited-handles list for detached starts.
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs	Uses a single null-device handle for unset stdio when detached; opens it outside the lock.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs	Applies detached stdio-to-null behavior for SafeProcessHandle.Start.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs	Threads StartDetached through to Interop.Sys.ForkAndExecProcess.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs	Adds DETACHED_PROCESS to creation flags when StartDetached is set.
src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx	Adds error message for StartDetached + UseShellExecute incompatibility.
src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj	Includes new StartDetachedTests.cs in the test project.
src/libraries/System.Diagnostics.Process/tests/StartDetachedTests.cs	Adds validation and behavioral tests for detached starts (including a Unix SIGHUP scenario).

[adamsitnik]

@copilot address my feedback

[Copilot]

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

Note

This review was generated by Copilot AI using three parallel models (Claude Sonnet 4.5, Gemini 2.5 Pro, GPT-4.1) and synthesized into a single report.

🤖 Copilot Code Review — PR #126632

Holistic Assessment

Motivation: Well-justified. A cross-platform API for starting daemon/background processes is a genuine gap — today users must resort to platform-specific flags or shell scripting. Issue #125838 carries the api-approved label and the PR directly addresses it.

Approach: Correct on all three platforms: DETACHED_PROCESS on Windows, POSIX_SPAWN_SETSID on macOS, and setsid() after fork() before exec() on other Unix — all established patterns. Defaulting unset stdio handles to the null device prevents accidental handle inheritance into long-lived detached processes.

Summary: ✅ LGTM. All three models independently raised two concerns that, after careful verification, are false positives:

---

Detailed Findings

✅ Handle disposal in Process.cs — Correctly handled (false positive resolved)

All three models flagged that the nullDeviceHandle in Process.cs might be disposed multiple times (once per childInputHandle, childOutputHandle, childErrorHandle in the finally block) if all three point to the same SafeFileHandle instance. This is safe: SafeHandle.Dispose() uses Interlocked.CompareExchange to set STATE_CLOSED atomically — subsequent calls are no-ops. The handle is properly disposed exactly once (OS-level), regardless of how many managed variables reference it.

✅ POSIX_SPAWN_SETSID availability — No issue

One model raised a concern about POSIX_SPAWN_SETSID not being available on older macOS. The runtime's CMAKE_OSX_DEPLOYMENT_TARGET is 14.0 (Sonoma). POSIX_SPAWN_SETSID was introduced in macOS 10.15, so this is fine.

✅ API Approval — Shape matches api-approved issue #125838

The ref assembly declares public bool StartDetached { get { throw null; } set { } } in alphabetical order (after StandardOutputHandle, before UseCredentialsForNetworkingOnly). This matches the approved shape exactly.

✅ Null device handle lifetime in SafeProcessHandle.cs

The using SafeFileHandle? nullDeviceHandle is disposed at end of scope, after StartCore returns. StartCore passes the handle to native code which immediately dup2s it — the managed-side dispose after return is safe.

✅ setsid() error handling

pal_process.c correctly propagates setsid() failures via ExitChild(waitForChildToExecPipe[WRITE_END_OF_PIPE], errno), so the managed caller receives the error.

✅ Test quality

• StartDetached_GrandchildSurvivesSignalingParent uses deterministic polling (20 ms increments up to WaitInMS) and guards with IsNotNanoServerAndRemoteExecutorSupported.
• StartDetached_StartsAndExitsSuccessfully provides a lightweight smoke test.
• StartDetached_ThrowsOnUseShellExecute covers the validation path.
• Windows SendSignal correctly discards entireProcessGroup with a documented explanation.

💡 Consider a Simpler Test for Basic StartDetached Functionality

The existing test (StartDetached_GrandchildSurvivesSignalingParent) covers the complex grandchild-survival scenario. A si...

The StartDetached_StartsAndExitsSuccessfully smoke test was added in commit 820aef5 and is already present in the current PR.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

[github-actions]

🤖 Copilot Code Review — PR #126632

Note

This review was generated by GitHub Copilot (Claude Opus 4.6). Full source files, git history, API approval issue, and existing review comments were analyzed.

Holistic Assessment

Motivation: Adding ProcessStartInfo.StartDetached addresses a well-known, legitimate need — starting daemon-like processes that are detached from the parent's session/console and survive parent termination. The approved API issue (#125838) demonstrates clear real-world use cases. Justified. ✅

Approach: The implementation correctly uses platform-appropriate mechanisms (DETACHED_PROCESS on Windows, POSIX_SPAWN_SETSID on macOS, setsid() on Linux). The single boolean property is minimal, consistent with existing ProcessStartInfo patterns (CreateNoWindow, CreateNewProcessGroup), and the defaulting of stdio to the null device is the right behavior for detached processes.

Summary: ⚠️Needs Human Review. The implementation is sound — the API matches the approved shape from issue #125838, native code is correct, and tests cover the critical scenarios. I'm flagging for human review because of a handle ownership pattern divergence between Process.Start() and SafeProcessHandle.Start() that, while functionally correct due to SafeHandle.Dispose() idempotency, warrants a human judgment call on whether to align the patterns.

---

Detailed Findings

✅ API Approval — Verified against issue #125838

The api-approved label was applied by @bartonjs on 2025-03-24. The approved API shape includes:

publicsealedpartialclassProcessStartInfo{publicboolStartDetached{get;set;}}

The implementation matches exactly — no extra public surface, correct property signature, correct placement in alphabetical order in the ref assembly (after StandardOutputHandle, before UseCredentialsForNetworkingOnly). ✅

---

✅ Native Implementation — Correct platform mechanisms

• macOS (posix_spawn path): Uses POSIX_SPAWN_SETSID flag (inside #if defined(TARGET_OSX) block). This is an Apple-specific extension available since macOS 10.14; .NET requires macOS 10.15+, so always available. ✅
• Linux (fork path): Calls setsid() after fork, before exec (pal_process.c). The freshly forked child always has PID ≠ PGID, so setsid() will succeed. Placement after dup2 but before credential setup and exec is correct. ✅
• WASI stub: Updated with (void)startDetached to suppress unused parameter warning. ✅

---

✅ Windows Implementation — Correct flag usage

DETACHED_PROCESS = 0x00000008 is correctly OR-ed into creation flags. The decision to allow DETACHED_PROCESS + CREATE_NO_WINDOW together was explicitly reviewed and approved by @jkotas (commit 389dcf7 removed the CreateNoWindow guard, noting the two flags are compatible per Windows API semantics — DETACHED_PROCESS takes precedence). ✅

---

✅ Validation Logic — Correct and complete

• StartDetached + UseShellExecute correctly throws InvalidOperationException ✅
• Error message stored in .resx following conventions ✅
• The StartDetached parameter threads correctly through the entire call chain: ProcessStartInfo → SafeProcessHandle.ForkAndExecProcess → P/Invoke → pal_process.c ✅

---

⚠️ Handle Ownership — Inconsistent pattern between Process.Start() and SafeProcessHandle.Start()

SafeProcessHandle.Start() uses the clean using pattern:

usingSafeFileHandle?nullDeviceHandle=startInfo.StartDetached&&(childInputHandleisnull||childOutputHandleisnull||childErrorHandleisnull)?File.OpenNullHandle():null;

The handle is owned by a single using variable and disposed exactly once.

Process.Start() assigns the same File.OpenNullHandle() result to up to three variables (childInputHandle, childOutputHandle, childErrorHandle), each then independently disposed in the finally block, resulting in up to three Dispose() calls on the same SafeFileHandle.

This is functionally safe — SafeHandle.Dispose() is idempotent by design; subsequent calls are no-ops. The finally block comment even acknowledges non-owning handles. I also see that @adamsitnik specifically requested this pattern in review: "call File.OpenNullHandle() at max once, ensure it gets disposed after process is started."

However, the shared-handle-with-multiple-dispose pattern in Process.Start() is inconsistent with the cleaner SafeProcessHandle.Start() approach. A human reviewer should confirm whether this divergence is intentional or whether a dedicated nullDeviceHandle local with using (matching the other call site) would be preferable.

Not blocking — code is correct as-is.

---

✅ Test Quality — Well-designed coverage

Three tests cover the critical scenarios:

1. StartDetached_ThrowsOnUseShellExecute — Validates the incompatibility check via SafeProcessHandle.Start(). ✅
2. StartDetached_StartsAndExitsSuccessfully — Smoke test verifying a detached process runs and exits normally. ✅
3. StartDetached_GrandchildSurvivesSignalingParent — [Theory] with true/false testing that a detached grandchild survives process-group kill while a non-detached one is killed. Good use of RemoteExecutor, proper cleanup via finally { grandchild.Kill(); }, and appropriate conditional attributes. ✅

The 300ms timeout for the "alive" assertion is reasonable since the grandchild does Thread.Sleep(Timeout.Infinite) — it cannot exit on its own, and setsid() ensures the SIGKILL to the parent's process group doesn't reach it. ✅

---

💡 XML Documentation — Minor clarity opportunity

The XML docs on StartDetached reference StandardInputHandle, StandardOutputHandle, StandardErrorHandle as properties that override null device defaulting. When using Process.Start() (the more common API path), RedirectStandardInput/RedirectStandardOutput/RedirectStandardError also override the null device behavior. The docs already mention these in the first <para>, but the "unless explicitly configured" phrasing in the second <para> could be slightly more inclusive.

Minor — not blocking.

Generated by Code Review for issue #126632 · ◷

[adamsitnik]

/ba-g unrelated

[Copilot]

Description

Adds ProcessStartInfo.StartDetached — a cross-platform way to start a process detached from the parent's session and console, such that it outlives the parent.

API surface

// ref/System.Diagnostics.Process.cspublicsealedclassProcessStartInfo{publicboolStartDetached{get;set;}}

Behavior by platform

Platform	Mechanism
Windows	DETACHED_PROCESS creation flag
macOS	POSIX_SPAWN_SETSID via posix_spawn
Other Unix	setsid() in child after fork, before exec

Key semantics

• ThrowsInvalidOperationException when combined with UseShellExecute = true
• Standard handles (stdin/stdout/stderr) not explicitly set are automatically redirected to the null device (NUL on Windows, /dev/null on Unix) — a single File.OpenNullHandle() is opened at most once and disposed after the process starts; override with RedirectStandardInput/RedirectStandardOutput/RedirectStandardError or StandardInputHandle/StandardOutputHandle/StandardErrorHandle

Example

varpsi=newProcessStartInfo("myserver"){StartDetached=true};// stdin/stdout/stderr automatically go to NUL/dev/nullProcess.Start(psi);

Files changed

• ProcessStartInfo.cs / ref — new property with XML docs linking to the Windows SDK DETACHED_PROCESS documentation; remarks note that UseShellExecute = true will throw; StartDetached placed in alphabetical order in the ref assembly (after StandardOutputHandle, before UseCredentialsForNetworkingOnly)
• Strings.resx — resource key StartDetachedNotCompatible; message states the property cannot be used with UseShellExecute set to true
• Process.cs — opens a single null device handle outside the lock region, reuses it for all unset stdio handles; existing finally block handles disposal
• SafeProcessHandle.cs — if/else if/else blocks; single nullDeviceHandle opened at most once; disposed in a finally block after StartCore
• SafeProcessHandle.Windows.cs — DETACHED_PROCESS flag; constant added to Interop.ProcessOptions.cs
• Interop.ForkAndExecProcess.cs / SafeProcessHandle.Unix.cs — thread startDetached through managed interop chain; fixed indentation of ForkAndExecProcess(...) call inside fixed block
• pal_process.c / pal_process.h / pal_process_wasi.c — native implementation: POSIX_SPAWN_SETSID (macOS), if (startDetached && setsid() == -1) check (other Unix)
• ProcessTests.cs — cross-platform StartDetached_GrandchildSurvivesSignalingParent test guarded by IsNotNanoServerAndRemoteExecutorSupported (PlatformDetection.IsNotWindowsNanoServer && RemoteExecutor.IsSupported); child started with StartDetached=true (own process group on Unix) or CreateNewProcessGroup=true (Windows); kills child's process group with SIGKILL on Unix and SIGQUIT (mapped to CTRL_BREAK_EVENT) on Windows via SendSignal(..., entireProcessGroup: true); grandchild-alive check uses deterministic polling (loop with 20 ms sleep up to WaitInMS) instead of a fixed Thread.Sleep; validates that detached grandchild survives while non-detached grandchild is killed; StartDetached_StartsAndExitsSuccessfully smoke test starts a process with StartDetached=true via RemoteExecutor and verifies it completes successfully
• ProcessTests.Unix.cs — SendSignal(..., entireProcessGroup: true) passes the given signal through Interop.Sys.GetPlatformSignalNumber to kill(-pgid, signalNumber), keeping the helper generic (not hardcoded to any particular signal); call site passes PosixSignal.SIGKILL on Unix for reliable process-group termination
• ProcessTests.Windows.cs — SendSignal documents that entireProcessGroup has no distinct effect on Windows (via _ = entireProcessGroup) since GenerateConsoleCtrlEvent is inherently process-group scoped
• ProcessHandlesTests.cs — [Fact] test for UseShellExecute validation

Changes

• Adds a new cross-platform ProcessStartInfo.StartDetached property
• Redirects standard I/O to null device (single handle, properly disposed) when detached and no explicit handle is provided
• Platform-specific implementation: DETACHED_PROCESS (Windows), POSIX_SPAWN_SETSID (macOS), setsid() (Linux/other Unix)
• Validation: throws when combined with UseShellExecute = true only (CreateNoWindow is not rejected — DETACHED_PROCESS and CREATE_NO_WINDOW are compatible per Windows API semantics)
• XML docs link to the Windows SDK DETACHED_PROCESS documentation; only UseShellExecute listed as incompatible
• Tests covering validation (UseShellExecute), cross-platform process-group isolation (detached grandchild survives, non-detached grandchild is killed), and basic smoke test (StartDetached_StartsAndExitsSuccessfully); all skipped on Windows Nano Server and where RemoteExecutor is unsupported
• SendSignal Unix helper kept generic (signal passed via GetPlatformSignalNumber); call site uses SIGKILL on Unix for reliable group termination
• Deterministic polling replaces fixed Thread.Sleep(200) in grandchild-survival assertion
• Windows SendSignal explicitly discards entireProcessGroup with a clarifying comment that GenerateConsoleCtrlEvent is inherently group-scoped
• StartDetached placed in alphabetical order in the ref assembly (after StandardOutputHandle, before UseCredentialsForNetworkingOnly)

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

@copilot address my feedback

[Copilot]

@copilot address my feedback

All feedback addressed in a6e4206. Summary of changes:

• SafeProcessHandle.cs: if/else if/else blocks, single null device handle, disposed in finally after StartCore
• Process.cs: single null device handle opened once after the lock, reused for all stdio
• ProcessStartInfo.cs docs: split into 3 paragraphs with StandardInputHandle/OutputHandle/ErrorHandle refs
• ProcessStartInfo.cs condition: added && string.IsNullOrEmpty(UserName) to the StartDetached branch
• pal_process.c: reduced setsid check nesting
• StartDetachedTests.cs: [Fact] for validation test; removed mobile platform skips; SIGHUP test skips only Windows; added using System.Runtime.InteropServices for PosixSignal

[Copilot]

Pull request overview

This PR introduces a new ProcessStartInfo.StartDetached API to start processes detached from the parent’s session/console across platforms, with additional safety defaults to avoid leaking standard handles and inheritable resources into long-lived detached processes.

Changes:

• Adds ProcessStartInfo.StartDetached to the public surface area (ref + implementation) with validation against UseShellExecute = true.
• Implements platform-specific detaching: DETACHED_PROCESS (Windows), POSIX_SPAWN_SETSID (macOS), and setsid() (other Unix), threading the flag through managed/native interop.
• When detached, defaults stdio to the null device (unless explicitly configured) and restricts handle inheritance by default.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/native/libs/System.Native/pal_process.h	Extends native fork/exec API with startDetached parameter.
src/native/libs/System.Native/pal_process.c	Implements session detachment (POSIX_SPAWN_SETSID / setsid()) based on startDetached.
src/native/libs/System.Native/pal_process_wasi.c	Updates WASI stub signature for startDetached.
src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs	Adds managed interop parameter and forwards startDetached to native.
src/libraries/Common/src/Interop/Windows/Advapi32/Interop.ProcessOptions.cs	Introduces DETACHED_PROCESS constant for process creation flags.
src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs	Adds ProcessStartInfo.StartDetached to the reference assembly API surface.
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs	Adds property + validation + default empty inherited-handles list for detached starts.
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs	Uses a single null-device handle for unset stdio when detached; opens it outside the lock.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs	Applies detached stdio-to-null behavior for SafeProcessHandle.Start.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs	Threads StartDetached through to Interop.Sys.ForkAndExecProcess.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs	Adds DETACHED_PROCESS to creation flags when StartDetached is set.
src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx	Adds error message for StartDetached + UseShellExecute incompatibility.
src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj	Includes new StartDetachedTests.cs in the test project.
src/libraries/System.Diagnostics.Process/tests/StartDetachedTests.cs	Adds validation and behavioral tests for detached starts (including a Unix SIGHUP scenario).

[adamsitnik]

@copilot address my feedback

[Copilot]

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

Note

This review was generated by Copilot AI using three parallel models (Claude Sonnet 4.5, Gemini 2.5 Pro, GPT-4.1) and synthesized into a single report.

🤖 Copilot Code Review — PR #126632

Holistic Assessment

Motivation: Well-justified. A cross-platform API for starting daemon/background processes is a genuine gap — today users must resort to platform-specific flags or shell scripting. Issue #125838 carries the api-approved label and the PR directly addresses it.

Approach: Correct on all three platforms: DETACHED_PROCESS on Windows, POSIX_SPAWN_SETSID on macOS, and setsid() after fork() before exec() on other Unix — all established patterns. Defaulting unset stdio handles to the null device prevents accidental handle inheritance into long-lived detached processes.

Summary: ✅ LGTM. All three models independently raised two concerns that, after careful verification, are false positives:

---

Detailed Findings

✅ Handle disposal in Process.cs — Correctly handled (false positive resolved)

All three models flagged that the nullDeviceHandle in Process.cs might be disposed multiple times (once per childInputHandle, childOutputHandle, childErrorHandle in the finally block) if all three point to the same SafeFileHandle instance. This is safe: SafeHandle.Dispose() uses Interlocked.CompareExchange to set STATE_CLOSED atomically — subsequent calls are no-ops. The handle is properly disposed exactly once (OS-level), regardless of how many managed variables reference it.

✅ POSIX_SPAWN_SETSID availability — No issue

One model raised a concern about POSIX_SPAWN_SETSID not being available on older macOS. The runtime's CMAKE_OSX_DEPLOYMENT_TARGET is 14.0 (Sonoma). POSIX_SPAWN_SETSID was introduced in macOS 10.15, so this is fine.

✅ API Approval — Shape matches api-approved issue #125838

The ref assembly declares public bool StartDetached { get { throw null; } set { } } in alphabetical order (after StandardOutputHandle, before UseCredentialsForNetworkingOnly). This matches the approved shape exactly.

✅ Null device handle lifetime in SafeProcessHandle.cs

The using SafeFileHandle? nullDeviceHandle is disposed at end of scope, after StartCore returns. StartCore passes the handle to native code which immediately dup2s it — the managed-side dispose after return is safe.

✅ setsid() error handling

pal_process.c correctly propagates setsid() failures via ExitChild(waitForChildToExecPipe[WRITE_END_OF_PIPE], errno), so the managed caller receives the error.

✅ Test quality

• StartDetached_GrandchildSurvivesSignalingParent uses deterministic polling (20 ms increments up to WaitInMS) and guards with IsNotNanoServerAndRemoteExecutorSupported.
• StartDetached_StartsAndExitsSuccessfully provides a lightweight smoke test.
• StartDetached_ThrowsOnUseShellExecute covers the validation path.
• Windows SendSignal correctly discards entireProcessGroup with a documented explanation.

💡 Consider a Simpler Test for Basic StartDetached Functionality

The existing test (StartDetached_GrandchildSurvivesSignalingParent) covers the complex grandchild-survival scenario. A si...

The StartDetached_StartsAndExitsSuccessfully smoke test was added in commit 820aef5 and is already present in the current PR.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

[github-actions]

🤖 Copilot Code Review — PR #126632

Note

This review was generated by GitHub Copilot (Claude Opus 4.6). Full source files, git history, API approval issue, and existing review comments were analyzed.

Holistic Assessment

Motivation: Adding ProcessStartInfo.StartDetached addresses a well-known, legitimate need — starting daemon-like processes that are detached from the parent's session/console and survive parent termination. The approved API issue (#125838) demonstrates clear real-world use cases. Justified. ✅

Approach: The implementation correctly uses platform-appropriate mechanisms (DETACHED_PROCESS on Windows, POSIX_SPAWN_SETSID on macOS, setsid() on Linux). The single boolean property is minimal, consistent with existing ProcessStartInfo patterns (CreateNoWindow, CreateNewProcessGroup), and the defaulting of stdio to the null device is the right behavior for detached processes.

Summary: ⚠️Needs Human Review. The implementation is sound — the API matches the approved shape from issue #125838, native code is correct, and tests cover the critical scenarios. I'm flagging for human review because of a handle ownership pattern divergence between Process.Start() and SafeProcessHandle.Start() that, while functionally correct due to SafeHandle.Dispose() idempotency, warrants a human judgment call on whether to align the patterns.

---

Detailed Findings

✅ API Approval — Verified against issue #125838

The api-approved label was applied by @bartonjs on 2025-03-24. The approved API shape includes:

publicsealedpartialclassProcessStartInfo{publicboolStartDetached{get;set;}}

The implementation matches exactly — no extra public surface, correct property signature, correct placement in alphabetical order in the ref assembly (after StandardOutputHandle, before UseCredentialsForNetworkingOnly). ✅

---

✅ Native Implementation — Correct platform mechanisms

• macOS (posix_spawn path): Uses POSIX_SPAWN_SETSID flag (inside #if defined(TARGET_OSX) block). This is an Apple-specific extension available since macOS 10.14; .NET requires macOS 10.15+, so always available. ✅
• Linux (fork path): Calls setsid() after fork, before exec (pal_process.c). The freshly forked child always has PID ≠ PGID, so setsid() will succeed. Placement after dup2 but before credential setup and exec is correct. ✅
• WASI stub: Updated with (void)startDetached to suppress unused parameter warning. ✅

---

✅ Windows Implementation — Correct flag usage

DETACHED_PROCESS = 0x00000008 is correctly OR-ed into creation flags. The decision to allow DETACHED_PROCESS + CREATE_NO_WINDOW together was explicitly reviewed and approved by @jkotas (commit 389dcf7 removed the CreateNoWindow guard, noting the two flags are compatible per Windows API semantics — DETACHED_PROCESS takes precedence). ✅

---

✅ Validation Logic — Correct and complete

• StartDetached + UseShellExecute correctly throws InvalidOperationException ✅
• Error message stored in .resx following conventions ✅
• The StartDetached parameter threads correctly through the entire call chain: ProcessStartInfo → SafeProcessHandle.ForkAndExecProcess → P/Invoke → pal_process.c ✅

---

⚠️ Handle Ownership — Inconsistent pattern between Process.Start() and SafeProcessHandle.Start()

SafeProcessHandle.Start() uses the clean using pattern:

usingSafeFileHandle?nullDeviceHandle=startInfo.StartDetached&&(childInputHandleisnull||childOutputHandleisnull||childErrorHandleisnull)?File.OpenNullHandle():null;

The handle is owned by a single using variable and disposed exactly once.

Process.Start() assigns the same File.OpenNullHandle() result to up to three variables (childInputHandle, childOutputHandle, childErrorHandle), each then independently disposed in the finally block, resulting in up to three Dispose() calls on the same SafeFileHandle.

This is functionally safe — SafeHandle.Dispose() is idempotent by design; subsequent calls are no-ops. The finally block comment even acknowledges non-owning handles. I also see that @adamsitnik specifically requested this pattern in review: "call File.OpenNullHandle() at max once, ensure it gets disposed after process is started."

However, the shared-handle-with-multiple-dispose pattern in Process.Start() is inconsistent with the cleaner SafeProcessHandle.Start() approach. A human reviewer should confirm whether this divergence is intentional or whether a dedicated nullDeviceHandle local with using (matching the other call site) would be preferable.

Not blocking — code is correct as-is.

---

✅ Test Quality — Well-designed coverage

Three tests cover the critical scenarios:

1. StartDetached_ThrowsOnUseShellExecute — Validates the incompatibility check via SafeProcessHandle.Start(). ✅
2. StartDetached_StartsAndExitsSuccessfully — Smoke test verifying a detached process runs and exits normally. ✅
3. StartDetached_GrandchildSurvivesSignalingParent — [Theory] with true/false testing that a detached grandchild survives process-group kill while a non-detached one is killed. Good use of RemoteExecutor, proper cleanup via finally { grandchild.Kill(); }, and appropriate conditional attributes. ✅

The 300ms timeout for the "alive" assertion is reasonable since the grandchild does Thread.Sleep(Timeout.Infinite) — it cannot exit on its own, and setsid() ensures the SIGKILL to the parent's process group doesn't reach it. ✅

---

💡 XML Documentation — Minor clarity opportunity

The XML docs on StartDetached reference StandardInputHandle, StandardOutputHandle, StandardErrorHandle as properties that override null device defaulting. When using Process.Start() (the more common API path), RedirectStandardInput/RedirectStandardOutput/RedirectStandardError also override the null device behavior. The docs already mention these in the first <para>, but the "unless explicitly configured" phrasing in the second <para> could be slightly more inclusive.

Minor — not blocking.

Generated by Code Review for issue #126632 · ◷

[adamsitnik]

/ba-g unrelated

[Copilot]

Description

Adds ProcessStartInfo.StartDetached — a cross-platform way to start a process detached from the parent's session and console, such that it outlives the parent.

API surface

// ref/System.Diagnostics.Process.cspublicsealedclassProcessStartInfo{publicboolStartDetached{get;set;}}

Behavior by platform

Platform	Mechanism
Windows	DETACHED_PROCESS creation flag
macOS	POSIX_SPAWN_SETSID via posix_spawn
Other Unix	setsid() in child after fork, before exec

Key semantics

• ThrowsInvalidOperationException when combined with UseShellExecute = true
• Standard handles (stdin/stdout/stderr) not explicitly set are automatically redirected to the null device (NUL on Windows, /dev/null on Unix) — a single File.OpenNullHandle() is opened at most once and disposed after the process starts; override with RedirectStandardInput/RedirectStandardOutput/RedirectStandardError or StandardInputHandle/StandardOutputHandle/StandardErrorHandle

Example

varpsi=newProcessStartInfo("myserver"){StartDetached=true};// stdin/stdout/stderr automatically go to NUL/dev/nullProcess.Start(psi);

Files changed

• ProcessStartInfo.cs / ref — new property with XML docs linking to the Windows SDK DETACHED_PROCESS documentation; remarks note that UseShellExecute = true will throw; StartDetached placed in alphabetical order in the ref assembly (after StandardOutputHandle, before UseCredentialsForNetworkingOnly)
• Strings.resx — resource key StartDetachedNotCompatible; message states the property cannot be used with UseShellExecute set to true
• Process.cs — opens a single null device handle outside the lock region, reuses it for all unset stdio handles; existing finally block handles disposal
• SafeProcessHandle.cs — if/else if/else blocks; single nullDeviceHandle opened at most once; disposed in a finally block after StartCore
• SafeProcessHandle.Windows.cs — DETACHED_PROCESS flag; constant added to Interop.ProcessOptions.cs
• Interop.ForkAndExecProcess.cs / SafeProcessHandle.Unix.cs — thread startDetached through managed interop chain; fixed indentation of ForkAndExecProcess(...) call inside fixed block
• pal_process.c / pal_process.h / pal_process_wasi.c — native implementation: POSIX_SPAWN_SETSID (macOS), if (startDetached && setsid() == -1) check (other Unix)
• ProcessTests.cs — cross-platform StartDetached_GrandchildSurvivesSignalingParent test guarded by IsNotNanoServerAndRemoteExecutorSupported (PlatformDetection.IsNotWindowsNanoServer && RemoteExecutor.IsSupported); child started with StartDetached=true (own process group on Unix) or CreateNewProcessGroup=true (Windows); kills child's process group with SIGKILL on Unix and SIGQUIT (mapped to CTRL_BREAK_EVENT) on Windows via SendSignal(..., entireProcessGroup: true); grandchild-alive check uses deterministic polling (loop with 20 ms sleep up to WaitInMS) instead of a fixed Thread.Sleep; validates that detached grandchild survives while non-detached grandchild is killed; StartDetached_StartsAndExitsSuccessfully smoke test starts a process with StartDetached=true via RemoteExecutor and verifies it completes successfully
• ProcessTests.Unix.cs — SendSignal(..., entireProcessGroup: true) passes the given signal through Interop.Sys.GetPlatformSignalNumber to kill(-pgid, signalNumber), keeping the helper generic (not hardcoded to any particular signal); call site passes PosixSignal.SIGKILL on Unix for reliable process-group termination
• ProcessTests.Windows.cs — SendSignal documents that entireProcessGroup has no distinct effect on Windows (via _ = entireProcessGroup) since GenerateConsoleCtrlEvent is inherently process-group scoped
• ProcessHandlesTests.cs — [Fact] test for UseShellExecute validation

Changes

• Adds a new cross-platform ProcessStartInfo.StartDetached property
• Redirects standard I/O to null device (single handle, properly disposed) when detached and no explicit handle is provided
• Platform-specific implementation: DETACHED_PROCESS (Windows), POSIX_SPAWN_SETSID (macOS), setsid() (Linux/other Unix)
• Validation: throws when combined with UseShellExecute = true only (CreateNoWindow is not rejected — DETACHED_PROCESS and CREATE_NO_WINDOW are compatible per Windows API semantics)
• XML docs link to the Windows SDK DETACHED_PROCESS documentation; only UseShellExecute listed as incompatible
• Tests covering validation (UseShellExecute), cross-platform process-group isolation (detached grandchild survives, non-detached grandchild is killed), and basic smoke test (StartDetached_StartsAndExitsSuccessfully); all skipped on Windows Nano Server and where RemoteExecutor is unsupported
• SendSignal Unix helper kept generic (signal passed via GetPlatformSignalNumber); call site uses SIGKILL on Unix for reliable group termination
• Deterministic polling replaces fixed Thread.Sleep(200) in grandchild-survival assertion
• Windows SendSignal explicitly discards entireProcessGroup with a clarifying comment that GenerateConsoleCtrlEvent is inherently group-scoped
• StartDetached placed in alphabetical order in the ref assembly (after StandardOutputHandle, before UseCredentialsForNetworkingOnly)

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

@copilot address my feedback

[Copilot]

@copilot address my feedback

All feedback addressed in a6e4206. Summary of changes:

• SafeProcessHandle.cs: if/else if/else blocks, single null device handle, disposed in finally after StartCore
• Process.cs: single null device handle opened once after the lock, reused for all stdio
• ProcessStartInfo.cs docs: split into 3 paragraphs with StandardInputHandle/OutputHandle/ErrorHandle refs
• ProcessStartInfo.cs condition: added && string.IsNullOrEmpty(UserName) to the StartDetached branch
• pal_process.c: reduced setsid check nesting
• StartDetachedTests.cs: [Fact] for validation test; removed mobile platform skips; SIGHUP test skips only Windows; added using System.Runtime.InteropServices for PosixSignal

[Copilot]

Pull request overview

This PR introduces a new ProcessStartInfo.StartDetached API to start processes detached from the parent’s session/console across platforms, with additional safety defaults to avoid leaking standard handles and inheritable resources into long-lived detached processes.

Changes:

• Adds ProcessStartInfo.StartDetached to the public surface area (ref + implementation) with validation against UseShellExecute = true.
• Implements platform-specific detaching: DETACHED_PROCESS (Windows), POSIX_SPAWN_SETSID (macOS), and setsid() (other Unix), threading the flag through managed/native interop.
• When detached, defaults stdio to the null device (unless explicitly configured) and restricts handle inheritance by default.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/native/libs/System.Native/pal_process.h	Extends native fork/exec API with startDetached parameter.
src/native/libs/System.Native/pal_process.c	Implements session detachment (POSIX_SPAWN_SETSID / setsid()) based on startDetached.
src/native/libs/System.Native/pal_process_wasi.c	Updates WASI stub signature for startDetached.
src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs	Adds managed interop parameter and forwards startDetached to native.
src/libraries/Common/src/Interop/Windows/Advapi32/Interop.ProcessOptions.cs	Introduces DETACHED_PROCESS constant for process creation flags.
src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs	Adds ProcessStartInfo.StartDetached to the reference assembly API surface.
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs	Adds property + validation + default empty inherited-handles list for detached starts.
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs	Uses a single null-device handle for unset stdio when detached; opens it outside the lock.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs	Applies detached stdio-to-null behavior for SafeProcessHandle.Start.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs	Threads StartDetached through to Interop.Sys.ForkAndExecProcess.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs	Adds DETACHED_PROCESS to creation flags when StartDetached is set.
src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx	Adds error message for StartDetached + UseShellExecute incompatibility.
src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj	Includes new StartDetachedTests.cs in the test project.
src/libraries/System.Diagnostics.Process/tests/StartDetachedTests.cs	Adds validation and behavioral tests for detached starts (including a Unix SIGHUP scenario).

[adamsitnik]

@copilot address my feedback

[Copilot]

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

Note

This review was generated by Copilot AI using three parallel models (Claude Sonnet 4.5, Gemini 2.5 Pro, GPT-4.1) and synthesized into a single report.

🤖 Copilot Code Review — PR #126632

Holistic Assessment

Motivation: Well-justified. A cross-platform API for starting daemon/background processes is a genuine gap — today users must resort to platform-specific flags or shell scripting. Issue #125838 carries the api-approved label and the PR directly addresses it.

Approach: Correct on all three platforms: DETACHED_PROCESS on Windows, POSIX_SPAWN_SETSID on macOS, and setsid() after fork() before exec() on other Unix — all established patterns. Defaulting unset stdio handles to the null device prevents accidental handle inheritance into long-lived detached processes.

Summary: ✅ LGTM. All three models independently raised two concerns that, after careful verification, are false positives:

---

Detailed Findings

✅ Handle disposal in Process.cs — Correctly handled (false positive resolved)

All three models flagged that the nullDeviceHandle in Process.cs might be disposed multiple times (once per childInputHandle, childOutputHandle, childErrorHandle in the finally block) if all three point to the same SafeFileHandle instance. This is safe: SafeHandle.Dispose() uses Interlocked.CompareExchange to set STATE_CLOSED atomically — subsequent calls are no-ops. The handle is properly disposed exactly once (OS-level), regardless of how many managed variables reference it.

✅ POSIX_SPAWN_SETSID availability — No issue

One model raised a concern about POSIX_SPAWN_SETSID not being available on older macOS. The runtime's CMAKE_OSX_DEPLOYMENT_TARGET is 14.0 (Sonoma). POSIX_SPAWN_SETSID was introduced in macOS 10.15, so this is fine.

✅ API Approval — Shape matches api-approved issue #125838

The ref assembly declares public bool StartDetached { get { throw null; } set { } } in alphabetical order (after StandardOutputHandle, before UseCredentialsForNetworkingOnly). This matches the approved shape exactly.

✅ Null device handle lifetime in SafeProcessHandle.cs

The using SafeFileHandle? nullDeviceHandle is disposed at end of scope, after StartCore returns. StartCore passes the handle to native code which immediately dup2s it — the managed-side dispose after return is safe.

✅ setsid() error handling

pal_process.c correctly propagates setsid() failures via ExitChild(waitForChildToExecPipe[WRITE_END_OF_PIPE], errno), so the managed caller receives the error.

✅ Test quality

• StartDetached_GrandchildSurvivesSignalingParent uses deterministic polling (20 ms increments up to WaitInMS) and guards with IsNotNanoServerAndRemoteExecutorSupported.
• StartDetached_StartsAndExitsSuccessfully provides a lightweight smoke test.
• StartDetached_ThrowsOnUseShellExecute covers the validation path.
• Windows SendSignal correctly discards entireProcessGroup with a documented explanation.

💡 Consider a Simpler Test for Basic StartDetached Functionality

The existing test (StartDetached_GrandchildSurvivesSignalingParent) covers the complex grandchild-survival scenario. A si...

The StartDetached_StartsAndExitsSuccessfully smoke test was added in commit 820aef5 and is already present in the current PR.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

[github-actions]

🤖 Copilot Code Review — PR #126632

Note

This review was generated by GitHub Copilot (Claude Opus 4.6). Full source files, git history, API approval issue, and existing review comments were analyzed.

Holistic Assessment

Motivation: Adding ProcessStartInfo.StartDetached addresses a well-known, legitimate need — starting daemon-like processes that are detached from the parent's session/console and survive parent termination. The approved API issue (#125838) demonstrates clear real-world use cases. Justified. ✅

Approach: The implementation correctly uses platform-appropriate mechanisms (DETACHED_PROCESS on Windows, POSIX_SPAWN_SETSID on macOS, setsid() on Linux). The single boolean property is minimal, consistent with existing ProcessStartInfo patterns (CreateNoWindow, CreateNewProcessGroup), and the defaulting of stdio to the null device is the right behavior for detached processes.

Summary: ⚠️Needs Human Review. The implementation is sound — the API matches the approved shape from issue #125838, native code is correct, and tests cover the critical scenarios. I'm flagging for human review because of a handle ownership pattern divergence between Process.Start() and SafeProcessHandle.Start() that, while functionally correct due to SafeHandle.Dispose() idempotency, warrants a human judgment call on whether to align the patterns.

---

Detailed Findings

✅ API Approval — Verified against issue #125838

The api-approved label was applied by @bartonjs on 2025-03-24. The approved API shape includes:

publicsealedpartialclassProcessStartInfo{publicboolStartDetached{get;set;}}

The implementation matches exactly — no extra public surface, correct property signature, correct placement in alphabetical order in the ref assembly (after StandardOutputHandle, before UseCredentialsForNetworkingOnly). ✅

---

✅ Native Implementation — Correct platform mechanisms

• macOS (posix_spawn path): Uses POSIX_SPAWN_SETSID flag (inside #if defined(TARGET_OSX) block). This is an Apple-specific extension available since macOS 10.14; .NET requires macOS 10.15+, so always available. ✅
• Linux (fork path): Calls setsid() after fork, before exec (pal_process.c). The freshly forked child always has PID ≠ PGID, so setsid() will succeed. Placement after dup2 but before credential setup and exec is correct. ✅
• WASI stub: Updated with (void)startDetached to suppress unused parameter warning. ✅

---

✅ Windows Implementation — Correct flag usage

DETACHED_PROCESS = 0x00000008 is correctly OR-ed into creation flags. The decision to allow DETACHED_PROCESS + CREATE_NO_WINDOW together was explicitly reviewed and approved by @jkotas (commit 389dcf7 removed the CreateNoWindow guard, noting the two flags are compatible per Windows API semantics — DETACHED_PROCESS takes precedence). ✅

---

✅ Validation Logic — Correct and complete

• StartDetached + UseShellExecute correctly throws InvalidOperationException ✅
• Error message stored in .resx following conventions ✅
• The StartDetached parameter threads correctly through the entire call chain: ProcessStartInfo → SafeProcessHandle.ForkAndExecProcess → P/Invoke → pal_process.c ✅

---

⚠️ Handle Ownership — Inconsistent pattern between Process.Start() and SafeProcessHandle.Start()

SafeProcessHandle.Start() uses the clean using pattern:

usingSafeFileHandle?nullDeviceHandle=startInfo.StartDetached&&(childInputHandleisnull||childOutputHandleisnull||childErrorHandleisnull)?File.OpenNullHandle():null;

The handle is owned by a single using variable and disposed exactly once.

Process.Start() assigns the same File.OpenNullHandle() result to up to three variables (childInputHandle, childOutputHandle, childErrorHandle), each then independently disposed in the finally block, resulting in up to three Dispose() calls on the same SafeFileHandle.

This is functionally safe — SafeHandle.Dispose() is idempotent by design; subsequent calls are no-ops. The finally block comment even acknowledges non-owning handles. I also see that @adamsitnik specifically requested this pattern in review: "call File.OpenNullHandle() at max once, ensure it gets disposed after process is started."

However, the shared-handle-with-multiple-dispose pattern in Process.Start() is inconsistent with the cleaner SafeProcessHandle.Start() approach. A human reviewer should confirm whether this divergence is intentional or whether a dedicated nullDeviceHandle local with using (matching the other call site) would be preferable.

Not blocking — code is correct as-is.

---

✅ Test Quality — Well-designed coverage

Three tests cover the critical scenarios:

1. StartDetached_ThrowsOnUseShellExecute — Validates the incompatibility check via SafeProcessHandle.Start(). ✅
2. StartDetached_StartsAndExitsSuccessfully — Smoke test verifying a detached process runs and exits normally. ✅
3. StartDetached_GrandchildSurvivesSignalingParent — [Theory] with true/false testing that a detached grandchild survives process-group kill while a non-detached one is killed. Good use of RemoteExecutor, proper cleanup via finally { grandchild.Kill(); }, and appropriate conditional attributes. ✅

The 300ms timeout for the "alive" assertion is reasonable since the grandchild does Thread.Sleep(Timeout.Infinite) — it cannot exit on its own, and setsid() ensures the SIGKILL to the parent's process group doesn't reach it. ✅

---

💡 XML Documentation — Minor clarity opportunity

The XML docs on StartDetached reference StandardInputHandle, StandardOutputHandle, StandardErrorHandle as properties that override null device defaulting. When using Process.Start() (the more common API path), RedirectStandardInput/RedirectStandardOutput/RedirectStandardError also override the null device behavior. The docs already mention these in the first <para>, but the "unless explicitly configured" phrasing in the second <para> could be slightly more inclusive.

Minor — not blocking.

Generated by Code Review for issue #126632 · ◷

[adamsitnik]

/ba-g unrelated

[Copilot]

Description

Adds ProcessStartInfo.StartDetached — a cross-platform way to start a process detached from the parent's session and console, such that it outlives the parent.

API surface

// ref/System.Diagnostics.Process.cspublicsealedclassProcessStartInfo{publicboolStartDetached{get;set;}}

Behavior by platform

Platform	Mechanism
Windows	DETACHED_PROCESS creation flag
macOS	POSIX_SPAWN_SETSID via posix_spawn
Other Unix	setsid() in child after fork, before exec

Key semantics

• ThrowsInvalidOperationException when combined with UseShellExecute = true
• Standard handles (stdin/stdout/stderr) not explicitly set are automatically redirected to the null device (NUL on Windows, /dev/null on Unix) — a single File.OpenNullHandle() is opened at most once and disposed after the process starts; override with RedirectStandardInput/RedirectStandardOutput/RedirectStandardError or StandardInputHandle/StandardOutputHandle/StandardErrorHandle

Example

varpsi=newProcessStartInfo("myserver"){StartDetached=true};// stdin/stdout/stderr automatically go to NUL/dev/nullProcess.Start(psi);

Files changed

• ProcessStartInfo.cs / ref — new property with XML docs linking to the Windows SDK DETACHED_PROCESS documentation; remarks note that UseShellExecute = true will throw; StartDetached placed in alphabetical order in the ref assembly (after StandardOutputHandle, before UseCredentialsForNetworkingOnly)
• Strings.resx — resource key StartDetachedNotCompatible; message states the property cannot be used with UseShellExecute set to true
• Process.cs — opens a single null device handle outside the lock region, reuses it for all unset stdio handles; existing finally block handles disposal
• SafeProcessHandle.cs — if/else if/else blocks; single nullDeviceHandle opened at most once; disposed in a finally block after StartCore
• SafeProcessHandle.Windows.cs — DETACHED_PROCESS flag; constant added to Interop.ProcessOptions.cs
• Interop.ForkAndExecProcess.cs / SafeProcessHandle.Unix.cs — thread startDetached through managed interop chain; fixed indentation of ForkAndExecProcess(...) call inside fixed block
• pal_process.c / pal_process.h / pal_process_wasi.c — native implementation: POSIX_SPAWN_SETSID (macOS), if (startDetached && setsid() == -1) check (other Unix)
• ProcessTests.cs — cross-platform StartDetached_GrandchildSurvivesSignalingParent test guarded by IsNotNanoServerAndRemoteExecutorSupported (PlatformDetection.IsNotWindowsNanoServer && RemoteExecutor.IsSupported); child started with StartDetached=true (own process group on Unix) or CreateNewProcessGroup=true (Windows); kills child's process group with SIGKILL on Unix and SIGQUIT (mapped to CTRL_BREAK_EVENT) on Windows via SendSignal(..., entireProcessGroup: true); grandchild-alive check uses deterministic polling (loop with 20 ms sleep up to WaitInMS) instead of a fixed Thread.Sleep; validates that detached grandchild survives while non-detached grandchild is killed; StartDetached_StartsAndExitsSuccessfully smoke test starts a process with StartDetached=true via RemoteExecutor and verifies it completes successfully
• ProcessTests.Unix.cs — SendSignal(..., entireProcessGroup: true) passes the given signal through Interop.Sys.GetPlatformSignalNumber to kill(-pgid, signalNumber), keeping the helper generic (not hardcoded to any particular signal); call site passes PosixSignal.SIGKILL on Unix for reliable process-group termination
• ProcessTests.Windows.cs — SendSignal documents that entireProcessGroup has no distinct effect on Windows (via _ = entireProcessGroup) since GenerateConsoleCtrlEvent is inherently process-group scoped
• ProcessHandlesTests.cs — [Fact] test for UseShellExecute validation

Changes

• Adds a new cross-platform ProcessStartInfo.StartDetached property
• Redirects standard I/O to null device (single handle, properly disposed) when detached and no explicit handle is provided
• Platform-specific implementation: DETACHED_PROCESS (Windows), POSIX_SPAWN_SETSID (macOS), setsid() (Linux/other Unix)
• Validation: throws when combined with UseShellExecute = true only (CreateNoWindow is not rejected — DETACHED_PROCESS and CREATE_NO_WINDOW are compatible per Windows API semantics)
• XML docs link to the Windows SDK DETACHED_PROCESS documentation; only UseShellExecute listed as incompatible
• Tests covering validation (UseShellExecute), cross-platform process-group isolation (detached grandchild survives, non-detached grandchild is killed), and basic smoke test (StartDetached_StartsAndExitsSuccessfully); all skipped on Windows Nano Server and where RemoteExecutor is unsupported
• SendSignal Unix helper kept generic (signal passed via GetPlatformSignalNumber); call site uses SIGKILL on Unix for reliable group termination
• Deterministic polling replaces fixed Thread.Sleep(200) in grandchild-survival assertion
• Windows SendSignal explicitly discards entireProcessGroup with a clarifying comment that GenerateConsoleCtrlEvent is inherently group-scoped
• StartDetached placed in alphabetical order in the ref assembly (after StandardOutputHandle, before UseCredentialsForNetworkingOnly)

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

@copilot address my feedback

[Copilot]

@copilot address my feedback

All feedback addressed in a6e4206. Summary of changes:

• SafeProcessHandle.cs: if/else if/else blocks, single null device handle, disposed in finally after StartCore
• Process.cs: single null device handle opened once after the lock, reused for all stdio
• ProcessStartInfo.cs docs: split into 3 paragraphs with StandardInputHandle/OutputHandle/ErrorHandle refs
• ProcessStartInfo.cs condition: added && string.IsNullOrEmpty(UserName) to the StartDetached branch
• pal_process.c: reduced setsid check nesting
• StartDetachedTests.cs: [Fact] for validation test; removed mobile platform skips; SIGHUP test skips only Windows; added using System.Runtime.InteropServices for PosixSignal

[Copilot]

Pull request overview

This PR introduces a new ProcessStartInfo.StartDetached API to start processes detached from the parent’s session/console across platforms, with additional safety defaults to avoid leaking standard handles and inheritable resources into long-lived detached processes.

Changes:

• Adds ProcessStartInfo.StartDetached to the public surface area (ref + implementation) with validation against UseShellExecute = true.
• Implements platform-specific detaching: DETACHED_PROCESS (Windows), POSIX_SPAWN_SETSID (macOS), and setsid() (other Unix), threading the flag through managed/native interop.
• When detached, defaults stdio to the null device (unless explicitly configured) and restricts handle inheritance by default.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/native/libs/System.Native/pal_process.h	Extends native fork/exec API with startDetached parameter.
src/native/libs/System.Native/pal_process.c	Implements session detachment (POSIX_SPAWN_SETSID / setsid()) based on startDetached.
src/native/libs/System.Native/pal_process_wasi.c	Updates WASI stub signature for startDetached.
src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs	Adds managed interop parameter and forwards startDetached to native.
src/libraries/Common/src/Interop/Windows/Advapi32/Interop.ProcessOptions.cs	Introduces DETACHED_PROCESS constant for process creation flags.
src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs	Adds ProcessStartInfo.StartDetached to the reference assembly API surface.
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs	Adds property + validation + default empty inherited-handles list for detached starts.
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs	Uses a single null-device handle for unset stdio when detached; opens it outside the lock.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs	Applies detached stdio-to-null behavior for SafeProcessHandle.Start.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs	Threads StartDetached through to Interop.Sys.ForkAndExecProcess.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs	Adds DETACHED_PROCESS to creation flags when StartDetached is set.
src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx	Adds error message for StartDetached + UseShellExecute incompatibility.
src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj	Includes new StartDetachedTests.cs in the test project.
src/libraries/System.Diagnostics.Process/tests/StartDetachedTests.cs	Adds validation and behavioral tests for detached starts (including a Unix SIGHUP scenario).

[adamsitnik]

@copilot address my feedback

[Copilot]

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

Note

This review was generated by Copilot AI using three parallel models (Claude Sonnet 4.5, Gemini 2.5 Pro, GPT-4.1) and synthesized into a single report.

🤖 Copilot Code Review — PR #126632

Holistic Assessment

Motivation: Well-justified. A cross-platform API for starting daemon/background processes is a genuine gap — today users must resort to platform-specific flags or shell scripting. Issue #125838 carries the api-approved label and the PR directly addresses it.

Approach: Correct on all three platforms: DETACHED_PROCESS on Windows, POSIX_SPAWN_SETSID on macOS, and setsid() after fork() before exec() on other Unix — all established patterns. Defaulting unset stdio handles to the null device prevents accidental handle inheritance into long-lived detached processes.

Summary: ✅ LGTM. All three models independently raised two concerns that, after careful verification, are false positives:

---

Detailed Findings

✅ Handle disposal in Process.cs — Correctly handled (false positive resolved)

All three models flagged that the nullDeviceHandle in Process.cs might be disposed multiple times (once per childInputHandle, childOutputHandle, childErrorHandle in the finally block) if all three point to the same SafeFileHandle instance. This is safe: SafeHandle.Dispose() uses Interlocked.CompareExchange to set STATE_CLOSED atomically — subsequent calls are no-ops. The handle is properly disposed exactly once (OS-level), regardless of how many managed variables reference it.

✅ POSIX_SPAWN_SETSID availability — No issue

One model raised a concern about POSIX_SPAWN_SETSID not being available on older macOS. The runtime's CMAKE_OSX_DEPLOYMENT_TARGET is 14.0 (Sonoma). POSIX_SPAWN_SETSID was introduced in macOS 10.15, so this is fine.

✅ API Approval — Shape matches api-approved issue #125838

The ref assembly declares public bool StartDetached { get { throw null; } set { } } in alphabetical order (after StandardOutputHandle, before UseCredentialsForNetworkingOnly). This matches the approved shape exactly.

✅ Null device handle lifetime in SafeProcessHandle.cs

The using SafeFileHandle? nullDeviceHandle is disposed at end of scope, after StartCore returns. StartCore passes the handle to native code which immediately dup2s it — the managed-side dispose after return is safe.

✅ setsid() error handling

pal_process.c correctly propagates setsid() failures via ExitChild(waitForChildToExecPipe[WRITE_END_OF_PIPE], errno), so the managed caller receives the error.

✅ Test quality

• StartDetached_GrandchildSurvivesSignalingParent uses deterministic polling (20 ms increments up to WaitInMS) and guards with IsNotNanoServerAndRemoteExecutorSupported.
• StartDetached_StartsAndExitsSuccessfully provides a lightweight smoke test.
• StartDetached_ThrowsOnUseShellExecute covers the validation path.
• Windows SendSignal correctly discards entireProcessGroup with a documented explanation.

💡 Consider a Simpler Test for Basic StartDetached Functionality

The existing test (StartDetached_GrandchildSurvivesSignalingParent) covers the complex grandchild-survival scenario. A si...

The StartDetached_StartsAndExitsSuccessfully smoke test was added in commit 820aef5 and is already present in the current PR.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

[github-actions]

🤖 Copilot Code Review — PR #126632

Note

This review was generated by GitHub Copilot (Claude Opus 4.6). Full source files, git history, API approval issue, and existing review comments were analyzed.

Holistic Assessment

Motivation: Adding ProcessStartInfo.StartDetached addresses a well-known, legitimate need — starting daemon-like processes that are detached from the parent's session/console and survive parent termination. The approved API issue (#125838) demonstrates clear real-world use cases. Justified. ✅

Approach: The implementation correctly uses platform-appropriate mechanisms (DETACHED_PROCESS on Windows, POSIX_SPAWN_SETSID on macOS, setsid() on Linux). The single boolean property is minimal, consistent with existing ProcessStartInfo patterns (CreateNoWindow, CreateNewProcessGroup), and the defaulting of stdio to the null device is the right behavior for detached processes.

Summary: ⚠️Needs Human Review. The implementation is sound — the API matches the approved shape from issue #125838, native code is correct, and tests cover the critical scenarios. I'm flagging for human review because of a handle ownership pattern divergence between Process.Start() and SafeProcessHandle.Start() that, while functionally correct due to SafeHandle.Dispose() idempotency, warrants a human judgment call on whether to align the patterns.

---

Detailed Findings

✅ API Approval — Verified against issue #125838

The api-approved label was applied by @bartonjs on 2025-03-24. The approved API shape includes:

publicsealedpartialclassProcessStartInfo{publicboolStartDetached{get;set;}}

The implementation matches exactly — no extra public surface, correct property signature, correct placement in alphabetical order in the ref assembly (after StandardOutputHandle, before UseCredentialsForNetworkingOnly). ✅

---

✅ Native Implementation — Correct platform mechanisms

• macOS (posix_spawn path): Uses POSIX_SPAWN_SETSID flag (inside #if defined(TARGET_OSX) block). This is an Apple-specific extension available since macOS 10.14; .NET requires macOS 10.15+, so always available. ✅
• Linux (fork path): Calls setsid() after fork, before exec (pal_process.c). The freshly forked child always has PID ≠ PGID, so setsid() will succeed. Placement after dup2 but before credential setup and exec is correct. ✅
• WASI stub: Updated with (void)startDetached to suppress unused parameter warning. ✅

---

✅ Windows Implementation — Correct flag usage

DETACHED_PROCESS = 0x00000008 is correctly OR-ed into creation flags. The decision to allow DETACHED_PROCESS + CREATE_NO_WINDOW together was explicitly reviewed and approved by @jkotas (commit 389dcf7 removed the CreateNoWindow guard, noting the two flags are compatible per Windows API semantics — DETACHED_PROCESS takes precedence). ✅

---

✅ Validation Logic — Correct and complete

• StartDetached + UseShellExecute correctly throws InvalidOperationException ✅
• Error message stored in .resx following conventions ✅
• The StartDetached parameter threads correctly through the entire call chain: ProcessStartInfo → SafeProcessHandle.ForkAndExecProcess → P/Invoke → pal_process.c ✅

---

⚠️ Handle Ownership — Inconsistent pattern between Process.Start() and SafeProcessHandle.Start()

SafeProcessHandle.Start() uses the clean using pattern:

usingSafeFileHandle?nullDeviceHandle=startInfo.StartDetached&&(childInputHandleisnull||childOutputHandleisnull||childErrorHandleisnull)?File.OpenNullHandle():null;

The handle is owned by a single using variable and disposed exactly once.

Process.Start() assigns the same File.OpenNullHandle() result to up to three variables (childInputHandle, childOutputHandle, childErrorHandle), each then independently disposed in the finally block, resulting in up to three Dispose() calls on the same SafeFileHandle.

This is functionally safe — SafeHandle.Dispose() is idempotent by design; subsequent calls are no-ops. The finally block comment even acknowledges non-owning handles. I also see that @adamsitnik specifically requested this pattern in review: "call File.OpenNullHandle() at max once, ensure it gets disposed after process is started."

However, the shared-handle-with-multiple-dispose pattern in Process.Start() is inconsistent with the cleaner SafeProcessHandle.Start() approach. A human reviewer should confirm whether this divergence is intentional or whether a dedicated nullDeviceHandle local with using (matching the other call site) would be preferable.

Not blocking — code is correct as-is.

---

✅ Test Quality — Well-designed coverage

Three tests cover the critical scenarios:

1. StartDetached_ThrowsOnUseShellExecute — Validates the incompatibility check via SafeProcessHandle.Start(). ✅
2. StartDetached_StartsAndExitsSuccessfully — Smoke test verifying a detached process runs and exits normally. ✅
3. StartDetached_GrandchildSurvivesSignalingParent — [Theory] with true/false testing that a detached grandchild survives process-group kill while a non-detached one is killed. Good use of RemoteExecutor, proper cleanup via finally { grandchild.Kill(); }, and appropriate conditional attributes. ✅

The 300ms timeout for the "alive" assertion is reasonable since the grandchild does Thread.Sleep(Timeout.Infinite) — it cannot exit on its own, and setsid() ensures the SIGKILL to the parent's process group doesn't reach it. ✅

---

💡 XML Documentation — Minor clarity opportunity

The XML docs on StartDetached reference StandardInputHandle, StandardOutputHandle, StandardErrorHandle as properties that override null device defaulting. When using Process.Start() (the more common API path), RedirectStandardInput/RedirectStandardOutput/RedirectStandardError also override the null device behavior. The docs already mention these in the first <para>, but the "unless explicitly configured" phrasing in the second <para> could be slightly more inclusive.

Minor — not blocking.

Generated by Code Review for issue #126632 · ◷

[adamsitnik]

/ba-g unrelated

[Copilot]

Description

Adds ProcessStartInfo.StartDetached — a cross-platform way to start a process detached from the parent's session and console, such that it outlives the parent.

API surface

// ref/System.Diagnostics.Process.cspublicsealedclassProcessStartInfo{publicboolStartDetached{get;set;}}

Behavior by platform

Platform	Mechanism
Windows	DETACHED_PROCESS creation flag
macOS	POSIX_SPAWN_SETSID via posix_spawn
Other Unix	setsid() in child after fork, before exec

Key semantics

• ThrowsInvalidOperationException when combined with UseShellExecute = true
• Standard handles (stdin/stdout/stderr) not explicitly set are automatically redirected to the null device (NUL on Windows, /dev/null on Unix) — a single File.OpenNullHandle() is opened at most once and disposed after the process starts; override with RedirectStandardInput/RedirectStandardOutput/RedirectStandardError or StandardInputHandle/StandardOutputHandle/StandardErrorHandle

Example

varpsi=newProcessStartInfo("myserver"){StartDetached=true};// stdin/stdout/stderr automatically go to NUL/dev/nullProcess.Start(psi);

Files changed

• ProcessStartInfo.cs / ref — new property with XML docs linking to the Windows SDK DETACHED_PROCESS documentation; remarks note that UseShellExecute = true will throw; StartDetached placed in alphabetical order in the ref assembly (after StandardOutputHandle, before UseCredentialsForNetworkingOnly)
• Strings.resx — resource key StartDetachedNotCompatible; message states the property cannot be used with UseShellExecute set to true
• Process.cs — opens a single null device handle outside the lock region, reuses it for all unset stdio handles; existing finally block handles disposal
• SafeProcessHandle.cs — if/else if/else blocks; single nullDeviceHandle opened at most once; disposed in a finally block after StartCore
• SafeProcessHandle.Windows.cs — DETACHED_PROCESS flag; constant added to Interop.ProcessOptions.cs
• Interop.ForkAndExecProcess.cs / SafeProcessHandle.Unix.cs — thread startDetached through managed interop chain; fixed indentation of ForkAndExecProcess(...) call inside fixed block
• pal_process.c / pal_process.h / pal_process_wasi.c — native implementation: POSIX_SPAWN_SETSID (macOS), if (startDetached && setsid() == -1) check (other Unix)
• ProcessTests.cs — cross-platform StartDetached_GrandchildSurvivesSignalingParent test guarded by IsNotNanoServerAndRemoteExecutorSupported (PlatformDetection.IsNotWindowsNanoServer && RemoteExecutor.IsSupported); child started with StartDetached=true (own process group on Unix) or CreateNewProcessGroup=true (Windows); kills child's process group with SIGKILL on Unix and SIGQUIT (mapped to CTRL_BREAK_EVENT) on Windows via SendSignal(..., entireProcessGroup: true); grandchild-alive check uses deterministic polling (loop with 20 ms sleep up to WaitInMS) instead of a fixed Thread.Sleep; validates that detached grandchild survives while non-detached grandchild is killed; StartDetached_StartsAndExitsSuccessfully smoke test starts a process with StartDetached=true via RemoteExecutor and verifies it completes successfully
• ProcessTests.Unix.cs — SendSignal(..., entireProcessGroup: true) passes the given signal through Interop.Sys.GetPlatformSignalNumber to kill(-pgid, signalNumber), keeping the helper generic (not hardcoded to any particular signal); call site passes PosixSignal.SIGKILL on Unix for reliable process-group termination
• ProcessTests.Windows.cs — SendSignal documents that entireProcessGroup has no distinct effect on Windows (via _ = entireProcessGroup) since GenerateConsoleCtrlEvent is inherently process-group scoped
• ProcessHandlesTests.cs — [Fact] test for UseShellExecute validation

Changes

• Adds a new cross-platform ProcessStartInfo.StartDetached property
• Redirects standard I/O to null device (single handle, properly disposed) when detached and no explicit handle is provided
• Platform-specific implementation: DETACHED_PROCESS (Windows), POSIX_SPAWN_SETSID (macOS), setsid() (Linux/other Unix)
• Validation: throws when combined with UseShellExecute = true only (CreateNoWindow is not rejected — DETACHED_PROCESS and CREATE_NO_WINDOW are compatible per Windows API semantics)
• XML docs link to the Windows SDK DETACHED_PROCESS documentation; only UseShellExecute listed as incompatible
• Tests covering validation (UseShellExecute), cross-platform process-group isolation (detached grandchild survives, non-detached grandchild is killed), and basic smoke test (StartDetached_StartsAndExitsSuccessfully); all skipped on Windows Nano Server and where RemoteExecutor is unsupported
• SendSignal Unix helper kept generic (signal passed via GetPlatformSignalNumber); call site uses SIGKILL on Unix for reliable group termination
• Deterministic polling replaces fixed Thread.Sleep(200) in grandchild-survival assertion
• Windows SendSignal explicitly discards entireProcessGroup with a clarifying comment that GenerateConsoleCtrlEvent is inherently group-scoped
• StartDetached placed in alphabetical order in the ref assembly (after StandardOutputHandle, before UseCredentialsForNetworkingOnly)

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

@copilot address my feedback

[Copilot]

@copilot address my feedback

All feedback addressed in a6e4206. Summary of changes:

• SafeProcessHandle.cs: if/else if/else blocks, single null device handle, disposed in finally after StartCore
• Process.cs: single null device handle opened once after the lock, reused for all stdio
• ProcessStartInfo.cs docs: split into 3 paragraphs with StandardInputHandle/OutputHandle/ErrorHandle refs
• ProcessStartInfo.cs condition: added && string.IsNullOrEmpty(UserName) to the StartDetached branch
• pal_process.c: reduced setsid check nesting
• StartDetachedTests.cs: [Fact] for validation test; removed mobile platform skips; SIGHUP test skips only Windows; added using System.Runtime.InteropServices for PosixSignal

[Copilot]

Pull request overview

This PR introduces a new ProcessStartInfo.StartDetached API to start processes detached from the parent’s session/console across platforms, with additional safety defaults to avoid leaking standard handles and inheritable resources into long-lived detached processes.

Changes:

• Adds ProcessStartInfo.StartDetached to the public surface area (ref + implementation) with validation against UseShellExecute = true.
• Implements platform-specific detaching: DETACHED_PROCESS (Windows), POSIX_SPAWN_SETSID (macOS), and setsid() (other Unix), threading the flag through managed/native interop.
• When detached, defaults stdio to the null device (unless explicitly configured) and restricts handle inheritance by default.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/native/libs/System.Native/pal_process.h	Extends native fork/exec API with startDetached parameter.
src/native/libs/System.Native/pal_process.c	Implements session detachment (POSIX_SPAWN_SETSID / setsid()) based on startDetached.
src/native/libs/System.Native/pal_process_wasi.c	Updates WASI stub signature for startDetached.
src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs	Adds managed interop parameter and forwards startDetached to native.
src/libraries/Common/src/Interop/Windows/Advapi32/Interop.ProcessOptions.cs	Introduces DETACHED_PROCESS constant for process creation flags.
src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs	Adds ProcessStartInfo.StartDetached to the reference assembly API surface.
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs	Adds property + validation + default empty inherited-handles list for detached starts.
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs	Uses a single null-device handle for unset stdio when detached; opens it outside the lock.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs	Applies detached stdio-to-null behavior for SafeProcessHandle.Start.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs	Threads StartDetached through to Interop.Sys.ForkAndExecProcess.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs	Adds DETACHED_PROCESS to creation flags when StartDetached is set.
src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx	Adds error message for StartDetached + UseShellExecute incompatibility.
src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj	Includes new StartDetachedTests.cs in the test project.
src/libraries/System.Diagnostics.Process/tests/StartDetachedTests.cs	Adds validation and behavioral tests for detached starts (including a Unix SIGHUP scenario).

[adamsitnik]

@copilot address my feedback

[Copilot]

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

Note

This review was generated by Copilot AI using three parallel models (Claude Sonnet 4.5, Gemini 2.5 Pro, GPT-4.1) and synthesized into a single report.

🤖 Copilot Code Review — PR #126632

Holistic Assessment

Motivation: Well-justified. A cross-platform API for starting daemon/background processes is a genuine gap — today users must resort to platform-specific flags or shell scripting. Issue #125838 carries the api-approved label and the PR directly addresses it.

Approach: Correct on all three platforms: DETACHED_PROCESS on Windows, POSIX_SPAWN_SETSID on macOS, and setsid() after fork() before exec() on other Unix — all established patterns. Defaulting unset stdio handles to the null device prevents accidental handle inheritance into long-lived detached processes.

Summary: ✅ LGTM. All three models independently raised two concerns that, after careful verification, are false positives:

---

Detailed Findings

✅ Handle disposal in Process.cs — Correctly handled (false positive resolved)

All three models flagged that the nullDeviceHandle in Process.cs might be disposed multiple times (once per childInputHandle, childOutputHandle, childErrorHandle in the finally block) if all three point to the same SafeFileHandle instance. This is safe: SafeHandle.Dispose() uses Interlocked.CompareExchange to set STATE_CLOSED atomically — subsequent calls are no-ops. The handle is properly disposed exactly once (OS-level), regardless of how many managed variables reference it.

✅ POSIX_SPAWN_SETSID availability — No issue

One model raised a concern about POSIX_SPAWN_SETSID not being available on older macOS. The runtime's CMAKE_OSX_DEPLOYMENT_TARGET is 14.0 (Sonoma). POSIX_SPAWN_SETSID was introduced in macOS 10.15, so this is fine.

✅ API Approval — Shape matches api-approved issue #125838

The ref assembly declares public bool StartDetached { get { throw null; } set { } } in alphabetical order (after StandardOutputHandle, before UseCredentialsForNetworkingOnly). This matches the approved shape exactly.

✅ Null device handle lifetime in SafeProcessHandle.cs

The using SafeFileHandle? nullDeviceHandle is disposed at end of scope, after StartCore returns. StartCore passes the handle to native code which immediately dup2s it — the managed-side dispose after return is safe.

✅ setsid() error handling

pal_process.c correctly propagates setsid() failures via ExitChild(waitForChildToExecPipe[WRITE_END_OF_PIPE], errno), so the managed caller receives the error.

✅ Test quality

• StartDetached_GrandchildSurvivesSignalingParent uses deterministic polling (20 ms increments up to WaitInMS) and guards with IsNotNanoServerAndRemoteExecutorSupported.
• StartDetached_StartsAndExitsSuccessfully provides a lightweight smoke test.
• StartDetached_ThrowsOnUseShellExecute covers the validation path.
• Windows SendSignal correctly discards entireProcessGroup with a documented explanation.

💡 Consider a Simpler Test for Basic StartDetached Functionality

The existing test (StartDetached_GrandchildSurvivesSignalingParent) covers the complex grandchild-survival scenario. A si...

The StartDetached_StartsAndExitsSuccessfully smoke test was added in commit 820aef5 and is already present in the current PR.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

[github-actions]

🤖 Copilot Code Review — PR #126632

Note

This review was generated by GitHub Copilot (Claude Opus 4.6). Full source files, git history, API approval issue, and existing review comments were analyzed.

Holistic Assessment

Motivation: Adding ProcessStartInfo.StartDetached addresses a well-known, legitimate need — starting daemon-like processes that are detached from the parent's session/console and survive parent termination. The approved API issue (#125838) demonstrates clear real-world use cases. Justified. ✅

Approach: The implementation correctly uses platform-appropriate mechanisms (DETACHED_PROCESS on Windows, POSIX_SPAWN_SETSID on macOS, setsid() on Linux). The single boolean property is minimal, consistent with existing ProcessStartInfo patterns (CreateNoWindow, CreateNewProcessGroup), and the defaulting of stdio to the null device is the right behavior for detached processes.

Summary: ⚠️Needs Human Review. The implementation is sound — the API matches the approved shape from issue #125838, native code is correct, and tests cover the critical scenarios. I'm flagging for human review because of a handle ownership pattern divergence between Process.Start() and SafeProcessHandle.Start() that, while functionally correct due to SafeHandle.Dispose() idempotency, warrants a human judgment call on whether to align the patterns.

---

Detailed Findings

✅ API Approval — Verified against issue #125838

The api-approved label was applied by @bartonjs on 2025-03-24. The approved API shape includes:

publicsealedpartialclassProcessStartInfo{publicboolStartDetached{get;set;}}

The implementation matches exactly — no extra public surface, correct property signature, correct placement in alphabetical order in the ref assembly (after StandardOutputHandle, before UseCredentialsForNetworkingOnly). ✅

---

✅ Native Implementation — Correct platform mechanisms

• macOS (posix_spawn path): Uses POSIX_SPAWN_SETSID flag (inside #if defined(TARGET_OSX) block). This is an Apple-specific extension available since macOS 10.14; .NET requires macOS 10.15+, so always available. ✅
• Linux (fork path): Calls setsid() after fork, before exec (pal_process.c). The freshly forked child always has PID ≠ PGID, so setsid() will succeed. Placement after dup2 but before credential setup and exec is correct. ✅
• WASI stub: Updated with (void)startDetached to suppress unused parameter warning. ✅

---

✅ Windows Implementation — Correct flag usage

DETACHED_PROCESS = 0x00000008 is correctly OR-ed into creation flags. The decision to allow DETACHED_PROCESS + CREATE_NO_WINDOW together was explicitly reviewed and approved by @jkotas (commit 389dcf7 removed the CreateNoWindow guard, noting the two flags are compatible per Windows API semantics — DETACHED_PROCESS takes precedence). ✅

---

✅ Validation Logic — Correct and complete

• StartDetached + UseShellExecute correctly throws InvalidOperationException ✅
• Error message stored in .resx following conventions ✅
• The StartDetached parameter threads correctly through the entire call chain: ProcessStartInfo → SafeProcessHandle.ForkAndExecProcess → P/Invoke → pal_process.c ✅

---

⚠️ Handle Ownership — Inconsistent pattern between Process.Start() and SafeProcessHandle.Start()

SafeProcessHandle.Start() uses the clean using pattern:

usingSafeFileHandle?nullDeviceHandle=startInfo.StartDetached&&(childInputHandleisnull||childOutputHandleisnull||childErrorHandleisnull)?File.OpenNullHandle():null;

The handle is owned by a single using variable and disposed exactly once.

Process.Start() assigns the same File.OpenNullHandle() result to up to three variables (childInputHandle, childOutputHandle, childErrorHandle), each then independently disposed in the finally block, resulting in up to three Dispose() calls on the same SafeFileHandle.

This is functionally safe — SafeHandle.Dispose() is idempotent by design; subsequent calls are no-ops. The finally block comment even acknowledges non-owning handles. I also see that @adamsitnik specifically requested this pattern in review: "call File.OpenNullHandle() at max once, ensure it gets disposed after process is started."

However, the shared-handle-with-multiple-dispose pattern in Process.Start() is inconsistent with the cleaner SafeProcessHandle.Start() approach. A human reviewer should confirm whether this divergence is intentional or whether a dedicated nullDeviceHandle local with using (matching the other call site) would be preferable.

Not blocking — code is correct as-is.

---

✅ Test Quality — Well-designed coverage

Three tests cover the critical scenarios:

1. StartDetached_ThrowsOnUseShellExecute — Validates the incompatibility check via SafeProcessHandle.Start(). ✅
2. StartDetached_StartsAndExitsSuccessfully — Smoke test verifying a detached process runs and exits normally. ✅
3. StartDetached_GrandchildSurvivesSignalingParent — [Theory] with true/false testing that a detached grandchild survives process-group kill while a non-detached one is killed. Good use of RemoteExecutor, proper cleanup via finally { grandchild.Kill(); }, and appropriate conditional attributes. ✅

The 300ms timeout for the "alive" assertion is reasonable since the grandchild does Thread.Sleep(Timeout.Infinite) — it cannot exit on its own, and setsid() ensures the SIGKILL to the parent's process group doesn't reach it. ✅

---

💡 XML Documentation — Minor clarity opportunity

The XML docs on StartDetached reference StandardInputHandle, StandardOutputHandle, StandardErrorHandle as properties that override null device defaulting. When using Process.Start() (the more common API path), RedirectStandardInput/RedirectStandardOutput/RedirectStandardError also override the null device behavior. The docs already mention these in the first <para>, but the "unless explicitly configured" phrasing in the second <para> could be slightly more inclusive.

Minor — not blocking.

Generated by Code Review for issue #126632 · ◷

[adamsitnik]

/ba-g unrelated

[Copilot]

Description

Adds ProcessStartInfo.StartDetached — a cross-platform way to start a process detached from the parent's session and console, such that it outlives the parent.

API surface

// ref/System.Diagnostics.Process.cspublicsealedclassProcessStartInfo{publicboolStartDetached{get;set;}}

Behavior by platform

Platform	Mechanism
Windows	DETACHED_PROCESS creation flag
macOS	POSIX_SPAWN_SETSID via posix_spawn
Other Unix	setsid() in child after fork, before exec

Key semantics

• ThrowsInvalidOperationException when combined with UseShellExecute = true
• Standard handles (stdin/stdout/stderr) not explicitly set are automatically redirected to the null device (NUL on Windows, /dev/null on Unix) — a single File.OpenNullHandle() is opened at most once and disposed after the process starts; override with RedirectStandardInput/RedirectStandardOutput/RedirectStandardError or StandardInputHandle/StandardOutputHandle/StandardErrorHandle

Example

varpsi=newProcessStartInfo("myserver"){StartDetached=true};// stdin/stdout/stderr automatically go to NUL/dev/nullProcess.Start(psi);

Files changed

• ProcessStartInfo.cs / ref — new property with XML docs linking to the Windows SDK DETACHED_PROCESS documentation; remarks note that UseShellExecute = true will throw; StartDetached placed in alphabetical order in the ref assembly (after StandardOutputHandle, before UseCredentialsForNetworkingOnly)
• Strings.resx — resource key StartDetachedNotCompatible; message states the property cannot be used with UseShellExecute set to true
• Process.cs — opens a single null device handle outside the lock region, reuses it for all unset stdio handles; existing finally block handles disposal
• SafeProcessHandle.cs — if/else if/else blocks; single nullDeviceHandle opened at most once; disposed in a finally block after StartCore
• SafeProcessHandle.Windows.cs — DETACHED_PROCESS flag; constant added to Interop.ProcessOptions.cs
• Interop.ForkAndExecProcess.cs / SafeProcessHandle.Unix.cs — thread startDetached through managed interop chain; fixed indentation of ForkAndExecProcess(...) call inside fixed block
• pal_process.c / pal_process.h / pal_process_wasi.c — native implementation: POSIX_SPAWN_SETSID (macOS), if (startDetached && setsid() == -1) check (other Unix)
• ProcessTests.cs — cross-platform StartDetached_GrandchildSurvivesSignalingParent test guarded by IsNotNanoServerAndRemoteExecutorSupported (PlatformDetection.IsNotWindowsNanoServer && RemoteExecutor.IsSupported); child started with StartDetached=true (own process group on Unix) or CreateNewProcessGroup=true (Windows); kills child's process group with SIGKILL on Unix and SIGQUIT (mapped to CTRL_BREAK_EVENT) on Windows via SendSignal(..., entireProcessGroup: true); grandchild-alive check uses deterministic polling (loop with 20 ms sleep up to WaitInMS) instead of a fixed Thread.Sleep; validates that detached grandchild survives while non-detached grandchild is killed; StartDetached_StartsAndExitsSuccessfully smoke test starts a process with StartDetached=true via RemoteExecutor and verifies it completes successfully
• ProcessTests.Unix.cs — SendSignal(..., entireProcessGroup: true) passes the given signal through Interop.Sys.GetPlatformSignalNumber to kill(-pgid, signalNumber), keeping the helper generic (not hardcoded to any particular signal); call site passes PosixSignal.SIGKILL on Unix for reliable process-group termination
• ProcessTests.Windows.cs — SendSignal documents that entireProcessGroup has no distinct effect on Windows (via _ = entireProcessGroup) since GenerateConsoleCtrlEvent is inherently process-group scoped
• ProcessHandlesTests.cs — [Fact] test for UseShellExecute validation

Changes

• Adds a new cross-platform ProcessStartInfo.StartDetached property
• Redirects standard I/O to null device (single handle, properly disposed) when detached and no explicit handle is provided
• Platform-specific implementation: DETACHED_PROCESS (Windows), POSIX_SPAWN_SETSID (macOS), setsid() (Linux/other Unix)
• Validation: throws when combined with UseShellExecute = true only (CreateNoWindow is not rejected — DETACHED_PROCESS and CREATE_NO_WINDOW are compatible per Windows API semantics)
• XML docs link to the Windows SDK DETACHED_PROCESS documentation; only UseShellExecute listed as incompatible
• Tests covering validation (UseShellExecute), cross-platform process-group isolation (detached grandchild survives, non-detached grandchild is killed), and basic smoke test (StartDetached_StartsAndExitsSuccessfully); all skipped on Windows Nano Server and where RemoteExecutor is unsupported
• SendSignal Unix helper kept generic (signal passed via GetPlatformSignalNumber); call site uses SIGKILL on Unix for reliable group termination
• Deterministic polling replaces fixed Thread.Sleep(200) in grandchild-survival assertion
• Windows SendSignal explicitly discards entireProcessGroup with a clarifying comment that GenerateConsoleCtrlEvent is inherently group-scoped
• StartDetached placed in alphabetical order in the ref assembly (after StandardOutputHandle, before UseCredentialsForNetworkingOnly)

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

@copilot address my feedback

[Copilot]

@copilot address my feedback

All feedback addressed in a6e4206. Summary of changes:

• SafeProcessHandle.cs: if/else if/else blocks, single null device handle, disposed in finally after StartCore
• Process.cs: single null device handle opened once after the lock, reused for all stdio
• ProcessStartInfo.cs docs: split into 3 paragraphs with StandardInputHandle/OutputHandle/ErrorHandle refs
• ProcessStartInfo.cs condition: added && string.IsNullOrEmpty(UserName) to the StartDetached branch
• pal_process.c: reduced setsid check nesting
• StartDetachedTests.cs: [Fact] for validation test; removed mobile platform skips; SIGHUP test skips only Windows; added using System.Runtime.InteropServices for PosixSignal

[Copilot]

Pull request overview

This PR introduces a new ProcessStartInfo.StartDetached API to start processes detached from the parent’s session/console across platforms, with additional safety defaults to avoid leaking standard handles and inheritable resources into long-lived detached processes.

Changes:

• Adds ProcessStartInfo.StartDetached to the public surface area (ref + implementation) with validation against UseShellExecute = true.
• Implements platform-specific detaching: DETACHED_PROCESS (Windows), POSIX_SPAWN_SETSID (macOS), and setsid() (other Unix), threading the flag through managed/native interop.
• When detached, defaults stdio to the null device (unless explicitly configured) and restricts handle inheritance by default.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/native/libs/System.Native/pal_process.h	Extends native fork/exec API with startDetached parameter.
src/native/libs/System.Native/pal_process.c	Implements session detachment (POSIX_SPAWN_SETSID / setsid()) based on startDetached.
src/native/libs/System.Native/pal_process_wasi.c	Updates WASI stub signature for startDetached.
src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs	Adds managed interop parameter and forwards startDetached to native.
src/libraries/Common/src/Interop/Windows/Advapi32/Interop.ProcessOptions.cs	Introduces DETACHED_PROCESS constant for process creation flags.
src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs	Adds ProcessStartInfo.StartDetached to the reference assembly API surface.
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs	Adds property + validation + default empty inherited-handles list for detached starts.
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs	Uses a single null-device handle for unset stdio when detached; opens it outside the lock.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs	Applies detached stdio-to-null behavior for SafeProcessHandle.Start.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs	Threads StartDetached through to Interop.Sys.ForkAndExecProcess.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs	Adds DETACHED_PROCESS to creation flags when StartDetached is set.
src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx	Adds error message for StartDetached + UseShellExecute incompatibility.
src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj	Includes new StartDetachedTests.cs in the test project.
src/libraries/System.Diagnostics.Process/tests/StartDetachedTests.cs	Adds validation and behavioral tests for detached starts (including a Unix SIGHUP scenario).

[adamsitnik]

@copilot address my feedback

[Copilot]

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

Note

This review was generated by Copilot AI using three parallel models (Claude Sonnet 4.5, Gemini 2.5 Pro, GPT-4.1) and synthesized into a single report.

🤖 Copilot Code Review — PR #126632

Holistic Assessment

Motivation: Well-justified. A cross-platform API for starting daemon/background processes is a genuine gap — today users must resort to platform-specific flags or shell scripting. Issue #125838 carries the api-approved label and the PR directly addresses it.

Approach: Correct on all three platforms: DETACHED_PROCESS on Windows, POSIX_SPAWN_SETSID on macOS, and setsid() after fork() before exec() on other Unix — all established patterns. Defaulting unset stdio handles to the null device prevents accidental handle inheritance into long-lived detached processes.

Summary: ✅ LGTM. All three models independently raised two concerns that, after careful verification, are false positives:

---

Detailed Findings

✅ Handle disposal in Process.cs — Correctly handled (false positive resolved)

All three models flagged that the nullDeviceHandle in Process.cs might be disposed multiple times (once per childInputHandle, childOutputHandle, childErrorHandle in the finally block) if all three point to the same SafeFileHandle instance. This is safe: SafeHandle.Dispose() uses Interlocked.CompareExchange to set STATE_CLOSED atomically — subsequent calls are no-ops. The handle is properly disposed exactly once (OS-level), regardless of how many managed variables reference it.

✅ POSIX_SPAWN_SETSID availability — No issue

One model raised a concern about POSIX_SPAWN_SETSID not being available on older macOS. The runtime's CMAKE_OSX_DEPLOYMENT_TARGET is 14.0 (Sonoma). POSIX_SPAWN_SETSID was introduced in macOS 10.15, so this is fine.

✅ API Approval — Shape matches api-approved issue #125838

The ref assembly declares public bool StartDetached { get { throw null; } set { } } in alphabetical order (after StandardOutputHandle, before UseCredentialsForNetworkingOnly). This matches the approved shape exactly.

✅ Null device handle lifetime in SafeProcessHandle.cs

The using SafeFileHandle? nullDeviceHandle is disposed at end of scope, after StartCore returns. StartCore passes the handle to native code which immediately dup2s it — the managed-side dispose after return is safe.

✅ setsid() error handling

pal_process.c correctly propagates setsid() failures via ExitChild(waitForChildToExecPipe[WRITE_END_OF_PIPE], errno), so the managed caller receives the error.

✅ Test quality

• StartDetached_GrandchildSurvivesSignalingParent uses deterministic polling (20 ms increments up to WaitInMS) and guards with IsNotNanoServerAndRemoteExecutorSupported.
• StartDetached_StartsAndExitsSuccessfully provides a lightweight smoke test.
• StartDetached_ThrowsOnUseShellExecute covers the validation path.
• Windows SendSignal correctly discards entireProcessGroup with a documented explanation.

💡 Consider a Simpler Test for Basic StartDetached Functionality

The existing test (StartDetached_GrandchildSurvivesSignalingParent) covers the complex grandchild-survival scenario. A si...

The StartDetached_StartsAndExitsSuccessfully smoke test was added in commit 820aef5 and is already present in the current PR.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

[github-actions]

🤖 Copilot Code Review — PR #126632

Note

This review was generated by GitHub Copilot (Claude Opus 4.6). Full source files, git history, API approval issue, and existing review comments were analyzed.

Holistic Assessment

Motivation: Adding ProcessStartInfo.StartDetached addresses a well-known, legitimate need — starting daemon-like processes that are detached from the parent's session/console and survive parent termination. The approved API issue (#125838) demonstrates clear real-world use cases. Justified. ✅

Approach: The implementation correctly uses platform-appropriate mechanisms (DETACHED_PROCESS on Windows, POSIX_SPAWN_SETSID on macOS, setsid() on Linux). The single boolean property is minimal, consistent with existing ProcessStartInfo patterns (CreateNoWindow, CreateNewProcessGroup), and the defaulting of stdio to the null device is the right behavior for detached processes.

Summary: ⚠️Needs Human Review. The implementation is sound — the API matches the approved shape from issue #125838, native code is correct, and tests cover the critical scenarios. I'm flagging for human review because of a handle ownership pattern divergence between Process.Start() and SafeProcessHandle.Start() that, while functionally correct due to SafeHandle.Dispose() idempotency, warrants a human judgment call on whether to align the patterns.

---

Detailed Findings

✅ API Approval — Verified against issue #125838

The api-approved label was applied by @bartonjs on 2025-03-24. The approved API shape includes:

publicsealedpartialclassProcessStartInfo{publicboolStartDetached{get;set;}}

The implementation matches exactly — no extra public surface, correct property signature, correct placement in alphabetical order in the ref assembly (after StandardOutputHandle, before UseCredentialsForNetworkingOnly). ✅

---

✅ Native Implementation — Correct platform mechanisms

• macOS (posix_spawn path): Uses POSIX_SPAWN_SETSID flag (inside #if defined(TARGET_OSX) block). This is an Apple-specific extension available since macOS 10.14; .NET requires macOS 10.15+, so always available. ✅
• Linux (fork path): Calls setsid() after fork, before exec (pal_process.c). The freshly forked child always has PID ≠ PGID, so setsid() will succeed. Placement after dup2 but before credential setup and exec is correct. ✅
• WASI stub: Updated with (void)startDetached to suppress unused parameter warning. ✅

---

✅ Windows Implementation — Correct flag usage

DETACHED_PROCESS = 0x00000008 is correctly OR-ed into creation flags. The decision to allow DETACHED_PROCESS + CREATE_NO_WINDOW together was explicitly reviewed and approved by @jkotas (commit 389dcf7 removed the CreateNoWindow guard, noting the two flags are compatible per Windows API semantics — DETACHED_PROCESS takes precedence). ✅

---

✅ Validation Logic — Correct and complete

• StartDetached + UseShellExecute correctly throws InvalidOperationException ✅
• Error message stored in .resx following conventions ✅
• The StartDetached parameter threads correctly through the entire call chain: ProcessStartInfo → SafeProcessHandle.ForkAndExecProcess → P/Invoke → pal_process.c ✅

---

⚠️ Handle Ownership — Inconsistent pattern between Process.Start() and SafeProcessHandle.Start()

SafeProcessHandle.Start() uses the clean using pattern:

usingSafeFileHandle?nullDeviceHandle=startInfo.StartDetached&&(childInputHandleisnull||childOutputHandleisnull||childErrorHandleisnull)?File.OpenNullHandle():null;

The handle is owned by a single using variable and disposed exactly once.

Process.Start() assigns the same File.OpenNullHandle() result to up to three variables (childInputHandle, childOutputHandle, childErrorHandle), each then independently disposed in the finally block, resulting in up to three Dispose() calls on the same SafeFileHandle.

This is functionally safe — SafeHandle.Dispose() is idempotent by design; subsequent calls are no-ops. The finally block comment even acknowledges non-owning handles. I also see that @adamsitnik specifically requested this pattern in review: "call File.OpenNullHandle() at max once, ensure it gets disposed after process is started."

However, the shared-handle-with-multiple-dispose pattern in Process.Start() is inconsistent with the cleaner SafeProcessHandle.Start() approach. A human reviewer should confirm whether this divergence is intentional or whether a dedicated nullDeviceHandle local with using (matching the other call site) would be preferable.

Not blocking — code is correct as-is.

---

✅ Test Quality — Well-designed coverage

Three tests cover the critical scenarios:

1. StartDetached_ThrowsOnUseShellExecute — Validates the incompatibility check via SafeProcessHandle.Start(). ✅
2. StartDetached_StartsAndExitsSuccessfully — Smoke test verifying a detached process runs and exits normally. ✅
3. StartDetached_GrandchildSurvivesSignalingParent — [Theory] with true/false testing that a detached grandchild survives process-group kill while a non-detached one is killed. Good use of RemoteExecutor, proper cleanup via finally { grandchild.Kill(); }, and appropriate conditional attributes. ✅

The 300ms timeout for the "alive" assertion is reasonable since the grandchild does Thread.Sleep(Timeout.Infinite) — it cannot exit on its own, and setsid() ensures the SIGKILL to the parent's process group doesn't reach it. ✅

---

💡 XML Documentation — Minor clarity opportunity

The XML docs on StartDetached reference StandardInputHandle, StandardOutputHandle, StandardErrorHandle as properties that override null device defaulting. When using Process.Start() (the more common API path), RedirectStandardInput/RedirectStandardOutput/RedirectStandardError also override the null device behavior. The docs already mention these in the first <para>, but the "unless explicitly configured" phrasing in the second <para> could be slightly more inclusive.

Minor — not blocking.

Generated by Code Review for issue #126632 · ◷

[adamsitnik]

/ba-g unrelated