Skip to content

Implement SafeProcessHandle APIs for macOS - #124923

Closed
adamsitnik with Copilot wants to merge 25 commits into
copilot/add-new-apis-to-safeprocesshandlefrom
copilot/implement-safeprocesshandle-apis
Closed

Implement SafeProcessHandle APIs for macOS#124923
adamsitnik with Copilot wants to merge 25 commits into
copilot/add-new-apis-to-safeprocesshandlefrom
copilot/implement-safeprocesshandle-apis

Conversation

CopilotAI commented Feb 26, 2026

Copy link
Copy Markdown
Contributor

Description

Implements the macOS-specific managed and native layers for the new SafeProcessHandle APIs (Start, Open, WaitForExit*, Kill, Resume, Signal), using posix_spawn for process creation and kqueue for wait/timeout/cancellation.

Managed changes

  • SafeProcessHandle.Unix.cs: Full implementation of StartCore, OpenCore, WaitForExitCore, TryWaitForExitCore, WaitForExitOrKillOnTimeoutCore, WaitForExitAsyncCore, WaitForExitOrKillOnCancellationAsyncCore, KillCore, ResumeCore, SendSignalCore. Uses NoPidFd = int.MinValue (not -1) since the handle derives from SafeHandleZeroOrMinusOneIsInvalid. All error paths use throw new Win32Exception() (parameterless) to let it fetch the last error automatically. OpenCore correctly detects group leader status by calling getpgid(pid) via native OpenProcess and comparing with pid, rather than hardcoding false. StartCore detects terminal usage via Interop.Sys.IsATty on stdin/stdout/stderr and calls ProcessUtils.ConfigureTerminalForChildProcesses accordingly, with proper s_processStartLock read/write lock usage. _usesTerminal readonly field is stored and used by OnProcessExited helper (called from all WaitForExit* methods) to decrement terminal child count on exit.
  • ProcessUtils.Unix.cs: Extracted CreateEnvp, AllocNullTerminatedArray, FreeArray as internal shared methods, reused by both Process.Unix.cs and SafeProcessHandle.Unix.cs.
  • ProcessUtils.ConfigureTerminalForChildProcesses.Unix.cs (new): Moved terminal configuration logic (ConfigureTerminalForChildProcesses, SetDelayedSigChildConsoleConfigurationHandler, DelayedSigChildConsoleConfiguration, AreChildrenUsingTerminal) from Process to ProcessUtils for shared access by both Process and SafeProcessHandle.
  • ProcessUtils.ConfigureTerminalForChildProcesses.iOS.cs (new): iOS stub with [Conditional("unnecessary")] no-ops, mirroring the original Process.ConfigureTerminalForChildProcesses.iOS.cs.
  • Process.Unix.cs: All callers updated to call ProcessUtils.ConfigureTerminalForChildProcesses, ProcessUtils.SetDelayedSigChildConsoleConfigurationHandler, and ProcessUtils.AreChildrenUsingTerminal directly (no wrapper methods).
  • ProcessWaitState.Unix.cs: Updated to call ProcessUtils.ConfigureTerminalForChildProcesses directly instead of through Process.
  • Process.ConfigureTerminalForChildProcesses.Unix.cs: Removed — all callers now use ProcessUtils directly.
  • Process.ConfigureTerminalForChildProcesses.iOS.cs: Removed — all callers now use ProcessUtils directly.
  • Interop.IsATty.cs: Added SafeFileHandle overload for terminal detection in StartCore.
  • Interop.SpawnProcess.cs: New LibraryImport declarations (EntryPoint = "SystemNative_*") for SpawnProcess, SendSignal, WaitForExitAndReap, TryWaitForExit, TryWaitForExitCancellable, WaitForExitOrKillOnTimeout, TryGetExitCode, OpenProcess. OpenProcess returns the group leader status via an isGroupLeader parameter.
  • Interop.ForkAndExecProcess.cs: Delegates to shared ProcessUtils methods.
  • csproj: Added Interop.Pipe.cs, Interop.SpawnProcess.cs, Interop.IsATty.cs, and new ProcessUtils.ConfigureTerminalForChildProcesses.*.cs files for Unix builds. Removed references to deleted Process.ConfigureTerminalForChildProcesses.*.cs files.

Native changes

  • pal_process.c: macOS implementation using posix_spawn with POSIX_SPAWN_CLOEXEC_DEFAULT, posix_spawn_file_actions_addinherit_np, posix_spawn_file_actions_addchdir_np, POSIX_SPAWN_START_SUSPENDED. The implementation is guarded by #if defined(__APPLE__) && !defined(TARGET_MACCATALYST) && !defined(TARGET_TVOS) to exclude Mac Catalyst and tvOS (iOS is supported). posix_spawn_file_actions_addchdir_np retains HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR_NP feature detection since it is not available on tvOS. Wait functions use kqueue/EVFILT_PROC/NOTE_EXIT (guarded by existing HAVE_KQUEUE) for timeout and cancellation support. Signal mapping reuses SystemNative_GetPlatformSignalNumber and TryConvertSignalCodeToPosixSignal from pal_signal.c instead of duplicating the logic. Environment variable resolution is extracted into a GetEnvVars helper for reuse. OpenProcess uses kill(pid, 0) to check process existence without requiring a child-process relationship, and detects group leader status via getpgid(pid) == pid, returning the result through an out_isGroupLeader output parameter. Both SpawnProcess and OpenProcess set out_pidfd to INT_MIN (not -1) since the managed layer treats -1 as an invalid handle due to SafeHandleZeroOrMinusOneIsInvalid.
  • pal_signal.c / pal_signal.h: Made TryConvertSignalCodeToPosixSignal non-static and added its declaration to the header for reuse by pal_process.c.
  • pal_process_wasi.c: Stubs returning -1 with errno = ENOTSUP.
  • pal_process.h: New function declarations with reordered SpawnProcess arguments (path, argv, envp, working_dir, inherited_handles, inherited_handles_count, stdin/stdout/stderr fds, flags, out params). OpenProcess signature includes out_isGroupLeader parameter.
  • entrypoints.c: New DllImportEntry entries.
  • configure.cmake / pal_config.h.in: Added HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR_NP feature detection (not available on tvOS). No other HAVE_POSIX_SPAWN* defines are added — macOS platform checks (#ifdef __APPLE__) are used instead. Existing HAVE_KQUEUE is reused for sys/event.h inclusion.

Test changes

  • SafeProcessHandleTests.cs: Extended [PlatformSpecific] to TestPlatforms.OSX | TestPlatforms.Windows. All ProcessStartOptions creation uses platform detection (OperatingSystem.IsWindows() ternary). CreateTenSecondSleep returns sleep 10 on Unix. Start_WithNoArguments_Succeeds uses hostname on Windows and pwd on Unix (platform-specific, since pwd is not a standalone executable on Windows). Kill tests assert PosixSignal.SIGKILL and exit code > 128 on Unix. Open_CurrentProcess_Succeeds is annotated [PlatformSpecific(TestPlatforms.Windows)].
  • SafeProcessHandleTests.Unix.cs: macOS-specific tests for Signal(SIGTERM), Signal(SIGINT), invalid signal, and signal-to-exited-process behavior. All long-running tests use CreateTenSecondSleep(). The ProcessId test uses started.Kill() (not the non-owning copy) since the copy handle has no valid pid.

CI changes

  • eng/pipelines/runtime.yml: Added copilot/*.* to CI branch triggers to enable pipeline runs for this branch.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

CopilotAIand others added 3 commits February 26, 2026 21:41
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
…rocess path)
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
CopilotAI changed the title [WIP] Implement SafeProcessHandle APIs for macOSImplement SafeProcessHandle APIs for macOSFeb 26, 2026

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address my feedback.

Moreover, please modify the .yml file that is required to let the CI run for this branch. I am not an expert in this area, but I think it should be enough to modify this file:

pr:
branches:
include:
- main
- release/*.*

and list copilot/*.* after release/*.*

 - release/*.* 
- copilot/*.* 

Comment threadsrc/libraries/Common/src/Interop/Unix/System.Native/Interop.SpawnProcess.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
…on throws, update tests, add CI branch trigger
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Feb 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please address my feedback.

Moreover, please modify the .yml file that is required to let the CI run for this branch. I am not an expert in this area, but I think it should be enough to mo...

All feedback addressed and copilot/*.* added to the CI branch triggers in eng/pipelines/runtime.yml in 28845d4.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements SafeProcessHandle APIs for macOS using posix_spawn for process creation and kqueue for wait/timeout/cancellation operations. The implementation provides parity with the Windows implementation, enabling cross-platform process management through the new SafeProcessHandle APIs.

Changes:

  • Native layer: Added macOS-specific implementations using posix_spawn, kqueue, and waitpid for process creation and management
  • Managed layer: Implemented all core SafeProcessHandle methods (Start, Open, WaitForExit*, Kill, Resume, Signal) with proper error handling
  • Code refactoring: Extracted shared utility methods (CreateEnvp, AllocNullTerminatedArray, FreeArray) from Interop.ForkAndExecProcess.cs to ProcessUtils.Unix.cs for reuse
  • Tests: Extended cross-platform tests to support macOS and added macOS-specific signal tests

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file
FileDescription
src/native/libs/configure.cmakeAdded CMake feature detection for posix_spawn APIs and kqueue on macOS
src/native/libs/Common/pal_config.h.inAdded configuration defines for detected posix_spawn features
src/native/libs/System.Native/pal_process.hDeclared new native API functions for process spawning, signaling, and waiting
src/native/libs/System.Native/pal_process.cImplemented macOS-specific process APIs using posix_spawn and kqueue, including signal mapping
src/native/libs/System.Native/pal_process_wasi.cAdded ENOTSUP stubs for WASI platform
src/native/libs/System.Native/entrypoints.cRegistered new native function entry points
src/libraries/Common/src/Interop/Unix/System.Native/Interop.SpawnProcess.csAdded P/Invoke declarations for new native APIs
src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.csRefactored to use shared ProcessUtils methods
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Unix.csAdded shared utility methods for environment and array marshalling
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.csUpdated to use ProcessUtils.CreateEnvp instead of local implementation
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.csImplemented all SafeProcessHandle core methods for macOS
src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csprojAdded Interop.SpawnProcess.cs and Interop.Pipe.cs to Unix builds
src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.csExtended platform support to include macOS and updated tests for cross-platform compatibility
src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.Unix.csAdded macOS-specific signal tests
src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csprojAdded SafeProcessHandleTests.Unix.cs to Unix test builds
eng/pipelines/runtime.ymlAdded copilot/. branch trigger (intended for development, should be removed)

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadeng/pipelines/runtime.yml

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address all unresolved feedback

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
…ping, fix pwd test
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Feb 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please address all unresolved feedback

All unresolved feedback addressed in cdd13b0: simplified OpenProcess to kill(pid, 0), removed SIGSTOP from signal mappings, and fixed pwd test to be platform-specific.

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please give my suggestion a try.

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
…hecks
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Feb 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please give my suggestion a try.

Done in 4b02d2e — replaced all HAVE_POSIX_SPAWN* feature detection with #ifdef __APPLE__ platform checks and removed the cmake/config defines.

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address my feedback

Also, annotate the Open_CurrentProcess_Succeeds test with [PlatformSpecific(TestPlatforms.Windows)]

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c
Comment threadsrc/native/libs/System.Native/pal_process.c
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
…e via getpgid
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Mar 9, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot Please address my feedback

Fixed the _isGroupLeader correctness concern in 33db32f. SystemNative_OpenProcess now detects whether the process is a group leader via getpgid(pid) == pid and returns the result to managed code.

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot I've asked you to address #124923 (comment) and you seem to have ignored it. Do address this particular feedback.

Comment threadsrc/libraries/Common/src/Interop/Unix/System.Native/Interop.SpawnProcess.cs Outdated
…d is already used
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Mar 9, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot I've asked you to address #124923 (comment) and you seem to have ignored it. Do address this particular feedback.

Applied the parameter rename in af33fd5.


private static SafeProcessHandle OpenCore(int processId)
{
int result = Interop.Sys.OpenProcess(processId, out int pidfd, out int isGroupLeader);

@jkotasjkotasMar 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that processes can change process groups as long as they stay within the same session. Is that correct? It means that this cached value can be stale.

… to ProcessUtils
- Add IsATty(SafeFileHandle) overload to Interop.IsATty.cs
- Move ConfigureTerminalForChildProcesses logic from Process.*.cs to ProcessUtils.*.cs
- Update Process.*.cs to delegate to ProcessUtils
- Add _usesTerminal field to SafeProcessHandle.Unix.cs
- Detect terminal usage in StartCore with IsATty
- Add OnProcessExited helper, called from all WaitForExit* methods
- Update csproj with new files and IsATty reference
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot address my feedback

…te all callers to use ProcessUtils directly
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Mar 9, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot address my feedback

Done in e88f121 — removed both Process.ConfigureTerminalForChildProcesses wrapper files and updated all callers to use ProcessUtils directly.

@adamsitnik

Copy link
Copy Markdown
Member

Closing due the removal of ProcessStartOptions

@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 20, 2026
@jkotas
jkotas deleted the copilot/implement-safeprocesshandle-apis branch May 22, 2026 15:56
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@adamsitnik@tmds@jkotas
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Implement SafeProcessHandle APIs for macOS by Copilot · Pull Request #124923 · dotnet/runtime · GitHub
Skip to content

Implement SafeProcessHandle APIs for macOS - #124923

Closed
adamsitnik with Copilot wants to merge 25 commits into
copilot/add-new-apis-to-safeprocesshandlefrom
copilot/implement-safeprocesshandle-apis
Closed

Implement SafeProcessHandle APIs for macOS#124923
adamsitnik with Copilot wants to merge 25 commits into
copilot/add-new-apis-to-safeprocesshandlefrom
copilot/implement-safeprocesshandle-apis

Conversation

CopilotAI commented Feb 26, 2026

Copy link
Copy Markdown
Contributor

Description

Implements the macOS-specific managed and native layers for the new SafeProcessHandle APIs (Start, Open, WaitForExit*, Kill, Resume, Signal), using posix_spawn for process creation and kqueue for wait/timeout/cancellation.

Managed changes

  • SafeProcessHandle.Unix.cs: Full implementation of StartCore, OpenCore, WaitForExitCore, TryWaitForExitCore, WaitForExitOrKillOnTimeoutCore, WaitForExitAsyncCore, WaitForExitOrKillOnCancellationAsyncCore, KillCore, ResumeCore, SendSignalCore. Uses NoPidFd = int.MinValue (not -1) since the handle derives from SafeHandleZeroOrMinusOneIsInvalid. All error paths use throw new Win32Exception() (parameterless) to let it fetch the last error automatically. OpenCore correctly detects group leader status by calling getpgid(pid) via native OpenProcess and comparing with pid, rather than hardcoding false. StartCore detects terminal usage via Interop.Sys.IsATty on stdin/stdout/stderr and calls ProcessUtils.ConfigureTerminalForChildProcesses accordingly, with proper s_processStartLock read/write lock usage. _usesTerminal readonly field is stored and used by OnProcessExited helper (called from all WaitForExit* methods) to decrement terminal child count on exit.
  • ProcessUtils.Unix.cs: Extracted CreateEnvp, AllocNullTerminatedArray, FreeArray as internal shared methods, reused by both Process.Unix.cs and SafeProcessHandle.Unix.cs.
  • ProcessUtils.ConfigureTerminalForChildProcesses.Unix.cs (new): Moved terminal configuration logic (ConfigureTerminalForChildProcesses, SetDelayedSigChildConsoleConfigurationHandler, DelayedSigChildConsoleConfiguration, AreChildrenUsingTerminal) from Process to ProcessUtils for shared access by both Process and SafeProcessHandle.
  • ProcessUtils.ConfigureTerminalForChildProcesses.iOS.cs (new): iOS stub with [Conditional("unnecessary")] no-ops, mirroring the original Process.ConfigureTerminalForChildProcesses.iOS.cs.
  • Process.Unix.cs: All callers updated to call ProcessUtils.ConfigureTerminalForChildProcesses, ProcessUtils.SetDelayedSigChildConsoleConfigurationHandler, and ProcessUtils.AreChildrenUsingTerminal directly (no wrapper methods).
  • ProcessWaitState.Unix.cs: Updated to call ProcessUtils.ConfigureTerminalForChildProcesses directly instead of through Process.
  • Process.ConfigureTerminalForChildProcesses.Unix.cs: Removed — all callers now use ProcessUtils directly.
  • Process.ConfigureTerminalForChildProcesses.iOS.cs: Removed — all callers now use ProcessUtils directly.
  • Interop.IsATty.cs: Added SafeFileHandle overload for terminal detection in StartCore.
  • Interop.SpawnProcess.cs: New LibraryImport declarations (EntryPoint = "SystemNative_*") for SpawnProcess, SendSignal, WaitForExitAndReap, TryWaitForExit, TryWaitForExitCancellable, WaitForExitOrKillOnTimeout, TryGetExitCode, OpenProcess. OpenProcess returns the group leader status via an isGroupLeader parameter.
  • Interop.ForkAndExecProcess.cs: Delegates to shared ProcessUtils methods.
  • csproj: Added Interop.Pipe.cs, Interop.SpawnProcess.cs, Interop.IsATty.cs, and new ProcessUtils.ConfigureTerminalForChildProcesses.*.cs files for Unix builds. Removed references to deleted Process.ConfigureTerminalForChildProcesses.*.cs files.

Native changes

  • pal_process.c: macOS implementation using posix_spawn with POSIX_SPAWN_CLOEXEC_DEFAULT, posix_spawn_file_actions_addinherit_np, posix_spawn_file_actions_addchdir_np, POSIX_SPAWN_START_SUSPENDED. The implementation is guarded by #if defined(__APPLE__) && !defined(TARGET_MACCATALYST) && !defined(TARGET_TVOS) to exclude Mac Catalyst and tvOS (iOS is supported). posix_spawn_file_actions_addchdir_np retains HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR_NP feature detection since it is not available on tvOS. Wait functions use kqueue/EVFILT_PROC/NOTE_EXIT (guarded by existing HAVE_KQUEUE) for timeout and cancellation support. Signal mapping reuses SystemNative_GetPlatformSignalNumber and TryConvertSignalCodeToPosixSignal from pal_signal.c instead of duplicating the logic. Environment variable resolution is extracted into a GetEnvVars helper for reuse. OpenProcess uses kill(pid, 0) to check process existence without requiring a child-process relationship, and detects group leader status via getpgid(pid) == pid, returning the result through an out_isGroupLeader output parameter. Both SpawnProcess and OpenProcess set out_pidfd to INT_MIN (not -1) since the managed layer treats -1 as an invalid handle due to SafeHandleZeroOrMinusOneIsInvalid.
  • pal_signal.c / pal_signal.h: Made TryConvertSignalCodeToPosixSignal non-static and added its declaration to the header for reuse by pal_process.c.
  • pal_process_wasi.c: Stubs returning -1 with errno = ENOTSUP.
  • pal_process.h: New function declarations with reordered SpawnProcess arguments (path, argv, envp, working_dir, inherited_handles, inherited_handles_count, stdin/stdout/stderr fds, flags, out params). OpenProcess signature includes out_isGroupLeader parameter.
  • entrypoints.c: New DllImportEntry entries.
  • configure.cmake / pal_config.h.in: Added HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR_NP feature detection (not available on tvOS). No other HAVE_POSIX_SPAWN* defines are added — macOS platform checks (#ifdef __APPLE__) are used instead. Existing HAVE_KQUEUE is reused for sys/event.h inclusion.

Test changes

  • SafeProcessHandleTests.cs: Extended [PlatformSpecific] to TestPlatforms.OSX | TestPlatforms.Windows. All ProcessStartOptions creation uses platform detection (OperatingSystem.IsWindows() ternary). CreateTenSecondSleep returns sleep 10 on Unix. Start_WithNoArguments_Succeeds uses hostname on Windows and pwd on Unix (platform-specific, since pwd is not a standalone executable on Windows). Kill tests assert PosixSignal.SIGKILL and exit code > 128 on Unix. Open_CurrentProcess_Succeeds is annotated [PlatformSpecific(TestPlatforms.Windows)].
  • SafeProcessHandleTests.Unix.cs: macOS-specific tests for Signal(SIGTERM), Signal(SIGINT), invalid signal, and signal-to-exited-process behavior. All long-running tests use CreateTenSecondSleep(). The ProcessId test uses started.Kill() (not the non-owning copy) since the copy handle has no valid pid.

CI changes

  • eng/pipelines/runtime.yml: Added copilot/*.* to CI branch triggers to enable pipeline runs for this branch.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

CopilotAIand others added 3 commits February 26, 2026 21:41
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
…rocess path)
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
CopilotAI changed the title [WIP] Implement SafeProcessHandle APIs for macOSImplement SafeProcessHandle APIs for macOSFeb 26, 2026

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address my feedback.

Moreover, please modify the .yml file that is required to let the CI run for this branch. I am not an expert in this area, but I think it should be enough to modify this file:

pr:
branches:
include:
- main
- release/*.*

and list copilot/*.* after release/*.*

 - release/*.* 
- copilot/*.* 

Comment threadsrc/libraries/Common/src/Interop/Unix/System.Native/Interop.SpawnProcess.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
…on throws, update tests, add CI branch trigger
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Feb 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please address my feedback.

Moreover, please modify the .yml file that is required to let the CI run for this branch. I am not an expert in this area, but I think it should be enough to mo...

All feedback addressed and copilot/*.* added to the CI branch triggers in eng/pipelines/runtime.yml in 28845d4.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements SafeProcessHandle APIs for macOS using posix_spawn for process creation and kqueue for wait/timeout/cancellation operations. The implementation provides parity with the Windows implementation, enabling cross-platform process management through the new SafeProcessHandle APIs.

Changes:

  • Native layer: Added macOS-specific implementations using posix_spawn, kqueue, and waitpid for process creation and management
  • Managed layer: Implemented all core SafeProcessHandle methods (Start, Open, WaitForExit*, Kill, Resume, Signal) with proper error handling
  • Code refactoring: Extracted shared utility methods (CreateEnvp, AllocNullTerminatedArray, FreeArray) from Interop.ForkAndExecProcess.cs to ProcessUtils.Unix.cs for reuse
  • Tests: Extended cross-platform tests to support macOS and added macOS-specific signal tests

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file
FileDescription
src/native/libs/configure.cmakeAdded CMake feature detection for posix_spawn APIs and kqueue on macOS
src/native/libs/Common/pal_config.h.inAdded configuration defines for detected posix_spawn features
src/native/libs/System.Native/pal_process.hDeclared new native API functions for process spawning, signaling, and waiting
src/native/libs/System.Native/pal_process.cImplemented macOS-specific process APIs using posix_spawn and kqueue, including signal mapping
src/native/libs/System.Native/pal_process_wasi.cAdded ENOTSUP stubs for WASI platform
src/native/libs/System.Native/entrypoints.cRegistered new native function entry points
src/libraries/Common/src/Interop/Unix/System.Native/Interop.SpawnProcess.csAdded P/Invoke declarations for new native APIs
src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.csRefactored to use shared ProcessUtils methods
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Unix.csAdded shared utility methods for environment and array marshalling
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.csUpdated to use ProcessUtils.CreateEnvp instead of local implementation
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.csImplemented all SafeProcessHandle core methods for macOS
src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csprojAdded Interop.SpawnProcess.cs and Interop.Pipe.cs to Unix builds
src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.csExtended platform support to include macOS and updated tests for cross-platform compatibility
src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.Unix.csAdded macOS-specific signal tests
src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csprojAdded SafeProcessHandleTests.Unix.cs to Unix test builds
eng/pipelines/runtime.ymlAdded copilot/. branch trigger (intended for development, should be removed)

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadeng/pipelines/runtime.yml

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address all unresolved feedback

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
…ping, fix pwd test
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Feb 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please address all unresolved feedback

All unresolved feedback addressed in cdd13b0: simplified OpenProcess to kill(pid, 0), removed SIGSTOP from signal mappings, and fixed pwd test to be platform-specific.

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please give my suggestion a try.

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
…hecks
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Feb 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please give my suggestion a try.

Done in 4b02d2e — replaced all HAVE_POSIX_SPAWN* feature detection with #ifdef __APPLE__ platform checks and removed the cmake/config defines.

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address my feedback

Also, annotate the Open_CurrentProcess_Succeeds test with [PlatformSpecific(TestPlatforms.Windows)]

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c
Comment threadsrc/native/libs/System.Native/pal_process.c
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
…e via getpgid
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Mar 9, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot Please address my feedback

Fixed the _isGroupLeader correctness concern in 33db32f. SystemNative_OpenProcess now detects whether the process is a group leader via getpgid(pid) == pid and returns the result to managed code.

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot I've asked you to address #124923 (comment) and you seem to have ignored it. Do address this particular feedback.

Comment threadsrc/libraries/Common/src/Interop/Unix/System.Native/Interop.SpawnProcess.cs Outdated
…d is already used
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Mar 9, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot I've asked you to address #124923 (comment) and you seem to have ignored it. Do address this particular feedback.

Applied the parameter rename in af33fd5.


private static SafeProcessHandle OpenCore(int processId)
{
int result = Interop.Sys.OpenProcess(processId, out int pidfd, out int isGroupLeader);

@jkotasjkotasMar 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that processes can change process groups as long as they stay within the same session. Is that correct? It means that this cached value can be stale.

… to ProcessUtils
- Add IsATty(SafeFileHandle) overload to Interop.IsATty.cs
- Move ConfigureTerminalForChildProcesses logic from Process.*.cs to ProcessUtils.*.cs
- Update Process.*.cs to delegate to ProcessUtils
- Add _usesTerminal field to SafeProcessHandle.Unix.cs
- Detect terminal usage in StartCore with IsATty
- Add OnProcessExited helper, called from all WaitForExit* methods
- Update csproj with new files and IsATty reference
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot address my feedback

…te all callers to use ProcessUtils directly
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Mar 9, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot address my feedback

Done in e88f121 — removed both Process.ConfigureTerminalForChildProcesses wrapper files and updated all callers to use ProcessUtils directly.

@adamsitnik

Copy link
Copy Markdown
Member

Closing due the removal of ProcessStartOptions

@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 20, 2026
@jkotas
jkotas deleted the copilot/implement-safeprocesshandle-apis branch May 22, 2026 15:56
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@adamsitnik@tmds@jkotas
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Implement SafeProcessHandle APIs for macOS by Copilot · Pull Request #124923 · dotnet/runtime · GitHub
Skip to content

Implement SafeProcessHandle APIs for macOS - #124923

Closed
adamsitnik with Copilot wants to merge 25 commits into
copilot/add-new-apis-to-safeprocesshandlefrom
copilot/implement-safeprocesshandle-apis
Closed

Implement SafeProcessHandle APIs for macOS#124923
adamsitnik with Copilot wants to merge 25 commits into
copilot/add-new-apis-to-safeprocesshandlefrom
copilot/implement-safeprocesshandle-apis

Conversation

CopilotAI commented Feb 26, 2026

Copy link
Copy Markdown
Contributor

Description

Implements the macOS-specific managed and native layers for the new SafeProcessHandle APIs (Start, Open, WaitForExit*, Kill, Resume, Signal), using posix_spawn for process creation and kqueue for wait/timeout/cancellation.

Managed changes

  • SafeProcessHandle.Unix.cs: Full implementation of StartCore, OpenCore, WaitForExitCore, TryWaitForExitCore, WaitForExitOrKillOnTimeoutCore, WaitForExitAsyncCore, WaitForExitOrKillOnCancellationAsyncCore, KillCore, ResumeCore, SendSignalCore. Uses NoPidFd = int.MinValue (not -1) since the handle derives from SafeHandleZeroOrMinusOneIsInvalid. All error paths use throw new Win32Exception() (parameterless) to let it fetch the last error automatically. OpenCore correctly detects group leader status by calling getpgid(pid) via native OpenProcess and comparing with pid, rather than hardcoding false. StartCore detects terminal usage via Interop.Sys.IsATty on stdin/stdout/stderr and calls ProcessUtils.ConfigureTerminalForChildProcesses accordingly, with proper s_processStartLock read/write lock usage. _usesTerminal readonly field is stored and used by OnProcessExited helper (called from all WaitForExit* methods) to decrement terminal child count on exit.
  • ProcessUtils.Unix.cs: Extracted CreateEnvp, AllocNullTerminatedArray, FreeArray as internal shared methods, reused by both Process.Unix.cs and SafeProcessHandle.Unix.cs.
  • ProcessUtils.ConfigureTerminalForChildProcesses.Unix.cs (new): Moved terminal configuration logic (ConfigureTerminalForChildProcesses, SetDelayedSigChildConsoleConfigurationHandler, DelayedSigChildConsoleConfiguration, AreChildrenUsingTerminal) from Process to ProcessUtils for shared access by both Process and SafeProcessHandle.
  • ProcessUtils.ConfigureTerminalForChildProcesses.iOS.cs (new): iOS stub with [Conditional("unnecessary")] no-ops, mirroring the original Process.ConfigureTerminalForChildProcesses.iOS.cs.
  • Process.Unix.cs: All callers updated to call ProcessUtils.ConfigureTerminalForChildProcesses, ProcessUtils.SetDelayedSigChildConsoleConfigurationHandler, and ProcessUtils.AreChildrenUsingTerminal directly (no wrapper methods).
  • ProcessWaitState.Unix.cs: Updated to call ProcessUtils.ConfigureTerminalForChildProcesses directly instead of through Process.
  • Process.ConfigureTerminalForChildProcesses.Unix.cs: Removed — all callers now use ProcessUtils directly.
  • Process.ConfigureTerminalForChildProcesses.iOS.cs: Removed — all callers now use ProcessUtils directly.
  • Interop.IsATty.cs: Added SafeFileHandle overload for terminal detection in StartCore.
  • Interop.SpawnProcess.cs: New LibraryImport declarations (EntryPoint = "SystemNative_*") for SpawnProcess, SendSignal, WaitForExitAndReap, TryWaitForExit, TryWaitForExitCancellable, WaitForExitOrKillOnTimeout, TryGetExitCode, OpenProcess. OpenProcess returns the group leader status via an isGroupLeader parameter.
  • Interop.ForkAndExecProcess.cs: Delegates to shared ProcessUtils methods.
  • csproj: Added Interop.Pipe.cs, Interop.SpawnProcess.cs, Interop.IsATty.cs, and new ProcessUtils.ConfigureTerminalForChildProcesses.*.cs files for Unix builds. Removed references to deleted Process.ConfigureTerminalForChildProcesses.*.cs files.

Native changes

  • pal_process.c: macOS implementation using posix_spawn with POSIX_SPAWN_CLOEXEC_DEFAULT, posix_spawn_file_actions_addinherit_np, posix_spawn_file_actions_addchdir_np, POSIX_SPAWN_START_SUSPENDED. The implementation is guarded by #if defined(__APPLE__) && !defined(TARGET_MACCATALYST) && !defined(TARGET_TVOS) to exclude Mac Catalyst and tvOS (iOS is supported). posix_spawn_file_actions_addchdir_np retains HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR_NP feature detection since it is not available on tvOS. Wait functions use kqueue/EVFILT_PROC/NOTE_EXIT (guarded by existing HAVE_KQUEUE) for timeout and cancellation support. Signal mapping reuses SystemNative_GetPlatformSignalNumber and TryConvertSignalCodeToPosixSignal from pal_signal.c instead of duplicating the logic. Environment variable resolution is extracted into a GetEnvVars helper for reuse. OpenProcess uses kill(pid, 0) to check process existence without requiring a child-process relationship, and detects group leader status via getpgid(pid) == pid, returning the result through an out_isGroupLeader output parameter. Both SpawnProcess and OpenProcess set out_pidfd to INT_MIN (not -1) since the managed layer treats -1 as an invalid handle due to SafeHandleZeroOrMinusOneIsInvalid.
  • pal_signal.c / pal_signal.h: Made TryConvertSignalCodeToPosixSignal non-static and added its declaration to the header for reuse by pal_process.c.
  • pal_process_wasi.c: Stubs returning -1 with errno = ENOTSUP.
  • pal_process.h: New function declarations with reordered SpawnProcess arguments (path, argv, envp, working_dir, inherited_handles, inherited_handles_count, stdin/stdout/stderr fds, flags, out params). OpenProcess signature includes out_isGroupLeader parameter.
  • entrypoints.c: New DllImportEntry entries.
  • configure.cmake / pal_config.h.in: Added HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR_NP feature detection (not available on tvOS). No other HAVE_POSIX_SPAWN* defines are added — macOS platform checks (#ifdef __APPLE__) are used instead. Existing HAVE_KQUEUE is reused for sys/event.h inclusion.

Test changes

  • SafeProcessHandleTests.cs: Extended [PlatformSpecific] to TestPlatforms.OSX | TestPlatforms.Windows. All ProcessStartOptions creation uses platform detection (OperatingSystem.IsWindows() ternary). CreateTenSecondSleep returns sleep 10 on Unix. Start_WithNoArguments_Succeeds uses hostname on Windows and pwd on Unix (platform-specific, since pwd is not a standalone executable on Windows). Kill tests assert PosixSignal.SIGKILL and exit code > 128 on Unix. Open_CurrentProcess_Succeeds is annotated [PlatformSpecific(TestPlatforms.Windows)].
  • SafeProcessHandleTests.Unix.cs: macOS-specific tests for Signal(SIGTERM), Signal(SIGINT), invalid signal, and signal-to-exited-process behavior. All long-running tests use CreateTenSecondSleep(). The ProcessId test uses started.Kill() (not the non-owning copy) since the copy handle has no valid pid.

CI changes

  • eng/pipelines/runtime.yml: Added copilot/*.* to CI branch triggers to enable pipeline runs for this branch.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

CopilotAIand others added 3 commits February 26, 2026 21:41
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
…rocess path)
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
CopilotAI changed the title [WIP] Implement SafeProcessHandle APIs for macOSImplement SafeProcessHandle APIs for macOSFeb 26, 2026

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address my feedback.

Moreover, please modify the .yml file that is required to let the CI run for this branch. I am not an expert in this area, but I think it should be enough to modify this file:

pr:
branches:
include:
- main
- release/*.*

and list copilot/*.* after release/*.*

 - release/*.* 
- copilot/*.* 

Comment threadsrc/libraries/Common/src/Interop/Unix/System.Native/Interop.SpawnProcess.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
…on throws, update tests, add CI branch trigger
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Feb 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please address my feedback.

Moreover, please modify the .yml file that is required to let the CI run for this branch. I am not an expert in this area, but I think it should be enough to mo...

All feedback addressed and copilot/*.* added to the CI branch triggers in eng/pipelines/runtime.yml in 28845d4.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements SafeProcessHandle APIs for macOS using posix_spawn for process creation and kqueue for wait/timeout/cancellation operations. The implementation provides parity with the Windows implementation, enabling cross-platform process management through the new SafeProcessHandle APIs.

Changes:

  • Native layer: Added macOS-specific implementations using posix_spawn, kqueue, and waitpid for process creation and management
  • Managed layer: Implemented all core SafeProcessHandle methods (Start, Open, WaitForExit*, Kill, Resume, Signal) with proper error handling
  • Code refactoring: Extracted shared utility methods (CreateEnvp, AllocNullTerminatedArray, FreeArray) from Interop.ForkAndExecProcess.cs to ProcessUtils.Unix.cs for reuse
  • Tests: Extended cross-platform tests to support macOS and added macOS-specific signal tests

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file
FileDescription
src/native/libs/configure.cmakeAdded CMake feature detection for posix_spawn APIs and kqueue on macOS
src/native/libs/Common/pal_config.h.inAdded configuration defines for detected posix_spawn features
src/native/libs/System.Native/pal_process.hDeclared new native API functions for process spawning, signaling, and waiting
src/native/libs/System.Native/pal_process.cImplemented macOS-specific process APIs using posix_spawn and kqueue, including signal mapping
src/native/libs/System.Native/pal_process_wasi.cAdded ENOTSUP stubs for WASI platform
src/native/libs/System.Native/entrypoints.cRegistered new native function entry points
src/libraries/Common/src/Interop/Unix/System.Native/Interop.SpawnProcess.csAdded P/Invoke declarations for new native APIs
src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.csRefactored to use shared ProcessUtils methods
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Unix.csAdded shared utility methods for environment and array marshalling
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.csUpdated to use ProcessUtils.CreateEnvp instead of local implementation
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.csImplemented all SafeProcessHandle core methods for macOS
src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csprojAdded Interop.SpawnProcess.cs and Interop.Pipe.cs to Unix builds
src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.csExtended platform support to include macOS and updated tests for cross-platform compatibility
src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.Unix.csAdded macOS-specific signal tests
src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csprojAdded SafeProcessHandleTests.Unix.cs to Unix test builds
eng/pipelines/runtime.ymlAdded copilot/. branch trigger (intended for development, should be removed)

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadeng/pipelines/runtime.yml

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address all unresolved feedback

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
…ping, fix pwd test
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Feb 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please address all unresolved feedback

All unresolved feedback addressed in cdd13b0: simplified OpenProcess to kill(pid, 0), removed SIGSTOP from signal mappings, and fixed pwd test to be platform-specific.

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please give my suggestion a try.

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
…hecks
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Feb 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please give my suggestion a try.

Done in 4b02d2e — replaced all HAVE_POSIX_SPAWN* feature detection with #ifdef __APPLE__ platform checks and removed the cmake/config defines.

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address my feedback

Also, annotate the Open_CurrentProcess_Succeeds test with [PlatformSpecific(TestPlatforms.Windows)]

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c
Comment threadsrc/native/libs/System.Native/pal_process.c
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
…e via getpgid
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Mar 9, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot Please address my feedback

Fixed the _isGroupLeader correctness concern in 33db32f. SystemNative_OpenProcess now detects whether the process is a group leader via getpgid(pid) == pid and returns the result to managed code.

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot I've asked you to address #124923 (comment) and you seem to have ignored it. Do address this particular feedback.

Comment threadsrc/libraries/Common/src/Interop/Unix/System.Native/Interop.SpawnProcess.cs Outdated
…d is already used
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Mar 9, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot I've asked you to address #124923 (comment) and you seem to have ignored it. Do address this particular feedback.

Applied the parameter rename in af33fd5.


private static SafeProcessHandle OpenCore(int processId)
{
int result = Interop.Sys.OpenProcess(processId, out int pidfd, out int isGroupLeader);

@jkotasjkotasMar 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that processes can change process groups as long as they stay within the same session. Is that correct? It means that this cached value can be stale.

… to ProcessUtils
- Add IsATty(SafeFileHandle) overload to Interop.IsATty.cs
- Move ConfigureTerminalForChildProcesses logic from Process.*.cs to ProcessUtils.*.cs
- Update Process.*.cs to delegate to ProcessUtils
- Add _usesTerminal field to SafeProcessHandle.Unix.cs
- Detect terminal usage in StartCore with IsATty
- Add OnProcessExited helper, called from all WaitForExit* methods
- Update csproj with new files and IsATty reference
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot address my feedback

…te all callers to use ProcessUtils directly
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Mar 9, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot address my feedback

Done in e88f121 — removed both Process.ConfigureTerminalForChildProcesses wrapper files and updated all callers to use ProcessUtils directly.

@adamsitnik

Copy link
Copy Markdown
Member

Closing due the removal of ProcessStartOptions

@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 20, 2026
@jkotas
jkotas deleted the copilot/implement-safeprocesshandle-apis branch May 22, 2026 15:56
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@adamsitnik@tmds@jkotas
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Implement SafeProcessHandle APIs for macOS by Copilot · Pull Request #124923 · dotnet/runtime · GitHub
Skip to content

Implement SafeProcessHandle APIs for macOS - #124923

Closed
adamsitnik with Copilot wants to merge 25 commits into
copilot/add-new-apis-to-safeprocesshandlefrom
copilot/implement-safeprocesshandle-apis
Closed

Implement SafeProcessHandle APIs for macOS#124923
adamsitnik with Copilot wants to merge 25 commits into
copilot/add-new-apis-to-safeprocesshandlefrom
copilot/implement-safeprocesshandle-apis

Conversation

CopilotAI commented Feb 26, 2026

Copy link
Copy Markdown
Contributor

Description

Implements the macOS-specific managed and native layers for the new SafeProcessHandle APIs (Start, Open, WaitForExit*, Kill, Resume, Signal), using posix_spawn for process creation and kqueue for wait/timeout/cancellation.

Managed changes

  • SafeProcessHandle.Unix.cs: Full implementation of StartCore, OpenCore, WaitForExitCore, TryWaitForExitCore, WaitForExitOrKillOnTimeoutCore, WaitForExitAsyncCore, WaitForExitOrKillOnCancellationAsyncCore, KillCore, ResumeCore, SendSignalCore. Uses NoPidFd = int.MinValue (not -1) since the handle derives from SafeHandleZeroOrMinusOneIsInvalid. All error paths use throw new Win32Exception() (parameterless) to let it fetch the last error automatically. OpenCore correctly detects group leader status by calling getpgid(pid) via native OpenProcess and comparing with pid, rather than hardcoding false. StartCore detects terminal usage via Interop.Sys.IsATty on stdin/stdout/stderr and calls ProcessUtils.ConfigureTerminalForChildProcesses accordingly, with proper s_processStartLock read/write lock usage. _usesTerminal readonly field is stored and used by OnProcessExited helper (called from all WaitForExit* methods) to decrement terminal child count on exit.
  • ProcessUtils.Unix.cs: Extracted CreateEnvp, AllocNullTerminatedArray, FreeArray as internal shared methods, reused by both Process.Unix.cs and SafeProcessHandle.Unix.cs.
  • ProcessUtils.ConfigureTerminalForChildProcesses.Unix.cs (new): Moved terminal configuration logic (ConfigureTerminalForChildProcesses, SetDelayedSigChildConsoleConfigurationHandler, DelayedSigChildConsoleConfiguration, AreChildrenUsingTerminal) from Process to ProcessUtils for shared access by both Process and SafeProcessHandle.
  • ProcessUtils.ConfigureTerminalForChildProcesses.iOS.cs (new): iOS stub with [Conditional("unnecessary")] no-ops, mirroring the original Process.ConfigureTerminalForChildProcesses.iOS.cs.
  • Process.Unix.cs: All callers updated to call ProcessUtils.ConfigureTerminalForChildProcesses, ProcessUtils.SetDelayedSigChildConsoleConfigurationHandler, and ProcessUtils.AreChildrenUsingTerminal directly (no wrapper methods).
  • ProcessWaitState.Unix.cs: Updated to call ProcessUtils.ConfigureTerminalForChildProcesses directly instead of through Process.
  • Process.ConfigureTerminalForChildProcesses.Unix.cs: Removed — all callers now use ProcessUtils directly.
  • Process.ConfigureTerminalForChildProcesses.iOS.cs: Removed — all callers now use ProcessUtils directly.
  • Interop.IsATty.cs: Added SafeFileHandle overload for terminal detection in StartCore.
  • Interop.SpawnProcess.cs: New LibraryImport declarations (EntryPoint = "SystemNative_*") for SpawnProcess, SendSignal, WaitForExitAndReap, TryWaitForExit, TryWaitForExitCancellable, WaitForExitOrKillOnTimeout, TryGetExitCode, OpenProcess. OpenProcess returns the group leader status via an isGroupLeader parameter.
  • Interop.ForkAndExecProcess.cs: Delegates to shared ProcessUtils methods.
  • csproj: Added Interop.Pipe.cs, Interop.SpawnProcess.cs, Interop.IsATty.cs, and new ProcessUtils.ConfigureTerminalForChildProcesses.*.cs files for Unix builds. Removed references to deleted Process.ConfigureTerminalForChildProcesses.*.cs files.

Native changes

  • pal_process.c: macOS implementation using posix_spawn with POSIX_SPAWN_CLOEXEC_DEFAULT, posix_spawn_file_actions_addinherit_np, posix_spawn_file_actions_addchdir_np, POSIX_SPAWN_START_SUSPENDED. The implementation is guarded by #if defined(__APPLE__) && !defined(TARGET_MACCATALYST) && !defined(TARGET_TVOS) to exclude Mac Catalyst and tvOS (iOS is supported). posix_spawn_file_actions_addchdir_np retains HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR_NP feature detection since it is not available on tvOS. Wait functions use kqueue/EVFILT_PROC/NOTE_EXIT (guarded by existing HAVE_KQUEUE) for timeout and cancellation support. Signal mapping reuses SystemNative_GetPlatformSignalNumber and TryConvertSignalCodeToPosixSignal from pal_signal.c instead of duplicating the logic. Environment variable resolution is extracted into a GetEnvVars helper for reuse. OpenProcess uses kill(pid, 0) to check process existence without requiring a child-process relationship, and detects group leader status via getpgid(pid) == pid, returning the result through an out_isGroupLeader output parameter. Both SpawnProcess and OpenProcess set out_pidfd to INT_MIN (not -1) since the managed layer treats -1 as an invalid handle due to SafeHandleZeroOrMinusOneIsInvalid.
  • pal_signal.c / pal_signal.h: Made TryConvertSignalCodeToPosixSignal non-static and added its declaration to the header for reuse by pal_process.c.
  • pal_process_wasi.c: Stubs returning -1 with errno = ENOTSUP.
  • pal_process.h: New function declarations with reordered SpawnProcess arguments (path, argv, envp, working_dir, inherited_handles, inherited_handles_count, stdin/stdout/stderr fds, flags, out params). OpenProcess signature includes out_isGroupLeader parameter.
  • entrypoints.c: New DllImportEntry entries.
  • configure.cmake / pal_config.h.in: Added HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR_NP feature detection (not available on tvOS). No other HAVE_POSIX_SPAWN* defines are added — macOS platform checks (#ifdef __APPLE__) are used instead. Existing HAVE_KQUEUE is reused for sys/event.h inclusion.

Test changes

  • SafeProcessHandleTests.cs: Extended [PlatformSpecific] to TestPlatforms.OSX | TestPlatforms.Windows. All ProcessStartOptions creation uses platform detection (OperatingSystem.IsWindows() ternary). CreateTenSecondSleep returns sleep 10 on Unix. Start_WithNoArguments_Succeeds uses hostname on Windows and pwd on Unix (platform-specific, since pwd is not a standalone executable on Windows). Kill tests assert PosixSignal.SIGKILL and exit code > 128 on Unix. Open_CurrentProcess_Succeeds is annotated [PlatformSpecific(TestPlatforms.Windows)].
  • SafeProcessHandleTests.Unix.cs: macOS-specific tests for Signal(SIGTERM), Signal(SIGINT), invalid signal, and signal-to-exited-process behavior. All long-running tests use CreateTenSecondSleep(). The ProcessId test uses started.Kill() (not the non-owning copy) since the copy handle has no valid pid.

CI changes

  • eng/pipelines/runtime.yml: Added copilot/*.* to CI branch triggers to enable pipeline runs for this branch.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

CopilotAIand others added 3 commits February 26, 2026 21:41
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
…rocess path)
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
CopilotAI changed the title [WIP] Implement SafeProcessHandle APIs for macOSImplement SafeProcessHandle APIs for macOSFeb 26, 2026

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address my feedback.

Moreover, please modify the .yml file that is required to let the CI run for this branch. I am not an expert in this area, but I think it should be enough to modify this file:

pr:
branches:
include:
- main
- release/*.*

and list copilot/*.* after release/*.*

 - release/*.* 
- copilot/*.* 

Comment threadsrc/libraries/Common/src/Interop/Unix/System.Native/Interop.SpawnProcess.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
…on throws, update tests, add CI branch trigger
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Feb 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please address my feedback.

Moreover, please modify the .yml file that is required to let the CI run for this branch. I am not an expert in this area, but I think it should be enough to mo...

All feedback addressed and copilot/*.* added to the CI branch triggers in eng/pipelines/runtime.yml in 28845d4.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements SafeProcessHandle APIs for macOS using posix_spawn for process creation and kqueue for wait/timeout/cancellation operations. The implementation provides parity with the Windows implementation, enabling cross-platform process management through the new SafeProcessHandle APIs.

Changes:

  • Native layer: Added macOS-specific implementations using posix_spawn, kqueue, and waitpid for process creation and management
  • Managed layer: Implemented all core SafeProcessHandle methods (Start, Open, WaitForExit*, Kill, Resume, Signal) with proper error handling
  • Code refactoring: Extracted shared utility methods (CreateEnvp, AllocNullTerminatedArray, FreeArray) from Interop.ForkAndExecProcess.cs to ProcessUtils.Unix.cs for reuse
  • Tests: Extended cross-platform tests to support macOS and added macOS-specific signal tests

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file
FileDescription
src/native/libs/configure.cmakeAdded CMake feature detection for posix_spawn APIs and kqueue on macOS
src/native/libs/Common/pal_config.h.inAdded configuration defines for detected posix_spawn features
src/native/libs/System.Native/pal_process.hDeclared new native API functions for process spawning, signaling, and waiting
src/native/libs/System.Native/pal_process.cImplemented macOS-specific process APIs using posix_spawn and kqueue, including signal mapping
src/native/libs/System.Native/pal_process_wasi.cAdded ENOTSUP stubs for WASI platform
src/native/libs/System.Native/entrypoints.cRegistered new native function entry points
src/libraries/Common/src/Interop/Unix/System.Native/Interop.SpawnProcess.csAdded P/Invoke declarations for new native APIs
src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.csRefactored to use shared ProcessUtils methods
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Unix.csAdded shared utility methods for environment and array marshalling
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.csUpdated to use ProcessUtils.CreateEnvp instead of local implementation
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.csImplemented all SafeProcessHandle core methods for macOS
src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csprojAdded Interop.SpawnProcess.cs and Interop.Pipe.cs to Unix builds
src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.csExtended platform support to include macOS and updated tests for cross-platform compatibility
src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.Unix.csAdded macOS-specific signal tests
src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csprojAdded SafeProcessHandleTests.Unix.cs to Unix test builds
eng/pipelines/runtime.ymlAdded copilot/. branch trigger (intended for development, should be removed)

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadeng/pipelines/runtime.yml

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address all unresolved feedback

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
…ping, fix pwd test
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Feb 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please address all unresolved feedback

All unresolved feedback addressed in cdd13b0: simplified OpenProcess to kill(pid, 0), removed SIGSTOP from signal mappings, and fixed pwd test to be platform-specific.

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please give my suggestion a try.

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
…hecks
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Feb 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please give my suggestion a try.

Done in 4b02d2e — replaced all HAVE_POSIX_SPAWN* feature detection with #ifdef __APPLE__ platform checks and removed the cmake/config defines.

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address my feedback

Also, annotate the Open_CurrentProcess_Succeeds test with [PlatformSpecific(TestPlatforms.Windows)]

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c
Comment threadsrc/native/libs/System.Native/pal_process.c
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
…e via getpgid
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Mar 9, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot Please address my feedback

Fixed the _isGroupLeader correctness concern in 33db32f. SystemNative_OpenProcess now detects whether the process is a group leader via getpgid(pid) == pid and returns the result to managed code.

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot I've asked you to address #124923 (comment) and you seem to have ignored it. Do address this particular feedback.

Comment threadsrc/libraries/Common/src/Interop/Unix/System.Native/Interop.SpawnProcess.cs Outdated
…d is already used
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Mar 9, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot I've asked you to address #124923 (comment) and you seem to have ignored it. Do address this particular feedback.

Applied the parameter rename in af33fd5.


private static SafeProcessHandle OpenCore(int processId)
{
int result = Interop.Sys.OpenProcess(processId, out int pidfd, out int isGroupLeader);

@jkotasjkotasMar 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that processes can change process groups as long as they stay within the same session. Is that correct? It means that this cached value can be stale.

… to ProcessUtils
- Add IsATty(SafeFileHandle) overload to Interop.IsATty.cs
- Move ConfigureTerminalForChildProcesses logic from Process.*.cs to ProcessUtils.*.cs
- Update Process.*.cs to delegate to ProcessUtils
- Add _usesTerminal field to SafeProcessHandle.Unix.cs
- Detect terminal usage in StartCore with IsATty
- Add OnProcessExited helper, called from all WaitForExit* methods
- Update csproj with new files and IsATty reference
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot address my feedback

…te all callers to use ProcessUtils directly
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Mar 9, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot address my feedback

Done in e88f121 — removed both Process.ConfigureTerminalForChildProcesses wrapper files and updated all callers to use ProcessUtils directly.

@adamsitnik

Copy link
Copy Markdown
Member

Closing due the removal of ProcessStartOptions

@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 20, 2026
@jkotas
jkotas deleted the copilot/implement-safeprocesshandle-apis branch May 22, 2026 15:56
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@adamsitnik@tmds@jkotas
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' Implement SafeProcessHandle APIs for macOS by Copilot · Pull Request #124923 · dotnet/runtime · GitHub
Skip to content

Implement SafeProcessHandle APIs for macOS - #124923

Closed
adamsitnik with Copilot wants to merge 25 commits into
copilot/add-new-apis-to-safeprocesshandlefrom
copilot/implement-safeprocesshandle-apis
Closed

Implement SafeProcessHandle APIs for macOS#124923
adamsitnik with Copilot wants to merge 25 commits into
copilot/add-new-apis-to-safeprocesshandlefrom
copilot/implement-safeprocesshandle-apis

Conversation

CopilotAI commented Feb 26, 2026

Copy link
Copy Markdown
Contributor

Description

Implements the macOS-specific managed and native layers for the new SafeProcessHandle APIs (Start, Open, WaitForExit*, Kill, Resume, Signal), using posix_spawn for process creation and kqueue for wait/timeout/cancellation.

Managed changes

  • SafeProcessHandle.Unix.cs: Full implementation of StartCore, OpenCore, WaitForExitCore, TryWaitForExitCore, WaitForExitOrKillOnTimeoutCore, WaitForExitAsyncCore, WaitForExitOrKillOnCancellationAsyncCore, KillCore, ResumeCore, SendSignalCore. Uses NoPidFd = int.MinValue (not -1) since the handle derives from SafeHandleZeroOrMinusOneIsInvalid. All error paths use throw new Win32Exception() (parameterless) to let it fetch the last error automatically. OpenCore correctly detects group leader status by calling getpgid(pid) via native OpenProcess and comparing with pid, rather than hardcoding false. StartCore detects terminal usage via Interop.Sys.IsATty on stdin/stdout/stderr and calls ProcessUtils.ConfigureTerminalForChildProcesses accordingly, with proper s_processStartLock read/write lock usage. _usesTerminal readonly field is stored and used by OnProcessExited helper (called from all WaitForExit* methods) to decrement terminal child count on exit.
  • ProcessUtils.Unix.cs: Extracted CreateEnvp, AllocNullTerminatedArray, FreeArray as internal shared methods, reused by both Process.Unix.cs and SafeProcessHandle.Unix.cs.
  • ProcessUtils.ConfigureTerminalForChildProcesses.Unix.cs (new): Moved terminal configuration logic (ConfigureTerminalForChildProcesses, SetDelayedSigChildConsoleConfigurationHandler, DelayedSigChildConsoleConfiguration, AreChildrenUsingTerminal) from Process to ProcessUtils for shared access by both Process and SafeProcessHandle.
  • ProcessUtils.ConfigureTerminalForChildProcesses.iOS.cs (new): iOS stub with [Conditional("unnecessary")] no-ops, mirroring the original Process.ConfigureTerminalForChildProcesses.iOS.cs.
  • Process.Unix.cs: All callers updated to call ProcessUtils.ConfigureTerminalForChildProcesses, ProcessUtils.SetDelayedSigChildConsoleConfigurationHandler, and ProcessUtils.AreChildrenUsingTerminal directly (no wrapper methods).
  • ProcessWaitState.Unix.cs: Updated to call ProcessUtils.ConfigureTerminalForChildProcesses directly instead of through Process.
  • Process.ConfigureTerminalForChildProcesses.Unix.cs: Removed — all callers now use ProcessUtils directly.
  • Process.ConfigureTerminalForChildProcesses.iOS.cs: Removed — all callers now use ProcessUtils directly.
  • Interop.IsATty.cs: Added SafeFileHandle overload for terminal detection in StartCore.
  • Interop.SpawnProcess.cs: New LibraryImport declarations (EntryPoint = "SystemNative_*") for SpawnProcess, SendSignal, WaitForExitAndReap, TryWaitForExit, TryWaitForExitCancellable, WaitForExitOrKillOnTimeout, TryGetExitCode, OpenProcess. OpenProcess returns the group leader status via an isGroupLeader parameter.
  • Interop.ForkAndExecProcess.cs: Delegates to shared ProcessUtils methods.
  • csproj: Added Interop.Pipe.cs, Interop.SpawnProcess.cs, Interop.IsATty.cs, and new ProcessUtils.ConfigureTerminalForChildProcesses.*.cs files for Unix builds. Removed references to deleted Process.ConfigureTerminalForChildProcesses.*.cs files.

Native changes

  • pal_process.c: macOS implementation using posix_spawn with POSIX_SPAWN_CLOEXEC_DEFAULT, posix_spawn_file_actions_addinherit_np, posix_spawn_file_actions_addchdir_np, POSIX_SPAWN_START_SUSPENDED. The implementation is guarded by #if defined(__APPLE__) && !defined(TARGET_MACCATALYST) && !defined(TARGET_TVOS) to exclude Mac Catalyst and tvOS (iOS is supported). posix_spawn_file_actions_addchdir_np retains HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR_NP feature detection since it is not available on tvOS. Wait functions use kqueue/EVFILT_PROC/NOTE_EXIT (guarded by existing HAVE_KQUEUE) for timeout and cancellation support. Signal mapping reuses SystemNative_GetPlatformSignalNumber and TryConvertSignalCodeToPosixSignal from pal_signal.c instead of duplicating the logic. Environment variable resolution is extracted into a GetEnvVars helper for reuse. OpenProcess uses kill(pid, 0) to check process existence without requiring a child-process relationship, and detects group leader status via getpgid(pid) == pid, returning the result through an out_isGroupLeader output parameter. Both SpawnProcess and OpenProcess set out_pidfd to INT_MIN (not -1) since the managed layer treats -1 as an invalid handle due to SafeHandleZeroOrMinusOneIsInvalid.
  • pal_signal.c / pal_signal.h: Made TryConvertSignalCodeToPosixSignal non-static and added its declaration to the header for reuse by pal_process.c.
  • pal_process_wasi.c: Stubs returning -1 with errno = ENOTSUP.
  • pal_process.h: New function declarations with reordered SpawnProcess arguments (path, argv, envp, working_dir, inherited_handles, inherited_handles_count, stdin/stdout/stderr fds, flags, out params). OpenProcess signature includes out_isGroupLeader parameter.
  • entrypoints.c: New DllImportEntry entries.
  • configure.cmake / pal_config.h.in: Added HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR_NP feature detection (not available on tvOS). No other HAVE_POSIX_SPAWN* defines are added — macOS platform checks (#ifdef __APPLE__) are used instead. Existing HAVE_KQUEUE is reused for sys/event.h inclusion.

Test changes

  • SafeProcessHandleTests.cs: Extended [PlatformSpecific] to TestPlatforms.OSX | TestPlatforms.Windows. All ProcessStartOptions creation uses platform detection (OperatingSystem.IsWindows() ternary). CreateTenSecondSleep returns sleep 10 on Unix. Start_WithNoArguments_Succeeds uses hostname on Windows and pwd on Unix (platform-specific, since pwd is not a standalone executable on Windows). Kill tests assert PosixSignal.SIGKILL and exit code > 128 on Unix. Open_CurrentProcess_Succeeds is annotated [PlatformSpecific(TestPlatforms.Windows)].
  • SafeProcessHandleTests.Unix.cs: macOS-specific tests for Signal(SIGTERM), Signal(SIGINT), invalid signal, and signal-to-exited-process behavior. All long-running tests use CreateTenSecondSleep(). The ProcessId test uses started.Kill() (not the non-owning copy) since the copy handle has no valid pid.

CI changes

  • eng/pipelines/runtime.yml: Added copilot/*.* to CI branch triggers to enable pipeline runs for this branch.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

CopilotAIand others added 3 commits February 26, 2026 21:41
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
…rocess path)
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
CopilotAI changed the title [WIP] Implement SafeProcessHandle APIs for macOSImplement SafeProcessHandle APIs for macOSFeb 26, 2026

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address my feedback.

Moreover, please modify the .yml file that is required to let the CI run for this branch. I am not an expert in this area, but I think it should be enough to modify this file:

pr:
branches:
include:
- main
- release/*.*

and list copilot/*.* after release/*.*

 - release/*.* 
- copilot/*.* 

Comment threadsrc/libraries/Common/src/Interop/Unix/System.Native/Interop.SpawnProcess.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
…on throws, update tests, add CI branch trigger
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Feb 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please address my feedback.

Moreover, please modify the .yml file that is required to let the CI run for this branch. I am not an expert in this area, but I think it should be enough to mo...

All feedback addressed and copilot/*.* added to the CI branch triggers in eng/pipelines/runtime.yml in 28845d4.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements SafeProcessHandle APIs for macOS using posix_spawn for process creation and kqueue for wait/timeout/cancellation operations. The implementation provides parity with the Windows implementation, enabling cross-platform process management through the new SafeProcessHandle APIs.

Changes:

  • Native layer: Added macOS-specific implementations using posix_spawn, kqueue, and waitpid for process creation and management
  • Managed layer: Implemented all core SafeProcessHandle methods (Start, Open, WaitForExit*, Kill, Resume, Signal) with proper error handling
  • Code refactoring: Extracted shared utility methods (CreateEnvp, AllocNullTerminatedArray, FreeArray) from Interop.ForkAndExecProcess.cs to ProcessUtils.Unix.cs for reuse
  • Tests: Extended cross-platform tests to support macOS and added macOS-specific signal tests

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file
FileDescription
src/native/libs/configure.cmakeAdded CMake feature detection for posix_spawn APIs and kqueue on macOS
src/native/libs/Common/pal_config.h.inAdded configuration defines for detected posix_spawn features
src/native/libs/System.Native/pal_process.hDeclared new native API functions for process spawning, signaling, and waiting
src/native/libs/System.Native/pal_process.cImplemented macOS-specific process APIs using posix_spawn and kqueue, including signal mapping
src/native/libs/System.Native/pal_process_wasi.cAdded ENOTSUP stubs for WASI platform
src/native/libs/System.Native/entrypoints.cRegistered new native function entry points
src/libraries/Common/src/Interop/Unix/System.Native/Interop.SpawnProcess.csAdded P/Invoke declarations for new native APIs
src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.csRefactored to use shared ProcessUtils methods
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Unix.csAdded shared utility methods for environment and array marshalling
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.csUpdated to use ProcessUtils.CreateEnvp instead of local implementation
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.csImplemented all SafeProcessHandle core methods for macOS
src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csprojAdded Interop.SpawnProcess.cs and Interop.Pipe.cs to Unix builds
src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.csExtended platform support to include macOS and updated tests for cross-platform compatibility
src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.Unix.csAdded macOS-specific signal tests
src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csprojAdded SafeProcessHandleTests.Unix.cs to Unix test builds
eng/pipelines/runtime.ymlAdded copilot/. branch trigger (intended for development, should be removed)

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadeng/pipelines/runtime.yml

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address all unresolved feedback

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
…ping, fix pwd test
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Feb 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please address all unresolved feedback

All unresolved feedback addressed in cdd13b0: simplified OpenProcess to kill(pid, 0), removed SIGSTOP from signal mappings, and fixed pwd test to be platform-specific.

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please give my suggestion a try.

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
…hecks
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Feb 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please give my suggestion a try.

Done in 4b02d2e — replaced all HAVE_POSIX_SPAWN* feature detection with #ifdef __APPLE__ platform checks and removed the cmake/config defines.

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address my feedback

Also, annotate the Open_CurrentProcess_Succeeds test with [PlatformSpecific(TestPlatforms.Windows)]

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c
Comment threadsrc/native/libs/System.Native/pal_process.c
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
…e via getpgid
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Mar 9, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot Please address my feedback

Fixed the _isGroupLeader correctness concern in 33db32f. SystemNative_OpenProcess now detects whether the process is a group leader via getpgid(pid) == pid and returns the result to managed code.

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot I've asked you to address #124923 (comment) and you seem to have ignored it. Do address this particular feedback.

Comment threadsrc/libraries/Common/src/Interop/Unix/System.Native/Interop.SpawnProcess.cs Outdated
…d is already used
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Mar 9, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot I've asked you to address #124923 (comment) and you seem to have ignored it. Do address this particular feedback.

Applied the parameter rename in af33fd5.


private static SafeProcessHandle OpenCore(int processId)
{
int result = Interop.Sys.OpenProcess(processId, out int pidfd, out int isGroupLeader);

@jkotasjkotasMar 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that processes can change process groups as long as they stay within the same session. Is that correct? It means that this cached value can be stale.

… to ProcessUtils
- Add IsATty(SafeFileHandle) overload to Interop.IsATty.cs
- Move ConfigureTerminalForChildProcesses logic from Process.*.cs to ProcessUtils.*.cs
- Update Process.*.cs to delegate to ProcessUtils
- Add _usesTerminal field to SafeProcessHandle.Unix.cs
- Detect terminal usage in StartCore with IsATty
- Add OnProcessExited helper, called from all WaitForExit* methods
- Update csproj with new files and IsATty reference
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot address my feedback

…te all callers to use ProcessUtils directly
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Mar 9, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot address my feedback

Done in e88f121 — removed both Process.ConfigureTerminalForChildProcesses wrapper files and updated all callers to use ProcessUtils directly.

@adamsitnik

Copy link
Copy Markdown
Member

Closing due the removal of ProcessStartOptions

@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 20, 2026
@jkotas
jkotas deleted the copilot/implement-safeprocesshandle-apis branch May 22, 2026 15:56
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@adamsitnik@tmds@jkotas
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Implement SafeProcessHandle APIs for macOS by Copilot · Pull Request #124923 · dotnet/runtime · GitHub
Skip to content

Implement SafeProcessHandle APIs for macOS - #124923

Closed
adamsitnik with Copilot wants to merge 25 commits into
copilot/add-new-apis-to-safeprocesshandlefrom
copilot/implement-safeprocesshandle-apis
Closed

Implement SafeProcessHandle APIs for macOS#124923
adamsitnik with Copilot wants to merge 25 commits into
copilot/add-new-apis-to-safeprocesshandlefrom
copilot/implement-safeprocesshandle-apis

Conversation

CopilotAI commented Feb 26, 2026

Copy link
Copy Markdown
Contributor

Description

Implements the macOS-specific managed and native layers for the new SafeProcessHandle APIs (Start, Open, WaitForExit*, Kill, Resume, Signal), using posix_spawn for process creation and kqueue for wait/timeout/cancellation.

Managed changes

  • SafeProcessHandle.Unix.cs: Full implementation of StartCore, OpenCore, WaitForExitCore, TryWaitForExitCore, WaitForExitOrKillOnTimeoutCore, WaitForExitAsyncCore, WaitForExitOrKillOnCancellationAsyncCore, KillCore, ResumeCore, SendSignalCore. Uses NoPidFd = int.MinValue (not -1) since the handle derives from SafeHandleZeroOrMinusOneIsInvalid. All error paths use throw new Win32Exception() (parameterless) to let it fetch the last error automatically. OpenCore correctly detects group leader status by calling getpgid(pid) via native OpenProcess and comparing with pid, rather than hardcoding false. StartCore detects terminal usage via Interop.Sys.IsATty on stdin/stdout/stderr and calls ProcessUtils.ConfigureTerminalForChildProcesses accordingly, with proper s_processStartLock read/write lock usage. _usesTerminal readonly field is stored and used by OnProcessExited helper (called from all WaitForExit* methods) to decrement terminal child count on exit.
  • ProcessUtils.Unix.cs: Extracted CreateEnvp, AllocNullTerminatedArray, FreeArray as internal shared methods, reused by both Process.Unix.cs and SafeProcessHandle.Unix.cs.
  • ProcessUtils.ConfigureTerminalForChildProcesses.Unix.cs (new): Moved terminal configuration logic (ConfigureTerminalForChildProcesses, SetDelayedSigChildConsoleConfigurationHandler, DelayedSigChildConsoleConfiguration, AreChildrenUsingTerminal) from Process to ProcessUtils for shared access by both Process and SafeProcessHandle.
  • ProcessUtils.ConfigureTerminalForChildProcesses.iOS.cs (new): iOS stub with [Conditional("unnecessary")] no-ops, mirroring the original Process.ConfigureTerminalForChildProcesses.iOS.cs.
  • Process.Unix.cs: All callers updated to call ProcessUtils.ConfigureTerminalForChildProcesses, ProcessUtils.SetDelayedSigChildConsoleConfigurationHandler, and ProcessUtils.AreChildrenUsingTerminal directly (no wrapper methods).
  • ProcessWaitState.Unix.cs: Updated to call ProcessUtils.ConfigureTerminalForChildProcesses directly instead of through Process.
  • Process.ConfigureTerminalForChildProcesses.Unix.cs: Removed — all callers now use ProcessUtils directly.
  • Process.ConfigureTerminalForChildProcesses.iOS.cs: Removed — all callers now use ProcessUtils directly.
  • Interop.IsATty.cs: Added SafeFileHandle overload for terminal detection in StartCore.
  • Interop.SpawnProcess.cs: New LibraryImport declarations (EntryPoint = "SystemNative_*") for SpawnProcess, SendSignal, WaitForExitAndReap, TryWaitForExit, TryWaitForExitCancellable, WaitForExitOrKillOnTimeout, TryGetExitCode, OpenProcess. OpenProcess returns the group leader status via an isGroupLeader parameter.
  • Interop.ForkAndExecProcess.cs: Delegates to shared ProcessUtils methods.
  • csproj: Added Interop.Pipe.cs, Interop.SpawnProcess.cs, Interop.IsATty.cs, and new ProcessUtils.ConfigureTerminalForChildProcesses.*.cs files for Unix builds. Removed references to deleted Process.ConfigureTerminalForChildProcesses.*.cs files.

Native changes

  • pal_process.c: macOS implementation using posix_spawn with POSIX_SPAWN_CLOEXEC_DEFAULT, posix_spawn_file_actions_addinherit_np, posix_spawn_file_actions_addchdir_np, POSIX_SPAWN_START_SUSPENDED. The implementation is guarded by #if defined(__APPLE__) && !defined(TARGET_MACCATALYST) && !defined(TARGET_TVOS) to exclude Mac Catalyst and tvOS (iOS is supported). posix_spawn_file_actions_addchdir_np retains HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR_NP feature detection since it is not available on tvOS. Wait functions use kqueue/EVFILT_PROC/NOTE_EXIT (guarded by existing HAVE_KQUEUE) for timeout and cancellation support. Signal mapping reuses SystemNative_GetPlatformSignalNumber and TryConvertSignalCodeToPosixSignal from pal_signal.c instead of duplicating the logic. Environment variable resolution is extracted into a GetEnvVars helper for reuse. OpenProcess uses kill(pid, 0) to check process existence without requiring a child-process relationship, and detects group leader status via getpgid(pid) == pid, returning the result through an out_isGroupLeader output parameter. Both SpawnProcess and OpenProcess set out_pidfd to INT_MIN (not -1) since the managed layer treats -1 as an invalid handle due to SafeHandleZeroOrMinusOneIsInvalid.
  • pal_signal.c / pal_signal.h: Made TryConvertSignalCodeToPosixSignal non-static and added its declaration to the header for reuse by pal_process.c.
  • pal_process_wasi.c: Stubs returning -1 with errno = ENOTSUP.
  • pal_process.h: New function declarations with reordered SpawnProcess arguments (path, argv, envp, working_dir, inherited_handles, inherited_handles_count, stdin/stdout/stderr fds, flags, out params). OpenProcess signature includes out_isGroupLeader parameter.
  • entrypoints.c: New DllImportEntry entries.
  • configure.cmake / pal_config.h.in: Added HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR_NP feature detection (not available on tvOS). No other HAVE_POSIX_SPAWN* defines are added — macOS platform checks (#ifdef __APPLE__) are used instead. Existing HAVE_KQUEUE is reused for sys/event.h inclusion.

Test changes

  • SafeProcessHandleTests.cs: Extended [PlatformSpecific] to TestPlatforms.OSX | TestPlatforms.Windows. All ProcessStartOptions creation uses platform detection (OperatingSystem.IsWindows() ternary). CreateTenSecondSleep returns sleep 10 on Unix. Start_WithNoArguments_Succeeds uses hostname on Windows and pwd on Unix (platform-specific, since pwd is not a standalone executable on Windows). Kill tests assert PosixSignal.SIGKILL and exit code > 128 on Unix. Open_CurrentProcess_Succeeds is annotated [PlatformSpecific(TestPlatforms.Windows)].
  • SafeProcessHandleTests.Unix.cs: macOS-specific tests for Signal(SIGTERM), Signal(SIGINT), invalid signal, and signal-to-exited-process behavior. All long-running tests use CreateTenSecondSleep(). The ProcessId test uses started.Kill() (not the non-owning copy) since the copy handle has no valid pid.

CI changes

  • eng/pipelines/runtime.yml: Added copilot/*.* to CI branch triggers to enable pipeline runs for this branch.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

CopilotAIand others added 3 commits February 26, 2026 21:41
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
…rocess path)
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
CopilotAI changed the title [WIP] Implement SafeProcessHandle APIs for macOSImplement SafeProcessHandle APIs for macOSFeb 26, 2026

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address my feedback.

Moreover, please modify the .yml file that is required to let the CI run for this branch. I am not an expert in this area, but I think it should be enough to modify this file:

pr:
branches:
include:
- main
- release/*.*

and list copilot/*.* after release/*.*

 - release/*.* 
- copilot/*.* 

Comment threadsrc/libraries/Common/src/Interop/Unix/System.Native/Interop.SpawnProcess.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
…on throws, update tests, add CI branch trigger
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Feb 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please address my feedback.

Moreover, please modify the .yml file that is required to let the CI run for this branch. I am not an expert in this area, but I think it should be enough to mo...

All feedback addressed and copilot/*.* added to the CI branch triggers in eng/pipelines/runtime.yml in 28845d4.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements SafeProcessHandle APIs for macOS using posix_spawn for process creation and kqueue for wait/timeout/cancellation operations. The implementation provides parity with the Windows implementation, enabling cross-platform process management through the new SafeProcessHandle APIs.

Changes:

  • Native layer: Added macOS-specific implementations using posix_spawn, kqueue, and waitpid for process creation and management
  • Managed layer: Implemented all core SafeProcessHandle methods (Start, Open, WaitForExit*, Kill, Resume, Signal) with proper error handling
  • Code refactoring: Extracted shared utility methods (CreateEnvp, AllocNullTerminatedArray, FreeArray) from Interop.ForkAndExecProcess.cs to ProcessUtils.Unix.cs for reuse
  • Tests: Extended cross-platform tests to support macOS and added macOS-specific signal tests

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file
FileDescription
src/native/libs/configure.cmakeAdded CMake feature detection for posix_spawn APIs and kqueue on macOS
src/native/libs/Common/pal_config.h.inAdded configuration defines for detected posix_spawn features
src/native/libs/System.Native/pal_process.hDeclared new native API functions for process spawning, signaling, and waiting
src/native/libs/System.Native/pal_process.cImplemented macOS-specific process APIs using posix_spawn and kqueue, including signal mapping
src/native/libs/System.Native/pal_process_wasi.cAdded ENOTSUP stubs for WASI platform
src/native/libs/System.Native/entrypoints.cRegistered new native function entry points
src/libraries/Common/src/Interop/Unix/System.Native/Interop.SpawnProcess.csAdded P/Invoke declarations for new native APIs
src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.csRefactored to use shared ProcessUtils methods
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Unix.csAdded shared utility methods for environment and array marshalling
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.csUpdated to use ProcessUtils.CreateEnvp instead of local implementation
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.csImplemented all SafeProcessHandle core methods for macOS
src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csprojAdded Interop.SpawnProcess.cs and Interop.Pipe.cs to Unix builds
src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.csExtended platform support to include macOS and updated tests for cross-platform compatibility
src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.Unix.csAdded macOS-specific signal tests
src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csprojAdded SafeProcessHandleTests.Unix.cs to Unix test builds
eng/pipelines/runtime.ymlAdded copilot/. branch trigger (intended for development, should be removed)

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadeng/pipelines/runtime.yml

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address all unresolved feedback

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
…ping, fix pwd test
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Feb 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please address all unresolved feedback

All unresolved feedback addressed in cdd13b0: simplified OpenProcess to kill(pid, 0), removed SIGSTOP from signal mappings, and fixed pwd test to be platform-specific.

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please give my suggestion a try.

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
…hecks
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Feb 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please give my suggestion a try.

Done in 4b02d2e — replaced all HAVE_POSIX_SPAWN* feature detection with #ifdef __APPLE__ platform checks and removed the cmake/config defines.

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address my feedback

Also, annotate the Open_CurrentProcess_Succeeds test with [PlatformSpecific(TestPlatforms.Windows)]

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c
Comment threadsrc/native/libs/System.Native/pal_process.c
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
…e via getpgid
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Mar 9, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot Please address my feedback

Fixed the _isGroupLeader correctness concern in 33db32f. SystemNative_OpenProcess now detects whether the process is a group leader via getpgid(pid) == pid and returns the result to managed code.

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot I've asked you to address #124923 (comment) and you seem to have ignored it. Do address this particular feedback.

Comment threadsrc/libraries/Common/src/Interop/Unix/System.Native/Interop.SpawnProcess.cs Outdated
…d is already used
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Mar 9, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot I've asked you to address #124923 (comment) and you seem to have ignored it. Do address this particular feedback.

Applied the parameter rename in af33fd5.


private static SafeProcessHandle OpenCore(int processId)
{
int result = Interop.Sys.OpenProcess(processId, out int pidfd, out int isGroupLeader);

@jkotasjkotasMar 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that processes can change process groups as long as they stay within the same session. Is that correct? It means that this cached value can be stale.

… to ProcessUtils
- Add IsATty(SafeFileHandle) overload to Interop.IsATty.cs
- Move ConfigureTerminalForChildProcesses logic from Process.*.cs to ProcessUtils.*.cs
- Update Process.*.cs to delegate to ProcessUtils
- Add _usesTerminal field to SafeProcessHandle.Unix.cs
- Detect terminal usage in StartCore with IsATty
- Add OnProcessExited helper, called from all WaitForExit* methods
- Update csproj with new files and IsATty reference
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot address my feedback

…te all callers to use ProcessUtils directly
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Mar 9, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot address my feedback

Done in e88f121 — removed both Process.ConfigureTerminalForChildProcesses wrapper files and updated all callers to use ProcessUtils directly.

@adamsitnik

Copy link
Copy Markdown
Member

Closing due the removal of ProcessStartOptions

@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 20, 2026
@jkotas
jkotas deleted the copilot/implement-safeprocesshandle-apis branch May 22, 2026 15:56
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@adamsitnik@tmds@jkotas
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Implement SafeProcessHandle APIs for macOS by Copilot · Pull Request #124923 · dotnet/runtime · GitHub
Skip to content

Implement SafeProcessHandle APIs for macOS - #124923

Closed
adamsitnik with Copilot wants to merge 25 commits into
copilot/add-new-apis-to-safeprocesshandlefrom
copilot/implement-safeprocesshandle-apis
Closed

Implement SafeProcessHandle APIs for macOS#124923
adamsitnik with Copilot wants to merge 25 commits into
copilot/add-new-apis-to-safeprocesshandlefrom
copilot/implement-safeprocesshandle-apis

Conversation

CopilotAI commented Feb 26, 2026

Copy link
Copy Markdown
Contributor

Description

Implements the macOS-specific managed and native layers for the new SafeProcessHandle APIs (Start, Open, WaitForExit*, Kill, Resume, Signal), using posix_spawn for process creation and kqueue for wait/timeout/cancellation.

Managed changes

  • SafeProcessHandle.Unix.cs: Full implementation of StartCore, OpenCore, WaitForExitCore, TryWaitForExitCore, WaitForExitOrKillOnTimeoutCore, WaitForExitAsyncCore, WaitForExitOrKillOnCancellationAsyncCore, KillCore, ResumeCore, SendSignalCore. Uses NoPidFd = int.MinValue (not -1) since the handle derives from SafeHandleZeroOrMinusOneIsInvalid. All error paths use throw new Win32Exception() (parameterless) to let it fetch the last error automatically. OpenCore correctly detects group leader status by calling getpgid(pid) via native OpenProcess and comparing with pid, rather than hardcoding false. StartCore detects terminal usage via Interop.Sys.IsATty on stdin/stdout/stderr and calls ProcessUtils.ConfigureTerminalForChildProcesses accordingly, with proper s_processStartLock read/write lock usage. _usesTerminal readonly field is stored and used by OnProcessExited helper (called from all WaitForExit* methods) to decrement terminal child count on exit.
  • ProcessUtils.Unix.cs: Extracted CreateEnvp, AllocNullTerminatedArray, FreeArray as internal shared methods, reused by both Process.Unix.cs and SafeProcessHandle.Unix.cs.
  • ProcessUtils.ConfigureTerminalForChildProcesses.Unix.cs (new): Moved terminal configuration logic (ConfigureTerminalForChildProcesses, SetDelayedSigChildConsoleConfigurationHandler, DelayedSigChildConsoleConfiguration, AreChildrenUsingTerminal) from Process to ProcessUtils for shared access by both Process and SafeProcessHandle.
  • ProcessUtils.ConfigureTerminalForChildProcesses.iOS.cs (new): iOS stub with [Conditional("unnecessary")] no-ops, mirroring the original Process.ConfigureTerminalForChildProcesses.iOS.cs.
  • Process.Unix.cs: All callers updated to call ProcessUtils.ConfigureTerminalForChildProcesses, ProcessUtils.SetDelayedSigChildConsoleConfigurationHandler, and ProcessUtils.AreChildrenUsingTerminal directly (no wrapper methods).
  • ProcessWaitState.Unix.cs: Updated to call ProcessUtils.ConfigureTerminalForChildProcesses directly instead of through Process.
  • Process.ConfigureTerminalForChildProcesses.Unix.cs: Removed — all callers now use ProcessUtils directly.
  • Process.ConfigureTerminalForChildProcesses.iOS.cs: Removed — all callers now use ProcessUtils directly.
  • Interop.IsATty.cs: Added SafeFileHandle overload for terminal detection in StartCore.
  • Interop.SpawnProcess.cs: New LibraryImport declarations (EntryPoint = "SystemNative_*") for SpawnProcess, SendSignal, WaitForExitAndReap, TryWaitForExit, TryWaitForExitCancellable, WaitForExitOrKillOnTimeout, TryGetExitCode, OpenProcess. OpenProcess returns the group leader status via an isGroupLeader parameter.
  • Interop.ForkAndExecProcess.cs: Delegates to shared ProcessUtils methods.
  • csproj: Added Interop.Pipe.cs, Interop.SpawnProcess.cs, Interop.IsATty.cs, and new ProcessUtils.ConfigureTerminalForChildProcesses.*.cs files for Unix builds. Removed references to deleted Process.ConfigureTerminalForChildProcesses.*.cs files.

Native changes

  • pal_process.c: macOS implementation using posix_spawn with POSIX_SPAWN_CLOEXEC_DEFAULT, posix_spawn_file_actions_addinherit_np, posix_spawn_file_actions_addchdir_np, POSIX_SPAWN_START_SUSPENDED. The implementation is guarded by #if defined(__APPLE__) && !defined(TARGET_MACCATALYST) && !defined(TARGET_TVOS) to exclude Mac Catalyst and tvOS (iOS is supported). posix_spawn_file_actions_addchdir_np retains HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR_NP feature detection since it is not available on tvOS. Wait functions use kqueue/EVFILT_PROC/NOTE_EXIT (guarded by existing HAVE_KQUEUE) for timeout and cancellation support. Signal mapping reuses SystemNative_GetPlatformSignalNumber and TryConvertSignalCodeToPosixSignal from pal_signal.c instead of duplicating the logic. Environment variable resolution is extracted into a GetEnvVars helper for reuse. OpenProcess uses kill(pid, 0) to check process existence without requiring a child-process relationship, and detects group leader status via getpgid(pid) == pid, returning the result through an out_isGroupLeader output parameter. Both SpawnProcess and OpenProcess set out_pidfd to INT_MIN (not -1) since the managed layer treats -1 as an invalid handle due to SafeHandleZeroOrMinusOneIsInvalid.
  • pal_signal.c / pal_signal.h: Made TryConvertSignalCodeToPosixSignal non-static and added its declaration to the header for reuse by pal_process.c.
  • pal_process_wasi.c: Stubs returning -1 with errno = ENOTSUP.
  • pal_process.h: New function declarations with reordered SpawnProcess arguments (path, argv, envp, working_dir, inherited_handles, inherited_handles_count, stdin/stdout/stderr fds, flags, out params). OpenProcess signature includes out_isGroupLeader parameter.
  • entrypoints.c: New DllImportEntry entries.
  • configure.cmake / pal_config.h.in: Added HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR_NP feature detection (not available on tvOS). No other HAVE_POSIX_SPAWN* defines are added — macOS platform checks (#ifdef __APPLE__) are used instead. Existing HAVE_KQUEUE is reused for sys/event.h inclusion.

Test changes

  • SafeProcessHandleTests.cs: Extended [PlatformSpecific] to TestPlatforms.OSX | TestPlatforms.Windows. All ProcessStartOptions creation uses platform detection (OperatingSystem.IsWindows() ternary). CreateTenSecondSleep returns sleep 10 on Unix. Start_WithNoArguments_Succeeds uses hostname on Windows and pwd on Unix (platform-specific, since pwd is not a standalone executable on Windows). Kill tests assert PosixSignal.SIGKILL and exit code > 128 on Unix. Open_CurrentProcess_Succeeds is annotated [PlatformSpecific(TestPlatforms.Windows)].
  • SafeProcessHandleTests.Unix.cs: macOS-specific tests for Signal(SIGTERM), Signal(SIGINT), invalid signal, and signal-to-exited-process behavior. All long-running tests use CreateTenSecondSleep(). The ProcessId test uses started.Kill() (not the non-owning copy) since the copy handle has no valid pid.

CI changes

  • eng/pipelines/runtime.yml: Added copilot/*.* to CI branch triggers to enable pipeline runs for this branch.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

CopilotAIand others added 3 commits February 26, 2026 21:41
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
…rocess path)
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
CopilotAI changed the title [WIP] Implement SafeProcessHandle APIs for macOSImplement SafeProcessHandle APIs for macOSFeb 26, 2026

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address my feedback.

Moreover, please modify the .yml file that is required to let the CI run for this branch. I am not an expert in this area, but I think it should be enough to modify this file:

pr:
branches:
include:
- main
- release/*.*

and list copilot/*.* after release/*.*

 - release/*.* 
- copilot/*.* 

Comment threadsrc/libraries/Common/src/Interop/Unix/System.Native/Interop.SpawnProcess.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
…on throws, update tests, add CI branch trigger
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Feb 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please address my feedback.

Moreover, please modify the .yml file that is required to let the CI run for this branch. I am not an expert in this area, but I think it should be enough to mo...

All feedback addressed and copilot/*.* added to the CI branch triggers in eng/pipelines/runtime.yml in 28845d4.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements SafeProcessHandle APIs for macOS using posix_spawn for process creation and kqueue for wait/timeout/cancellation operations. The implementation provides parity with the Windows implementation, enabling cross-platform process management through the new SafeProcessHandle APIs.

Changes:

  • Native layer: Added macOS-specific implementations using posix_spawn, kqueue, and waitpid for process creation and management
  • Managed layer: Implemented all core SafeProcessHandle methods (Start, Open, WaitForExit*, Kill, Resume, Signal) with proper error handling
  • Code refactoring: Extracted shared utility methods (CreateEnvp, AllocNullTerminatedArray, FreeArray) from Interop.ForkAndExecProcess.cs to ProcessUtils.Unix.cs for reuse
  • Tests: Extended cross-platform tests to support macOS and added macOS-specific signal tests

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file
FileDescription
src/native/libs/configure.cmakeAdded CMake feature detection for posix_spawn APIs and kqueue on macOS
src/native/libs/Common/pal_config.h.inAdded configuration defines for detected posix_spawn features
src/native/libs/System.Native/pal_process.hDeclared new native API functions for process spawning, signaling, and waiting
src/native/libs/System.Native/pal_process.cImplemented macOS-specific process APIs using posix_spawn and kqueue, including signal mapping
src/native/libs/System.Native/pal_process_wasi.cAdded ENOTSUP stubs for WASI platform
src/native/libs/System.Native/entrypoints.cRegistered new native function entry points
src/libraries/Common/src/Interop/Unix/System.Native/Interop.SpawnProcess.csAdded P/Invoke declarations for new native APIs
src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.csRefactored to use shared ProcessUtils methods
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Unix.csAdded shared utility methods for environment and array marshalling
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.csUpdated to use ProcessUtils.CreateEnvp instead of local implementation
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.csImplemented all SafeProcessHandle core methods for macOS
src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csprojAdded Interop.SpawnProcess.cs and Interop.Pipe.cs to Unix builds
src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.csExtended platform support to include macOS and updated tests for cross-platform compatibility
src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.Unix.csAdded macOS-specific signal tests
src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csprojAdded SafeProcessHandleTests.Unix.cs to Unix test builds
eng/pipelines/runtime.ymlAdded copilot/. branch trigger (intended for development, should be removed)

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadeng/pipelines/runtime.yml

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address all unresolved feedback

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
…ping, fix pwd test
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Feb 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please address all unresolved feedback

All unresolved feedback addressed in cdd13b0: simplified OpenProcess to kill(pid, 0), removed SIGSTOP from signal mappings, and fixed pwd test to be platform-specific.

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please give my suggestion a try.

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
…hecks
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Feb 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please give my suggestion a try.

Done in 4b02d2e — replaced all HAVE_POSIX_SPAWN* feature detection with #ifdef __APPLE__ platform checks and removed the cmake/config defines.

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address my feedback

Also, annotate the Open_CurrentProcess_Succeeds test with [PlatformSpecific(TestPlatforms.Windows)]

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c
Comment threadsrc/native/libs/System.Native/pal_process.c
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
…e via getpgid
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Mar 9, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot Please address my feedback

Fixed the _isGroupLeader correctness concern in 33db32f. SystemNative_OpenProcess now detects whether the process is a group leader via getpgid(pid) == pid and returns the result to managed code.

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot I've asked you to address #124923 (comment) and you seem to have ignored it. Do address this particular feedback.

Comment threadsrc/libraries/Common/src/Interop/Unix/System.Native/Interop.SpawnProcess.cs Outdated
…d is already used
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Mar 9, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot I've asked you to address #124923 (comment) and you seem to have ignored it. Do address this particular feedback.

Applied the parameter rename in af33fd5.


private static SafeProcessHandle OpenCore(int processId)
{
int result = Interop.Sys.OpenProcess(processId, out int pidfd, out int isGroupLeader);

@jkotasjkotasMar 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that processes can change process groups as long as they stay within the same session. Is that correct? It means that this cached value can be stale.

… to ProcessUtils
- Add IsATty(SafeFileHandle) overload to Interop.IsATty.cs
- Move ConfigureTerminalForChildProcesses logic from Process.*.cs to ProcessUtils.*.cs
- Update Process.*.cs to delegate to ProcessUtils
- Add _usesTerminal field to SafeProcessHandle.Unix.cs
- Detect terminal usage in StartCore with IsATty
- Add OnProcessExited helper, called from all WaitForExit* methods
- Update csproj with new files and IsATty reference
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot address my feedback

…te all callers to use ProcessUtils directly
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Mar 9, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot address my feedback

Done in e88f121 — removed both Process.ConfigureTerminalForChildProcesses wrapper files and updated all callers to use ProcessUtils directly.

@adamsitnik

Copy link
Copy Markdown
Member

Closing due the removal of ProcessStartOptions

@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 20, 2026
@jkotas
jkotas deleted the copilot/implement-safeprocesshandle-apis branch May 22, 2026 15:56
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@adamsitnik@tmds@jkotas
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); Implement SafeProcessHandle APIs for macOS by Copilot · Pull Request #124923 · dotnet/runtime · GitHub
Skip to content

Implement SafeProcessHandle APIs for macOS - #124923

Closed
adamsitnik with Copilot wants to merge 25 commits into
copilot/add-new-apis-to-safeprocesshandlefrom
copilot/implement-safeprocesshandle-apis
Closed

Implement SafeProcessHandle APIs for macOS#124923
adamsitnik with Copilot wants to merge 25 commits into
copilot/add-new-apis-to-safeprocesshandlefrom
copilot/implement-safeprocesshandle-apis

Conversation

CopilotAI commented Feb 26, 2026

Copy link
Copy Markdown
Contributor

Description

Implements the macOS-specific managed and native layers for the new SafeProcessHandle APIs (Start, Open, WaitForExit*, Kill, Resume, Signal), using posix_spawn for process creation and kqueue for wait/timeout/cancellation.

Managed changes

  • SafeProcessHandle.Unix.cs: Full implementation of StartCore, OpenCore, WaitForExitCore, TryWaitForExitCore, WaitForExitOrKillOnTimeoutCore, WaitForExitAsyncCore, WaitForExitOrKillOnCancellationAsyncCore, KillCore, ResumeCore, SendSignalCore. Uses NoPidFd = int.MinValue (not -1) since the handle derives from SafeHandleZeroOrMinusOneIsInvalid. All error paths use throw new Win32Exception() (parameterless) to let it fetch the last error automatically. OpenCore correctly detects group leader status by calling getpgid(pid) via native OpenProcess and comparing with pid, rather than hardcoding false. StartCore detects terminal usage via Interop.Sys.IsATty on stdin/stdout/stderr and calls ProcessUtils.ConfigureTerminalForChildProcesses accordingly, with proper s_processStartLock read/write lock usage. _usesTerminal readonly field is stored and used by OnProcessExited helper (called from all WaitForExit* methods) to decrement terminal child count on exit.
  • ProcessUtils.Unix.cs: Extracted CreateEnvp, AllocNullTerminatedArray, FreeArray as internal shared methods, reused by both Process.Unix.cs and SafeProcessHandle.Unix.cs.
  • ProcessUtils.ConfigureTerminalForChildProcesses.Unix.cs (new): Moved terminal configuration logic (ConfigureTerminalForChildProcesses, SetDelayedSigChildConsoleConfigurationHandler, DelayedSigChildConsoleConfiguration, AreChildrenUsingTerminal) from Process to ProcessUtils for shared access by both Process and SafeProcessHandle.
  • ProcessUtils.ConfigureTerminalForChildProcesses.iOS.cs (new): iOS stub with [Conditional("unnecessary")] no-ops, mirroring the original Process.ConfigureTerminalForChildProcesses.iOS.cs.
  • Process.Unix.cs: All callers updated to call ProcessUtils.ConfigureTerminalForChildProcesses, ProcessUtils.SetDelayedSigChildConsoleConfigurationHandler, and ProcessUtils.AreChildrenUsingTerminal directly (no wrapper methods).
  • ProcessWaitState.Unix.cs: Updated to call ProcessUtils.ConfigureTerminalForChildProcesses directly instead of through Process.
  • Process.ConfigureTerminalForChildProcesses.Unix.cs: Removed — all callers now use ProcessUtils directly.
  • Process.ConfigureTerminalForChildProcesses.iOS.cs: Removed — all callers now use ProcessUtils directly.
  • Interop.IsATty.cs: Added SafeFileHandle overload for terminal detection in StartCore.
  • Interop.SpawnProcess.cs: New LibraryImport declarations (EntryPoint = "SystemNative_*") for SpawnProcess, SendSignal, WaitForExitAndReap, TryWaitForExit, TryWaitForExitCancellable, WaitForExitOrKillOnTimeout, TryGetExitCode, OpenProcess. OpenProcess returns the group leader status via an isGroupLeader parameter.
  • Interop.ForkAndExecProcess.cs: Delegates to shared ProcessUtils methods.
  • csproj: Added Interop.Pipe.cs, Interop.SpawnProcess.cs, Interop.IsATty.cs, and new ProcessUtils.ConfigureTerminalForChildProcesses.*.cs files for Unix builds. Removed references to deleted Process.ConfigureTerminalForChildProcesses.*.cs files.

Native changes

  • pal_process.c: macOS implementation using posix_spawn with POSIX_SPAWN_CLOEXEC_DEFAULT, posix_spawn_file_actions_addinherit_np, posix_spawn_file_actions_addchdir_np, POSIX_SPAWN_START_SUSPENDED. The implementation is guarded by #if defined(__APPLE__) && !defined(TARGET_MACCATALYST) && !defined(TARGET_TVOS) to exclude Mac Catalyst and tvOS (iOS is supported). posix_spawn_file_actions_addchdir_np retains HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR_NP feature detection since it is not available on tvOS. Wait functions use kqueue/EVFILT_PROC/NOTE_EXIT (guarded by existing HAVE_KQUEUE) for timeout and cancellation support. Signal mapping reuses SystemNative_GetPlatformSignalNumber and TryConvertSignalCodeToPosixSignal from pal_signal.c instead of duplicating the logic. Environment variable resolution is extracted into a GetEnvVars helper for reuse. OpenProcess uses kill(pid, 0) to check process existence without requiring a child-process relationship, and detects group leader status via getpgid(pid) == pid, returning the result through an out_isGroupLeader output parameter. Both SpawnProcess and OpenProcess set out_pidfd to INT_MIN (not -1) since the managed layer treats -1 as an invalid handle due to SafeHandleZeroOrMinusOneIsInvalid.
  • pal_signal.c / pal_signal.h: Made TryConvertSignalCodeToPosixSignal non-static and added its declaration to the header for reuse by pal_process.c.
  • pal_process_wasi.c: Stubs returning -1 with errno = ENOTSUP.
  • pal_process.h: New function declarations with reordered SpawnProcess arguments (path, argv, envp, working_dir, inherited_handles, inherited_handles_count, stdin/stdout/stderr fds, flags, out params). OpenProcess signature includes out_isGroupLeader parameter.
  • entrypoints.c: New DllImportEntry entries.
  • configure.cmake / pal_config.h.in: Added HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR_NP feature detection (not available on tvOS). No other HAVE_POSIX_SPAWN* defines are added — macOS platform checks (#ifdef __APPLE__) are used instead. Existing HAVE_KQUEUE is reused for sys/event.h inclusion.

Test changes

  • SafeProcessHandleTests.cs: Extended [PlatformSpecific] to TestPlatforms.OSX | TestPlatforms.Windows. All ProcessStartOptions creation uses platform detection (OperatingSystem.IsWindows() ternary). CreateTenSecondSleep returns sleep 10 on Unix. Start_WithNoArguments_Succeeds uses hostname on Windows and pwd on Unix (platform-specific, since pwd is not a standalone executable on Windows). Kill tests assert PosixSignal.SIGKILL and exit code > 128 on Unix. Open_CurrentProcess_Succeeds is annotated [PlatformSpecific(TestPlatforms.Windows)].
  • SafeProcessHandleTests.Unix.cs: macOS-specific tests for Signal(SIGTERM), Signal(SIGINT), invalid signal, and signal-to-exited-process behavior. All long-running tests use CreateTenSecondSleep(). The ProcessId test uses started.Kill() (not the non-owning copy) since the copy handle has no valid pid.

CI changes

  • eng/pipelines/runtime.yml: Added copilot/*.* to CI branch triggers to enable pipeline runs for this branch.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

CopilotAIand others added 3 commits February 26, 2026 21:41
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
…rocess path)
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
CopilotAI changed the title [WIP] Implement SafeProcessHandle APIs for macOSImplement SafeProcessHandle APIs for macOSFeb 26, 2026

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address my feedback.

Moreover, please modify the .yml file that is required to let the CI run for this branch. I am not an expert in this area, but I think it should be enough to modify this file:

pr:
branches:
include:
- main
- release/*.*

and list copilot/*.* after release/*.*

 - release/*.* 
- copilot/*.* 

Comment threadsrc/libraries/Common/src/Interop/Unix/System.Native/Interop.SpawnProcess.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
…on throws, update tests, add CI branch trigger
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Feb 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please address my feedback.

Moreover, please modify the .yml file that is required to let the CI run for this branch. I am not an expert in this area, but I think it should be enough to mo...

All feedback addressed and copilot/*.* added to the CI branch triggers in eng/pipelines/runtime.yml in 28845d4.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements SafeProcessHandle APIs for macOS using posix_spawn for process creation and kqueue for wait/timeout/cancellation operations. The implementation provides parity with the Windows implementation, enabling cross-platform process management through the new SafeProcessHandle APIs.

Changes:

  • Native layer: Added macOS-specific implementations using posix_spawn, kqueue, and waitpid for process creation and management
  • Managed layer: Implemented all core SafeProcessHandle methods (Start, Open, WaitForExit*, Kill, Resume, Signal) with proper error handling
  • Code refactoring: Extracted shared utility methods (CreateEnvp, AllocNullTerminatedArray, FreeArray) from Interop.ForkAndExecProcess.cs to ProcessUtils.Unix.cs for reuse
  • Tests: Extended cross-platform tests to support macOS and added macOS-specific signal tests

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file
FileDescription
src/native/libs/configure.cmakeAdded CMake feature detection for posix_spawn APIs and kqueue on macOS
src/native/libs/Common/pal_config.h.inAdded configuration defines for detected posix_spawn features
src/native/libs/System.Native/pal_process.hDeclared new native API functions for process spawning, signaling, and waiting
src/native/libs/System.Native/pal_process.cImplemented macOS-specific process APIs using posix_spawn and kqueue, including signal mapping
src/native/libs/System.Native/pal_process_wasi.cAdded ENOTSUP stubs for WASI platform
src/native/libs/System.Native/entrypoints.cRegistered new native function entry points
src/libraries/Common/src/Interop/Unix/System.Native/Interop.SpawnProcess.csAdded P/Invoke declarations for new native APIs
src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.csRefactored to use shared ProcessUtils methods
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Unix.csAdded shared utility methods for environment and array marshalling
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.csUpdated to use ProcessUtils.CreateEnvp instead of local implementation
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.csImplemented all SafeProcessHandle core methods for macOS
src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csprojAdded Interop.SpawnProcess.cs and Interop.Pipe.cs to Unix builds
src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.csExtended platform support to include macOS and updated tests for cross-platform compatibility
src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.Unix.csAdded macOS-specific signal tests
src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csprojAdded SafeProcessHandleTests.Unix.cs to Unix test builds
eng/pipelines/runtime.ymlAdded copilot/. branch trigger (intended for development, should be removed)

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated
Comment threadeng/pipelines/runtime.yml

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address all unresolved feedback

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
…ping, fix pwd test
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Feb 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please address all unresolved feedback

All unresolved feedback addressed in cdd13b0: simplified OpenProcess to kill(pid, 0), removed SIGSTOP from signal mappings, and fixed pwd test to be platform-specific.

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please give my suggestion a try.

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
…hecks
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Feb 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please give my suggestion a try.

Done in 4b02d2e — replaced all HAVE_POSIX_SPAWN* feature detection with #ifdef __APPLE__ platform checks and removed the cmake/config defines.

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address my feedback

Also, annotate the Open_CurrentProcess_Succeeds test with [PlatformSpecific(TestPlatforms.Windows)]

Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c
Comment threadsrc/native/libs/System.Native/pal_process.c
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
Comment threadsrc/native/libs/System.Native/pal_process.c Outdated
…e via getpgid
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Mar 9, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot Please address my feedback

Fixed the _isGroupLeader correctness concern in 33db32f. SystemNative_OpenProcess now detects whether the process is a group leader via getpgid(pid) == pid and returns the result to managed code.

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot I've asked you to address #124923 (comment) and you seem to have ignored it. Do address this particular feedback.

Comment threadsrc/libraries/Common/src/Interop/Unix/System.Native/Interop.SpawnProcess.cs Outdated
…d is already used
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Mar 9, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot I've asked you to address #124923 (comment) and you seem to have ignored it. Do address this particular feedback.

Applied the parameter rename in af33fd5.


private static SafeProcessHandle OpenCore(int processId)
{
int result = Interop.Sys.OpenProcess(processId, out int pidfd, out int isGroupLeader);

@jkotasjkotasMar 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that processes can change process groups as long as they stay within the same session. Is that correct? It means that this cached value can be stale.

… to ProcessUtils
- Add IsATty(SafeFileHandle) overload to Interop.IsATty.cs
- Move ConfigureTerminalForChildProcesses logic from Process.*.cs to ProcessUtils.*.cs
- Update Process.*.cs to delegate to ProcessUtils
- Add _usesTerminal field to SafeProcessHandle.Unix.cs
- Detect terminal usage in StartCore with IsATty
- Add OnProcessExited helper, called from all WaitForExit* methods
- Update csproj with new files and IsATty reference
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot address my feedback

…te all callers to use ProcessUtils directly
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

CopilotAI commented Mar 9, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot address my feedback

Done in e88f121 — removed both Process.ConfigureTerminalForChildProcesses wrapper files and updated all callers to use ProcessUtils directly.

@adamsitnik

Copy link
Copy Markdown
Member

Closing due the removal of ProcessStartOptions

@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 20, 2026
@jkotas
jkotas deleted the copilot/implement-safeprocesshandle-apis branch May 22, 2026 15:56
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@adamsitnik@tmds@jkotas