Add Linux support for ProcessStartInfo.KillOnParentExit using prctl(PR_SET_PDEATHSIG)

[Copilot]

Contributes to #101985
Approved API: #125838 (comment)

Description

Extends ProcessStartInfo.KillOnParentExit with Linux support using prctl(PR_SET_PDEATHSIG, SIGKILL).

On Linux, PR_SET_PDEATHSIG sends the death signal when the thread that called prctl exits, not when the process exits. To ensure the signal fires only on process exit, this implementation creates a dedicated long-lived thread that performs all fork+exec operations when KillOnParentExit is set. The child process calls prctl(PR_SET_PDEATHSIG, SIGKILL) after fork, before exec. After prctl, the child verifies it hasn't been reparented (e.g., to a subreaper in a container) by comparing getppid() against the expected parent PID captured before fork.

Changes

Native layer (pal_process.c, pal_process.h, pal_process_wasi.c)

• Added killOnParentExit parameter to SystemNative_ForkAndExecProcess
• Implemented dedicated pdeathsig thread with mutex/condvar synchronization
• Child calls prctl(PR_SET_PDEATHSIG, SIGKILL) after fork, before exec
• Thread-safe initialization using C11 _Atomic int with atomic_load_explicit/atomic_store_explicit for double-checked locking
• Post-prctl orphan check captures parent PID before fork and verifies getppid() matches after prctl, correctly detecting reparenting to subreapers
• Moved ForkAndExecProcessInternal forward declaration outside #if HAVE_PR_SET_PDEATHSIG so it is visible on all platforms (including WASM)
• Used pthread_cond_broadcast (not pthread_cond_signal) for done condition to prevent lost-wakeup deadlock when multiple callers wait concurrently
• Added error checking for pthread_attr_init and pthread_attr_setdetachstate return values, with proper cleanup on failure (following the established pattern from pal_signal.c)
• Placed expectedParentPid declaration in the variable declarations block before the first goto to fix C++ -Wjump-misses-init build error
• Fixed concurrent-submit race: callers now wait for the request slot to be free (s_pdeathsig_request == NULL) before submitting, and each request has a per-request done flag so callers wait on their own completion rather than a shared pointer becoming NULL
• Added assert(s_pdeathsig_request == NULL) before submitting request to the dedicated thread

CMake / Config

• Added HAVE_PR_SET_PDEATHSIG check in configure.cmake (checks sys/prctl.h)
• Added #cmakedefine01 HAVE_PR_SET_PDEATHSIG in pal_config.h.in

Managed layer

• Added killOnParentExit parameter to Interop.ForkAndExecProcess.cs P/Invoke
• Passed startInfo.KillOnParentExit through SafeProcessHandle.Unix.cs, gated with OperatingSystem.IsLinux() to pass false on non-Linux Unix platforms
• Added [SupportedOSPlatform("linux")] to ref and implementation

Tests

• Updated KillOnParentExitTests.cs to run on TestPlatforms.Windows | TestPlatforms.Linux

[adamsitnik]

@copilot fix following build failure:

FAILED: libs-native/System.Native/CMakeFiles/System.Native-Static.dir/pal_process.c.o 
  /usr/bin/gcc-15 -DBUILDENV_CHECKED=1 -DDEBUG -DDISABLE_CONTRACTS -DHAS_CONSOLE_SIGNALS -DHOST_64BIT -DHOST_AMD64 -DHOST_UNIX -DPALEXPORT=EXTERN_C -DTARGET_64BIT -DTARGET_AMD64 -DTARGET_LINUX -DTARGET_UNIX -DURTBLDENV_FRIENDLY=Checked -D_DBG -D_DEBUG -D_FILE_OFFSET_BITS=64 -D_TIME_BITS=64 -I/__w/1/s/artifacts/obj/coreclr/linux.x64.Checked/libs-native/System.Native -I/__w/1/s/src/native/libs/System.Native -I/__w/1/s/src/native -I/__w/1/s/src/native/inc -std=gnu99 -Wno-declaration-after-statement -D_GNU_SOURCE -std=gnu11 -fPIC -O2 -Wall -g -fno-omit-frame-pointer -fno-strict-overflow -fno-strict-aliasing -fstack-protector-strong -Werror -Wno-unused-variable -Wno-unused-value -Wno-unused-function -Wno-tautological-compare -Wno-unknown-pragmas -Wimplicit-fallthrough -Wno-unused-but-set-variable -ffp-contract=off -Wno-uninitialized -Wno-strict-aliasing -Wno-array-bounds -Wno-stringop-truncation -fsigned-char -fvisibility=hidden -ffunction-sections -I/__w/1/s/src/native/libs/Common -I/__w/1/s/artifacts/obj/coreclr/linux.x64.Checked/libs-native/Common -MD -MT libs-native/System.Native/CMakeFiles/System.Native-Static.dir/pal_process.c.o -MF libs-native/System.Native/CMakeFiles/System.Native-Static.dir/pal_process.c.o.d -o libs-native/System.Native/CMakeFiles/System.Native-Static.dir/pal_process.c.o -c /__w/1/s/src/native/libs/System.Native/pal_process.c
  /__w/1/s/src/native/libs/System.Native/pal_process.c: In function ‘PDeathSigThreadFunc’:
  /__w/1/s/src/native/libs/System.Native/pal_process.c:411:1: error: no return statement in function returning non-void [-Werror=return-type]
    411 | }
        | ^
  cc1: all warnings being treated as errors

[Copilot]

@copilot fix following build failure:

'''log...

Fixed in dc2712a. Added return NULL; after the infinite while(1) loop to satisfy GCC's -Werror=return-type, with a clang pragma to suppress -Wunreachable-code-return (which was the original reason the return was removed).

[adamsitnik]

@copilot fix the WASM build

  FAILED: libs-native/System.Native/CMakeFiles/System.Native-Static.dir/pal_process.c.o 
  /__w/1/s/src/mono/browser/emsdk/emscripten/emcc -DCOMPILER_SUPPORTS_W_RESERVED_IDENTIFIER -DDISABLE_CONTRACTS -DGEN_PINVOKE=1 -DHOST_32BIT=1 -DHOST_BROWSER -DHOST_UNIX -DHOST_WASM -DNDEBUG -DTARGET_32BIT -DTARGET_BROWSER -DTARGET_UNIX -DTARGET_WASM -DTARGET_WASM32 -DURTBLDENV_FRIENDLY=Retail -D_FILE_OFFSET_BITS=64 -D_TIME_BITS=64 -I/__w/1/s/artifacts/obj/coreclr/browser.wasm.Release/libs-native/System.Native -I/__w/1/s/src/native/libs/System.Native -I/__w/1/s/src/native -I/__w/1/s/src/native/inc -std=gnu99 -Wno-declaration-after-statement -Wno-pre-c11-compat -O3 -DNDEBUG -std=gnu11 -fwasm-exceptions -mbulk-memory -msimd128 -O3 -Wall -Wno-null-conversion -glldb -Wno-unused-parameter -Wno-alloca -Wno-implicit-int-float-conversion -fno-omit-frame-pointer -fno-strict-overflow -fno-strict-aliasing -Werror -Wno-unused-variable -Wno-unused-value -Wno-unused-function -Wno-tautological-compare -Wno-unknown-pragmas -Wimplicit-fallthrough -Wno-unused-but-set-variable -ffp-contract=off -Wno-unknown-warning-option -ferror-limit=4096 -Wno-unused-private-field -Wno-constant-logical-operand -Wno-pragma-pack -Wno-incompatible-ms-struct -Wno-reserved-identifier -Wno-unsafe-buffer-usage -Wno-single-bit-bitfield-constant-conversion -Wno-cast-function-type-strict -Wno-switch-default -fsigned-char -fvisibility=hidden -ffunction-sections -I/__w/1/s/src/native/libs/Common -I/__w/1/s/artifacts/obj/coreclr/browser.wasm.Release/libs-native/Common -Oz -Weverything -Wno-format-nonliteral -Wno-disabled-macro-expansion -Wno-padded -Wno-empty-translation-unit -Wno-cast-align -Wno-typedef-redefinition -Wno-thread-safety-analysis -Wno-c11-extensions -Wno-pre-c11-compat -MD -MT libs-native/System.Native/CMakeFiles/System.Native-Static.dir/pal_process.c.o -MF libs-native/System.Native/CMakeFiles/System.Native-Static.dir/pal_process.c.o.d -o libs-native/System.Native/CMakeFiles/System.Native-Static.dir/pal_process.c.o -c /__w/1/s/src/native/libs/System.Native/pal_process.c
  /__w/1/s/src/native/libs/System.Native/pal_process.c:537:12: error: call to undeclared function 'ForkAndExecProcessInternal'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    537 |     return ForkAndExecProcessInternal(
        |            ^
  /__w/1/s/src/native/libs/System.Native/pal_process.c:544:16: error: static declaration of 'ForkAndExecProcessInternal' follows non-static declaration
    544 | static int32_t ForkAndExecProcessInternal(
        |                ^
  /__w/1/s/src/native/libs/System.Native/pal_process.c:537:12: note: previous implicit declaration is here
    537 |     return ForkAndExecProcessInternal(
        |            ^
  2 errors generated.

[Copilot]

@copilot fix the WASM build

'''log...

Fixed in 3723cc4. The ForkAndExecProcessInternal forward declaration was inside the #if HAVE_PR_SET_PDEATHSIG block, so on WASM (where it's 0) it wasn't visible. Moved the forward declaration before the #if block so it's always available.

[Copilot]

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

🤖 Copilot Code Review — PR #127112

Note

This review was AI/Copilot-generated using the code-review skill, with sub-agent analysis from Gemini 2.5 Pro and GPT-5.3-Codex.

Holistic Assessment

Motivation: The PR extends ProcessStartInfo.KillOnParentExit to Linux using prctl(PR_SET_PDEATHSIG, SIGKILL). This is part of the approved API in #125838 (api-approved label). The feature addresses a genuine need — ensuring child process cleanup on parent exit is a common requirement.

Approach: The dedicated long-lived thread design is the correct approach. Since PR_SET_PDEATHSIG fires when the calling thread exits (not the process), a dedicated thread that lives for the process lifetime is necessary. The mutex/condvar synchronization pattern is standard and follows the existing pal_signal.c pattern.

Summary: ✅ LGTM. All previous review findings have been addressed, including the concurrent-submit race (fixed in 0f92552). The code is correct and well-structured. API shape matches the approved proposal. Both sub-agent models verified the core correctness.

---

Detailed Findings

✅ API approval — Verified

The api-approved label is present on #125838. The PR adds [SupportedOSPlatform("linux")] to the existing approved KillOnParentExit property. The ref assembly change matches the implementation.

✅ Thread safety of double-checked locking — Verified correct (both models agree)

EnsurePDeathSigThread uses C11 _Atomic int with atomic_load_explicit(memory_order_acquire) / atomic_store_explicit(memory_order_release) correctly. The fast path avoids locking, and the slow path rechecks under the mutex. Both models verified this as textbook double-checked locking.

✅ Mutex/condvar pattern — Verified correct (both models agree)

The producer-consumer pattern between ForkAndExecOnPDeathSigThread (caller) and PDeathSigThreadFunc (worker) is sound. Key properties verified:

• No concurrent-submit race: Callers wait for s_pdeathsig_request == NULL before submitting (Phase 1), and wait for their own req.done flag (Phase 2). Stack-allocated req lifetime is protected.
• No lost wakeup: pthread_cond_broadcast wakes all waiters (both the requester waiting for req.done and other callers waiting for the slot).
• The worker thread holds the mutex during ForkAndExecProcessInternal, which enforces true serialization.

✅ prctl placement — Verified correct

One sub-agent flagged that prctl is called "late" (after fd remapping, credentials, chdir). However, this placement is intentionally correct: Linux clears PR_SET_PDEATHSIG on credential changes (setuid/setgid). The prctl call MUST come after setCredentials (lines 883-891) to avoid being silently cleared. Current placement (line 916) is correct.

✅ vfork safety — Verified correct (both models agree)

prctl is a raw syscall, not a library function. It is safe to call after both fork and vfork. The getppid() call is also a syscall and safe in this context.

✅ Orphan check (expectedParentPid) — Verified correct

The pattern of capturing getpid() before fork and checking getppid() != expectedParentPid after prctl is the standard recommended defense against the fork-to-prctl race window. If the parent dies between fork and prctl, the child is reparented (possibly to a subreaper, not just init), and this check correctly catches it. One sub-agent incorrectly flagged this as racy — the check is the standard mitigation documented in the prctl(2) man page.

✅ Error handling — Verified correct

pthread_attr_init, pthread_attr_setdetachstate, and pthread_create return values are all checked with proper cleanup on failure, following the pattern from pal_signal.c:CreateSignalHandlerThread.

✅ Test coverage — Adequate

Tests cover: default value, set to true, UseShellExecute conflict, normal exit, parent graceful exit, parent kill, and parent crash — all with [InlineData] for both KillOnParentExit=true and false, and with/without handle inheritance restriction. Platform gate correctly expanded to TestPlatforms.Windows | TestPlatforms.Linux.

💡 Concurrent KillOnParentExit=true stress test — Follow-up suggestion (advisory, non-blocking)

Both sub-agents noted the lack of a concurrent stress test (multiple threads calling Process.Start with KillOnParentExit=true simultaneously). This would exercise the Phase 1/Phase 2 waiting logic. This is a nice-to-have for a follow-up, not a merge blocker.

[adamsitnik]

@jkotas the PR is ready for the first round of review. Please let me know what you think about this approach.

[adamsitnik]

@jkotas the PR is ready for the first round of review. Please let me know what you think about this approach.

cc @tmds

[jkotas]

Please let me know what you think about this approach.

The approach looks fine to me.

[Copilot]

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

🤖 Copilot Code Review — PR #127112

Note

This review was AI/Copilot-generated using the code-review skill, with sub-agent analysis from Gemini 2.5 Pro and GPT-5.3-Codex.

Holistic Assessment

Motivation: The PR extends ProcessStartInfo.KillOnParentExit to Linux using prctl(PR_SET_PDEATHSIG, SIGKILL). This is part of the approved API in #125838 (api-approved label). The feature addresses a genuine and common need — ensuring child process cleanup on parent exit.

Approach: The dedicated long-lived thread design is the correct approach. Since PR_SET_PDEATHSIG fires when the calling thread exits (not the process), a dedicated thread that lives for the process lifetime is necessary. The mutex/condvar synchronization pattern is standard and follows the existing pal_signal.c pattern.

Summary: ✅ LGTM. All previous review findings have been addressed. The code is correct and well-structured. API shape matches the approved proposal. Both sub-agent models verified the core correctness.

---

Detailed Findings

✅ API approval — Verified

The api-approved label is present on #125838. The approved comment explicitly includes [SupportedOS("linux")] on KillOnParentExit. The ref assembly change matches the approved shape exactly.

✅ Thread safety of double-checked locking — Verified correct (both models agree)

EnsurePDeathSigThread uses C11 _Atomic int with atomic_load_explicit(memory_order_acquire) / atomic_store_explicit(memory_order_release) correctly. The fast path avoids locking, and the slow path rechecks under the mutex. Textbook double-checked locking with correct memory ordering.

✅ Mutex/condvar pattern — Verified correct (both models agree)

• No overwrite race: Callers wait for s_pdeathsig_request == NULL before submitting.
• Per-request done flag: Callers wait on req.done, not the global pointer.
• pthread_cond_broadcast: Correctly wakes all waiters for done_cond.
• pthread_cond_signal: Correctly used for request_cond (single worker).
• Stack variable safety: The caller blocks on pthread_cond_wait until req.done is set. Stack frame is guaranteed live.

✅ vfork safety — Verified correct

prctl() and getppid() are syscalls, safe after vfork. ExitChild uses write and _exit, both async-signal-safe.

✅ Orphan detection — Verified correct

Captures expectedParentPid = getpid() before fork, verifies getppid() != expectedParentPid after prctl. Correctly detects reparenting to subreapers.

✅ Error handling in EnsurePDeathSigThread — Verified correct

All pthread_attr_* and pthread_create return values checked with proper cleanup. Follows pal_signal.c pattern.

✅ Managed code and tests — Verified correct

OperatingSystem.IsLinux() gates the flag. Platform attributes match approved API. Tests cover default, enabled/disabled, graceful/killed/crashed parent exit scenarios.

---

Models contributing: Gemini 2.5 Pro, GPT-5.3-Codex

[adamsitnik]

LGTM and is ready for review (cc @jkotas @tmds @davidfowl )

[jkotas]

Is this needed? Would it be better to pass startInfo.KillOnParentExit to the native shim unconditionally so that we do not need to worry about keeping the native shim logic in sync with what's here?

[jkotas]

Just wondering - are we going to detect the presence of this flag on Android?