From bad944e4ccf0d45efb59ecfdd3410b743b5d9b3d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 9 Apr 2026 11:51:39 +0000 Subject: [PATCH 1/5] Implement SafeProcessHandle.Open API with pidfd support on Linux Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/f57caa85-c528-444f-ab43-a628d6ddd488 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Unix/System.Native/Interop.OpenProcess.cs | 13 +++++ .../ref/System.Diagnostics.Process.cs | 4 ++ .../SafeHandles/SafeProcessHandle.Unix.cs | 26 +++++++++ .../SafeHandles/SafeProcessHandle.Windows.cs | 19 +++++++ .../Win32/SafeHandles/SafeProcessHandle.cs | 34 ++++++++++++ .../src/System.Diagnostics.Process.csproj | 2 + .../tests/SafeProcessHandleTests.cs | 54 +++++++++++++++++++ src/native/libs/System.Native/pal_process.c | 36 +++++++++++++ src/native/libs/System.Native/pal_process.h | 11 ++++ .../libs/System.Native/pal_process_wasi.c | 8 +++ 10 files changed, 207 insertions(+) create mode 100644 src/libraries/Common/src/Interop/Unix/System.Native/Interop.OpenProcess.cs diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.OpenProcess.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.OpenProcess.cs new file mode 100644 index 00000000000000..bb94fc51bacd87 --- /dev/null +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.OpenProcess.cs @@ -0,0 +1,13 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.InteropServices; + +internal static partial class Interop +{ + internal static partial class Sys + { + [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_OpenProcess", SetLastError = true)] + internal static partial int OpenProcess(int processId, out int pidfd); + } +} diff --git a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs index 886615c05ef1cb..89e605ee2ceaa1 100644 --- a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs +++ b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs @@ -23,6 +23,10 @@ public void Kill() { } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] + public static Microsoft.Win32.SafeHandles.SafeProcessHandle Open(int processId) { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] public static Microsoft.Win32.SafeHandles.SafeProcessHandle Start(System.Diagnostics.ProcessStartInfo startInfo) { throw null; } } } diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs index d8d2e401bee827..117130f92f7bb0 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs @@ -28,6 +28,7 @@ public sealed partial class SafeProcessHandle : SafeHandleZeroOrMinusOneIsInvali private readonly SafeWaitHandle? _handle; private readonly bool _releaseRef; + private int _pidfd = -1; private SafeProcessHandle(int processId, ProcessWaitState.Holder waitStateHolder) : base(ownsHandle: true) { @@ -46,6 +47,13 @@ internal SafeProcessHandle(int processId, SafeWaitHandle handle) : handle.DangerousAddRef(ref _releaseRef); } + private SafeProcessHandle(int pidfd, int processId) : base(ownsHandle: true) + { + ProcessId = processId; + _pidfd = pidfd; + SetHandle(new IntPtr(pidfd)); + } + protected override bool ReleaseHandle() { if (_releaseRef) @@ -53,12 +61,30 @@ protected override bool ReleaseHandle() Debug.Assert(_handle != null); _handle.DangerousRelease(); } + + if (_pidfd >= 0) + { + Interop.Sys.Close(_pidfd); + } + return true; } // On Unix, we don't use process descriptors yet, so we can't get PID. private static int GetProcessIdCore() => throw new PlatformNotSupportedException(); + private static SafeProcessHandle OpenCore(int processId) + { + int result = Interop.Sys.OpenProcess(processId, out int pidfd); + + if (result == -1) + { + throw new Win32Exception(); + } + + return new SafeProcessHandle(pidfd, processId); + } + private bool SignalCore(PosixSignal signal) { if (!ProcessUtils.PlatformSupportsProcessStartAndKill) diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs index b790749df0c9c3..65b312a0d23ecd 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs @@ -19,6 +19,25 @@ protected override bool ReleaseHandle() return Interop.Kernel32.CloseHandle(handle); } + private static SafeProcessHandle OpenCore(int processId) + { + const int desiredAccess = Interop.Advapi32.ProcessOptions.PROCESS_QUERY_LIMITED_INFORMATION + | Interop.Advapi32.ProcessOptions.SYNCHRONIZE + | Interop.Advapi32.ProcessOptions.PROCESS_TERMINATE; + + SafeProcessHandle safeHandle = Interop.Kernel32.OpenProcess(desiredAccess, inherit: false, processId); + + if (safeHandle.IsInvalid) + { + int error = Marshal.GetLastPInvokeError(); + safeHandle.Dispose(); + throw new Win32Exception(error); + } + + safeHandle.ProcessId = processId; + return safeHandle; + } + private static Func? s_startWithShellExecute; internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs index e203baddf263d9..7a46507a427811 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs @@ -71,6 +71,40 @@ public SafeProcessHandle(IntPtr existingHandle, bool ownsHandle) SetHandle(existingHandle); } + /// + /// Opens an existing process by its process ID. + /// + /// The process ID of the process to open. + /// A that represents the opened process. + /// Thrown when is negative or zero. + /// Thrown when the process could not be opened. + /// + /// + /// On Windows, this method uses OpenProcess with PROCESS_QUERY_LIMITED_INFORMATION, SYNCHRONIZE, and PROCESS_TERMINATE permissions. + /// + /// + /// On Linux with pidfd support, this method uses the pidfd_open syscall. + /// + /// + /// On other Unix systems, this method uses kill(pid, 0) to verify the process exists and the caller has permission to signal it. + /// If it's not a child process of the current process, the returned handle is prone to process ID reuse issues in this case. + /// + /// + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + public static SafeProcessHandle Open(int processId) + { + ArgumentOutOfRangeException.ThrowIfLessThanOrEqual(processId, 0); + + if (!ProcessUtils.PlatformSupportsProcessStartAndKill) + { + throw new PlatformNotSupportedException(); + } + + return OpenCore(processId); + } + /// /// Starts a process using the specified . /// diff --git a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj index b32cd9a542d787..644aa84d1efd10 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -264,6 +264,8 @@ Link="Common\Interop\Unix\Interop.InitializeTerminalAndSignalHandling.cs" /> + (() => SafeProcessHandle.Open(processId)); + } + + [Fact] + public void Open_NonExistentProcessId_ThrowsWin32Exception() + { + // Use an unlikely process ID that should not exist. + Assert.Throws(() => SafeProcessHandle.Open(int.MaxValue)); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void Open_RunningProcess_ReturnsValidHandle() + { + Process process = CreateProcess(static () => + { + Thread.Sleep(Timeout.Infinite); + return RemoteExecutor.SuccessExitCode; + }); + process.Start(); + + try + { + using SafeProcessHandle handle = SafeProcessHandle.Open(process.Id); + Assert.False(handle.IsInvalid); + Assert.Equal(process.Id, handle.ProcessId); + } + finally + { + process.Kill(); + process.WaitForExit(); + } + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void Open_ThenKill_TerminatesProcess() + { + Process process = CreateProcess(static () => + { + Thread.Sleep(Timeout.Infinite); + return RemoteExecutor.SuccessExitCode; + }); + process.Start(); + + using SafeProcessHandle handle = SafeProcessHandle.Open(process.Id); + handle.Kill(); + + Assert.True(process.WaitForExit(WaitInMS)); + } } } diff --git a/src/native/libs/System.Native/pal_process.c b/src/native/libs/System.Native/pal_process.c index 1f1c23acb4e9af..792a429d8d6aa5 100644 --- a/src/native/libs/System.Native/pal_process.c +++ b/src/native/libs/System.Native/pal_process.c @@ -36,7 +36,23 @@ # define __NR_close_range 436 # endif #endif // !defined(__NR_close_range) +#else // HAVE_CLOSE_RANGE +#include #endif // !defined(HAVE_CLOSE_RANGE) +#if !defined(SYS_pidfd_open) && !defined(__NR_pidfd_open) +// pidfd_open was added in Linux 5.3. The syscall number is 434 for all +// architectures using the generic syscall table (asm-generic/unistd.h), +// which covers aarch64, riscv, s390x, ppc64le, and others. The exception +// is alpha, which has its own syscall table and uses 544 instead. +# if defined(__alpha__) +# define __NR_pidfd_open 544 +# else +# define __NR_pidfd_open 434 +# endif +#endif // !defined(SYS_pidfd_open) && !defined(__NR_pidfd_open) +#if !defined(SYS_pidfd_open) && defined(__NR_pidfd_open) +#define SYS_pidfd_open __NR_pidfd_open +#endif #endif // defined(__linux__) #if (HAVE_CLOSE_RANGE || defined(__NR_close_range)) && !defined(CLOSE_RANGE_CLOEXEC) #define CLOSE_RANGE_CLOEXEC (1U << 2) @@ -1073,3 +1089,23 @@ char* SystemNative_GetProcessPath(void) { return minipal_getexepath(); } + +int32_t SystemNative_OpenProcess(int32_t pid, int32_t* out_pidfd) +{ + *out_pidfd = -1; + +#if defined(__linux__) + int pidfd = (int)syscall(SYS_pidfd_open, pid, 0); + if (pidfd < 0 && errno != ENOTSUP) + { + return -1; + } + else if (pidfd > 0) + { + *out_pidfd = pidfd; + return 0; + } +#endif + + return kill(pid, 0); +} diff --git a/src/native/libs/System.Native/pal_process.h b/src/native/libs/System.Native/pal_process.h index 082573e57787b6..2b2e29740ffbb2 100644 --- a/src/native/libs/System.Native/pal_process.h +++ b/src/native/libs/System.Native/pal_process.h @@ -236,3 +236,14 @@ PALEXPORT int32_t SystemNative_SchedGetAffinity(int32_t pid, intptr_t* mask); * resolving symbolic links. The caller is responsible for releasing the buffer. */ PALEXPORT char* SystemNative_GetProcessPath(void); + +/** + * Opens a process by its process ID. + * + * On Linux with pidfd support, uses pidfd_open to obtain a process file descriptor. + * On other systems, uses kill(pid, 0) to verify the process exists. + * + * Returns 0 on success; returns -1 on failure and errno is set. + * On success, out_pidfd is set to the pidfd (or -1 if pidfd is not available). + */ +PALEXPORT int32_t SystemNative_OpenProcess(int32_t pid, int32_t* out_pidfd); diff --git a/src/native/libs/System.Native/pal_process_wasi.c b/src/native/libs/System.Native/pal_process_wasi.c index 6895ed8f179435..c7e73d885db0c9 100644 --- a/src/native/libs/System.Native/pal_process_wasi.c +++ b/src/native/libs/System.Native/pal_process_wasi.c @@ -124,3 +124,11 @@ char* SystemNative_GetProcessPath(void) { return minipal_getexepath(); } + +int32_t SystemNative_OpenProcess(int32_t pid, int32_t* out_pidfd) +{ + (void)pid; + *out_pidfd = -1; + errno = ENOTSUP; + return -1; +} From 2ed39ab31435c4e13b265c2d35df8ec5fb81d2eb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 9 Apr 2026 12:33:47 +0000 Subject: [PATCH 2/5] Add SystemNative_OpenProcess to entrypoints.c Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/f57caa85-c528-444f-ab43-a628d6ddd488 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- src/native/libs/System.Native/entrypoints.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/native/libs/System.Native/entrypoints.c b/src/native/libs/System.Native/entrypoints.c index ffff33bfadd6cc..d5ecd28a33946a 100644 --- a/src/native/libs/System.Native/entrypoints.c +++ b/src/native/libs/System.Native/entrypoints.c @@ -228,6 +228,7 @@ static const Entry s_sysNative[] = DllImportEntry(SystemNative_SchedSetAffinity) DllImportEntry(SystemNative_SchedGetAffinity) DllImportEntry(SystemNative_GetProcessPath) + DllImportEntry(SystemNative_OpenProcess) DllImportEntry(SystemNative_GetNonCryptographicallySecureRandomBytes) DllImportEntry(SystemNative_GetCryptographicallySecureRandomBytes) DllImportEntry(SystemNative_GetUnixRelease) From e2bc39d11e5d81dd05eb2e45df3078ad45663483 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 9 Apr 2026 13:26:46 +0000 Subject: [PATCH 3/5] Refactor pidfd_open into TryOpenPidfd helper, extend ForkAndExecProcess with outPidfd parameter Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/664957d4-993f-4f40-aa79-ce45ec55fd11 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Interop.ForkAndExecProcess.cs | 7 +-- .../SafeHandles/SafeProcessHandle.Unix.cs | 9 ++-- src/native/libs/System.Native/pal_process.c | 46 ++++++++++++++----- src/native/libs/System.Native/pal_process.h | 3 +- .../libs/System.Native/pal_process_wasi.c | 3 +- 5 files changed, 48 insertions(+), 20 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs index 23e88dcf9667e9..fb28426d9e9c98 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs @@ -17,7 +17,8 @@ internal static unsafe int ForkAndExecProcess( string filename, string[] argv, IDictionary env, string? cwd, bool setUser, uint userId, uint groupId, uint[]? groups, out int lpChildPid, SafeFileHandle? stdinFd, SafeFileHandle? stdoutFd, SafeFileHandle? stderrFd, - SafeHandle[]? inheritedHandles = null) + SafeHandle[]? inheritedHandles = null, + int* outPidfd = null) { byte** argvPtr = null, envpPtr = null; int result = -1; @@ -76,7 +77,7 @@ internal static unsafe int ForkAndExecProcess( filename, argvPtr, envpPtr, cwd, setUser ? 1 : 0, userId, groupId, pGroups, groups?.Length ?? 0, out lpChildPid, stdinRawFd, stdoutRawFd, stderrRawFd, - pInheritedFds, inheritedFdCount); + pInheritedFds, inheritedFdCount, outPidfd); } return result == 0 ? 0 : Marshal.GetLastPInvokeError(); } @@ -105,7 +106,7 @@ private static unsafe partial int ForkAndExecProcess( string filename, byte** argv, byte** envp, string? cwd, int setUser, uint userId, uint groupId, uint* groups, int groupsLength, out int lpChildPid, int stdinFd, int stdoutFd, int stderrFd, - int* inheritedFds, int inheritedFdCount); + int* inheritedFds, int inheritedFdCount, int* outPidfd); /// /// Allocates a single native memory block containing both a null-terminated pointer array diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs index 117130f92f7bb0..08f8185ac9ed92 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs @@ -245,7 +245,7 @@ private static SafeProcessHandle StartWithShellExecute(ProcessStartInfo startInf private static bool UsesTerminal(SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle) => ProcessUtils.IsTerminal(stdinHandle) || ProcessUtils.IsTerminal(stdoutHandle) || ProcessUtils.IsTerminal(stderrHandle); - private static SafeProcessHandle ForkAndExecProcess( + private static unsafe SafeProcessHandle ForkAndExecProcess( ProcessStartInfo startInfo, string? resolvedFilename, string[] argv, IDictionary env, string? cwd, bool setCredentials, uint userId, uint groupId, uint[]? groups, @@ -261,6 +261,7 @@ private static SafeProcessHandle ForkAndExecProcess( } int childPid, errno; + int pidfd = -1; // Lock to avoid races with OnSigChild // By using a ReaderWriterLock we allow multiple processes to start concurrently. @@ -281,7 +282,7 @@ private static SafeProcessHandle ForkAndExecProcess( resolvedFilename, argv, env, cwd, setCredentials, userId, groupId, groups, out childPid, stdinHandle, stdoutHandle, stderrHandle, - inheritedHandles); + inheritedHandles, &pidfd); if (errno == 0) { @@ -318,7 +319,9 @@ private static SafeProcessHandle ForkAndExecProcess( throw ProcessUtils.CreateExceptionForErrorStartingProcess(new Interop.ErrorInfo(errno).GetErrorMessage(), errno, resolvedFilename, cwd); } - return new SafeProcessHandle(childPid, waitStateHolder!); + SafeProcessHandle processHandle = new SafeProcessHandle(childPid, waitStateHolder!); + processHandle._pidfd = pidfd; + return processHandle; } } } diff --git a/src/native/libs/System.Native/pal_process.c b/src/native/libs/System.Native/pal_process.c index 792a429d8d6aa5..bcfd2d41ca9f7b 100644 --- a/src/native/libs/System.Native/pal_process.c +++ b/src/native/libs/System.Native/pal_process.c @@ -340,6 +340,22 @@ static void RestrictHandleInheritance(int32_t* inheritedFds, int32_t inheritedFd } } +// Attempts to open a pidfd for the given pid using pidfd_open. +// Returns the pidfd on success, or -1 if pidfd is not available. +static int32_t TryOpenPidfd(int32_t pid) +{ +#if defined(__linux__) + int pidfd = (int)syscall(SYS_pidfd_open, pid, 0); + if (pidfd >= 0) + { + return pidfd; + } +#else + (void)pid; +#endif + return -1; +} + int32_t SystemNative_ForkAndExecProcess(const char* filename, char* const argv[], char* const envp[], @@ -354,13 +370,18 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename, int32_t stdoutFd, int32_t stderrFd, int32_t* inheritedFds, - int32_t inheritedFdCount) + int32_t inheritedFdCount, + int32_t* outPidfd) { #if HAVE_FORK || defined(TARGET_OSX) assert(NULL != filename && NULL != argv && NULL != envp && NULL != childPid && (groupsLength == 0 || groups != NULL) && "null argument."); *childPid = -1; + if (outPidfd != NULL) + { + *outPidfd = -1; + } // Make sure we can find and access the executable. exec will do this, of course, but at that point it's already // in the child process, at which point it'll translate to the child process' exit code rather than to failing @@ -489,6 +510,10 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename, } *childPid = spawnedPid; + if (outPidfd != NULL) + { + *outPidfd = TryOpenPidfd(spawnedPid); + } return 0; } #endif @@ -713,6 +738,11 @@ done:; free(getGroupsBuffer); + if (success && outPidfd != NULL) + { + *outPidfd = TryOpenPidfd(*childPid); + } + return success ? 0 : -1; #else // ignore unused parameters @@ -731,6 +761,7 @@ done:; (void)stderrFd; (void)inheritedFds; (void)inheritedFdCount; + (void)outPidfd; return -1; #endif } @@ -1092,20 +1123,11 @@ char* SystemNative_GetProcessPath(void) int32_t SystemNative_OpenProcess(int32_t pid, int32_t* out_pidfd) { - *out_pidfd = -1; - -#if defined(__linux__) - int pidfd = (int)syscall(SYS_pidfd_open, pid, 0); - if (pidfd < 0 && errno != ENOTSUP) - { - return -1; - } - else if (pidfd > 0) + *out_pidfd = TryOpenPidfd(pid); + if (*out_pidfd >= 0) { - *out_pidfd = pidfd; return 0; } -#endif return kill(pid, 0); } diff --git a/src/native/libs/System.Native/pal_process.h b/src/native/libs/System.Native/pal_process.h index 2b2e29740ffbb2..04349f224e16c1 100644 --- a/src/native/libs/System.Native/pal_process.h +++ b/src/native/libs/System.Native/pal_process.h @@ -33,7 +33,8 @@ PALEXPORT int32_t SystemNative_ForkAndExecProcess( int32_t stdoutFd, // the fd for the child's stdout int32_t stderrFd, // the fd for the child's stderr int32_t* inheritedFds, // array of fds to explicitly inherit (-1 to disable restriction) - int32_t inheritedFdCount); // count of fds in inheritedFds; -1 means no restriction + int32_t inheritedFdCount, // count of fds in inheritedFds; -1 means no restriction + int32_t* outPidfd); // [out] the pidfd for the child process (-1 if not available) /************ * The values below in the header are fixed and correct for managed callers to use forever. diff --git a/src/native/libs/System.Native/pal_process_wasi.c b/src/native/libs/System.Native/pal_process_wasi.c index c7e73d885db0c9..25d436246e92f1 100644 --- a/src/native/libs/System.Native/pal_process_wasi.c +++ b/src/native/libs/System.Native/pal_process_wasi.c @@ -31,7 +31,8 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename, int32_t stdoutFd, int32_t stderrFd, int32_t* inheritedFds, - int32_t inheritedFdCount) + int32_t inheritedFdCount, + int32_t* outPidfd) { return -1; } From a114b090fd4505595553d22162674a518dd8b05b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 9 Apr 2026 14:36:39 +0000 Subject: [PATCH 4/5] Address review: use out instead of int* for pidfd params, set -1 on macOS posix_spawn path Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/ed9ebae7-b0ac-4a66-a222-8f7305c55a9b Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Unix/System.Native/Interop.ForkAndExecProcess.cs | 8 ++++---- .../Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs | 4 ++-- src/native/libs/System.Native/pal_process.c | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs index fb28426d9e9c98..305141b9128dd9 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs @@ -17,8 +17,8 @@ internal static unsafe int ForkAndExecProcess( string filename, string[] argv, IDictionary env, string? cwd, bool setUser, uint userId, uint groupId, uint[]? groups, out int lpChildPid, SafeFileHandle? stdinFd, SafeFileHandle? stdoutFd, SafeFileHandle? stderrFd, - SafeHandle[]? inheritedHandles = null, - int* outPidfd = null) + SafeHandle[]? inheritedHandles, + out int lpPidfd) { byte** argvPtr = null, envpPtr = null; int result = -1; @@ -77,7 +77,7 @@ internal static unsafe int ForkAndExecProcess( filename, argvPtr, envpPtr, cwd, setUser ? 1 : 0, userId, groupId, pGroups, groups?.Length ?? 0, out lpChildPid, stdinRawFd, stdoutRawFd, stderrRawFd, - pInheritedFds, inheritedFdCount, outPidfd); + pInheritedFds, inheritedFdCount, out lpPidfd); } return result == 0 ? 0 : Marshal.GetLastPInvokeError(); } @@ -106,7 +106,7 @@ private static unsafe partial int ForkAndExecProcess( string filename, byte** argv, byte** envp, string? cwd, int setUser, uint userId, uint groupId, uint* groups, int groupsLength, out int lpChildPid, int stdinFd, int stdoutFd, int stderrFd, - int* inheritedFds, int inheritedFdCount, int* outPidfd); + int* inheritedFds, int inheritedFdCount, out int outPidfd); /// /// Allocates a single native memory block containing both a null-terminated pointer array diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs index 08f8185ac9ed92..51b9dc055a1d02 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs @@ -245,7 +245,7 @@ private static SafeProcessHandle StartWithShellExecute(ProcessStartInfo startInf private static bool UsesTerminal(SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle) => ProcessUtils.IsTerminal(stdinHandle) || ProcessUtils.IsTerminal(stdoutHandle) || ProcessUtils.IsTerminal(stderrHandle); - private static unsafe SafeProcessHandle ForkAndExecProcess( + private static SafeProcessHandle ForkAndExecProcess( ProcessStartInfo startInfo, string? resolvedFilename, string[] argv, IDictionary env, string? cwd, bool setCredentials, uint userId, uint groupId, uint[]? groups, @@ -282,7 +282,7 @@ private static unsafe SafeProcessHandle ForkAndExecProcess( resolvedFilename, argv, env, cwd, setCredentials, userId, groupId, groups, out childPid, stdinHandle, stdoutHandle, stderrHandle, - inheritedHandles, &pidfd); + inheritedHandles, out pidfd); if (errno == 0) { diff --git a/src/native/libs/System.Native/pal_process.c b/src/native/libs/System.Native/pal_process.c index bcfd2d41ca9f7b..0767eb6b902f75 100644 --- a/src/native/libs/System.Native/pal_process.c +++ b/src/native/libs/System.Native/pal_process.c @@ -512,7 +512,7 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename, *childPid = spawnedPid; if (outPidfd != NULL) { - *outPidfd = TryOpenPidfd(spawnedPid); + *outPidfd = -1; } return 0; } From b63ececd18e9f4b99af76c1be5ca035c6f49aa39 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 10 Apr 2026 10:46:15 +0000 Subject: [PATCH 5/5] Address review: add outPidfd to assert, remove redundant null checks, use int.MinValue for non-pidfd handles Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/99759f32-cf6d-48fa-9f60-1ec4f2630ad2 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Win32/SafeHandles/SafeProcessHandle.Unix.cs | 2 +- src/native/libs/System.Native/pal_process.c | 13 +++---------- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs index 51b9dc055a1d02..dce9b9da057a2a 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs @@ -82,7 +82,7 @@ private static SafeProcessHandle OpenCore(int processId) throw new Win32Exception(); } - return new SafeProcessHandle(pidfd, processId); + return new SafeProcessHandle(pidfd != -1 ? pidfd : int.MinValue, processId); } private bool SignalCore(PosixSignal signal) diff --git a/src/native/libs/System.Native/pal_process.c b/src/native/libs/System.Native/pal_process.c index 0767eb6b902f75..07b80210b0a72f 100644 --- a/src/native/libs/System.Native/pal_process.c +++ b/src/native/libs/System.Native/pal_process.c @@ -375,13 +375,10 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename, { #if HAVE_FORK || defined(TARGET_OSX) assert(NULL != filename && NULL != argv && NULL != envp && NULL != childPid && - (groupsLength == 0 || groups != NULL) && "null argument."); + outPidfd != NULL && (groupsLength == 0 || groups != NULL) && "null argument."); *childPid = -1; - if (outPidfd != NULL) - { - *outPidfd = -1; - } + *outPidfd = -1; // Make sure we can find and access the executable. exec will do this, of course, but at that point it's already // in the child process, at which point it'll translate to the child process' exit code rather than to failing @@ -510,10 +507,6 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename, } *childPid = spawnedPid; - if (outPidfd != NULL) - { - *outPidfd = -1; - } return 0; } #endif @@ -738,7 +731,7 @@ done:; free(getGroupsBuffer); - if (success && outPidfd != NULL) + if (success) { *outPidfd = TryOpenPidfd(*childPid); }