From abb8c0ec4e908a22bfc5b7fa21d9a4888666ed73 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 9 Apr 2026 11:02:27 +0000 Subject: [PATCH 01/23] Implement ProcessStartInfo.KillOnParentExit for Windows Add KillOnParentExit property to ProcessStartInfo that terminates child processes when the parent process exits. Windows implementation uses Job Objects with JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE flag. For CreateProcess: uses PROC_THREAD_ATTRIBUTE_JOB_LIST for atomic assignment. For CreateProcessWithLogonW: starts suspended, assigns to job, then resumes. Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/61612ca1-930c-4836-8554-d143d05c8321 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Advapi32/Interop.ProcessOptions.cs | 1 + .../Windows/Kernel32/Interop.JobObjects.cs | 79 ++++++++ .../ref/System.Diagnostics.Process.cs | 1 + .../SafeHandles/SafeProcessHandle.Windows.cs | 131 ++++++++++++- .../src/Resources/Strings.resx | 3 + .../src/System.Diagnostics.Process.csproj | 2 + .../System/Diagnostics/ProcessStartInfo.cs | 23 +++ .../tests/KillOnParentExitTests.cs | 184 ++++++++++++++++++ .../System.Diagnostics.Process.Tests.csproj | 1 + 9 files changed, 416 insertions(+), 9 deletions(-) create mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.JobObjects.cs create mode 100644 src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs diff --git a/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.ProcessOptions.cs b/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.ProcessOptions.cs index 893a0aaed9073c..95daea39e5fdf2 100644 --- a/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.ProcessOptions.cs +++ b/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.ProcessOptions.cs @@ -44,6 +44,7 @@ internal static partial class StartupInfoOptions internal const int CREATE_UNICODE_ENVIRONMENT = 0x00000400; internal const int CREATE_NO_WINDOW = 0x08000000; internal const int CREATE_NEW_PROCESS_GROUP = 0x00000200; + internal const int CREATE_SUSPENDED = 0x00000004; } } } diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.JobObjects.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.JobObjects.cs new file mode 100644 index 00000000000000..dc618f38262e64 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.JobObjects.cs @@ -0,0 +1,79 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.InteropServices; +using Microsoft.Win32.SafeHandles; + +internal static partial class Interop +{ + internal static partial class Kernel32 + { + internal sealed class SafeJobHandle : SafeHandleZeroOrMinusOneIsInvalid + { + public SafeJobHandle() : base(true) { } + + protected override bool ReleaseHandle() => Interop.Kernel32.CloseHandle(handle); + } + + [LibraryImport(Libraries.Kernel32, SetLastError = true)] + internal static partial SafeJobHandle CreateJobObjectW(IntPtr lpJobAttributes, IntPtr lpName); + + internal const uint JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE = 0x00002000; + + internal enum JOBOBJECTINFOCLASS + { + JobObjectBasicLimitInformation = 2, + JobObjectExtendedLimitInformation = 9 + } + + [StructLayout(LayoutKind.Sequential)] + internal struct IO_COUNTERS + { + internal ulong ReadOperationCount; + internal ulong WriteOperationCount; + internal ulong OtherOperationCount; + internal ulong ReadTransferCount; + internal ulong WriteTransferCount; + internal ulong OtherTransferCount; + } + + [StructLayout(LayoutKind.Sequential)] + internal struct JOBOBJECT_BASIC_LIMIT_INFORMATION + { + internal long PerProcessUserTimeLimit; + internal long PerJobUserTimeLimit; + internal uint LimitFlags; + internal UIntPtr MinimumWorkingSetSize; + internal UIntPtr MaximumWorkingSetSize; + internal uint ActiveProcessLimit; + internal UIntPtr Affinity; + internal uint PriorityClass; + internal uint SchedulingClass; + } + + [StructLayout(LayoutKind.Sequential)] + internal struct JOBOBJECT_EXTENDED_LIMIT_INFORMATION + { + internal JOBOBJECT_BASIC_LIMIT_INFORMATION BasicLimitInformation; + internal IO_COUNTERS IoInfo; + internal UIntPtr ProcessMemoryLimit; + internal UIntPtr JobMemoryLimit; + internal UIntPtr PeakProcessMemoryUsed; + internal UIntPtr PeakJobMemoryUsed; + } + + [LibraryImport(Libraries.Kernel32, SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static partial bool SetInformationJobObject(SafeJobHandle hJob, JOBOBJECTINFOCLASS JobObjectInfoClass, ref JOBOBJECT_EXTENDED_LIMIT_INFORMATION lpJobObjectInfo, uint cbJobObjectInfoLength); + + [LibraryImport(Libraries.Kernel32, SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static partial bool AssignProcessToJobObject(SafeJobHandle hJob, IntPtr hProcess); + + [LibraryImport(Libraries.Kernel32, SetLastError = true)] + internal static partial int ResumeThread(IntPtr hThread); + + internal const int PROC_THREAD_ATTRIBUTE_JOB_LIST = 0x0002000D; + } +} 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..92c57df71264f6 100644 --- a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs +++ b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs @@ -254,6 +254,7 @@ public ProcessStartInfo(string fileName, System.Collections.Generic.IEnumerable< [System.Diagnostics.CodeAnalysis.AllowNullAttribute] public string FileName { get { throw null; } set { } } public System.Collections.Generic.IList? InheritedHandles { get { throw null; } set { } } + public bool KillOnParentExit { get { throw null; } set { } } [System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")] public bool LoadUserProfile { get { throw null; } set { } } [System.CLSCompliantAttribute(false)] 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..3415bbc91840f5 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 @@ -14,11 +14,38 @@ namespace Microsoft.Win32.SafeHandles { public sealed partial class SafeProcessHandle : SafeHandleZeroOrMinusOneIsInvalid { + // Static job object used for KillOnParentExit functionality. + // All child processes with KillOnParentExit=true are assigned to this job. + // The job handle is intentionally never closed - it should live for the + // lifetime of the process. When this process exits, the job object is destroyed + // by the OS, which terminates all child processes in the job. + private static readonly Lazy s_killOnParentExitJob = new(CreateKillOnParentExitJob); + protected override bool ReleaseHandle() { return Interop.Kernel32.CloseHandle(handle); } + private static unsafe Interop.Kernel32.SafeJobHandle CreateKillOnParentExitJob() + { + Interop.Kernel32.JOBOBJECT_EXTENDED_LIMIT_INFORMATION limitInfo = default; + limitInfo.BasicLimitInformation.LimitFlags = Interop.Kernel32.JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; + + Interop.Kernel32.SafeJobHandle jobHandle = Interop.Kernel32.CreateJobObjectW(IntPtr.Zero, IntPtr.Zero); + if (jobHandle.IsInvalid || !Interop.Kernel32.SetInformationJobObject( + jobHandle, + Interop.Kernel32.JOBOBJECTINFOCLASS.JobObjectExtendedLimitInformation, + ref limitInfo, + (uint)sizeof(Interop.Kernel32.JOBOBJECT_EXTENDED_LIMIT_INFORMATION))) + { + int error = Marshal.GetLastWin32Error(); + jobHandle.Dispose(); + throw new Win32Exception(error); + } + + return jobHandle; + } + private static Func? s_startWithShellExecute; internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, @@ -48,6 +75,10 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S // Inheritable copies of the child handles for CreateProcess bool stdinRefAdded = false, stdoutRefAdded = false, stderrRefAdded = false; bool hasInheritedHandles = inheritedHandles is not null; + bool killOnParentExit = startInfo.KillOnParentExit; + // We need extended startup info when we have inherited handles or when we need to pass + // a job list via PROC_THREAD_ATTRIBUTE_JOB_LIST (only for CreateProcess, not CreateProcessWithLogonW). + bool useExtendedStartupInfo = hasInheritedHandles || (killOnParentExit && startInfo.UserName.Length == 0); // When InheritedHandles is set, we use PROC_THREAD_ATTRIBUTE_HANDLE_LIST to restrict inheritance. // For that, we need a reader lock (concurrent starts with different explicit lists are safe). @@ -68,10 +99,11 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S void* attributeListBuffer = null; SafeHandle?[]? handlesToRelease = null; IntPtr* handlesToInherit = null; + IntPtr* jobHandles = null; try { - startupInfoEx.StartupInfo.cb = hasInheritedHandles ? sizeof(Interop.Kernel32.STARTUPINFOEX) : sizeof(Interop.Kernel32.STARTUPINFO); + startupInfoEx.StartupInfo.cb = useExtendedStartupInfo ? sizeof(Interop.Kernel32.STARTUPINFOEX) : sizeof(Interop.Kernel32.STARTUPINFO); ProcessUtils.DuplicateAsInheritableIfNeeded(stdinHandle, ref startupInfoEx.StartupInfo.hStdInput, ref stdinRefAdded); ProcessUtils.DuplicateAsInheritableIfNeeded(stdoutHandle, ref startupInfoEx.StartupInfo.hStdOutput, ref stdoutRefAdded); @@ -87,7 +119,7 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S } // set up the creation flags parameter - int creationFlags = hasInheritedHandles ? Interop.Kernel32.EXTENDED_STARTUPINFO_PRESENT : 0; + int creationFlags = useExtendedStartupInfo ? Interop.Kernel32.EXTENDED_STARTUPINFO_PRESENT : 0; if (startInfo.CreateNoWindow) creationFlags |= Interop.Advapi32.StartupInfoOptions.CREATE_NO_WINDOW; if (startInfo.CreateNewProcessGroup) creationFlags |= Interop.Advapi32.StartupInfoOptions.CREATE_NEW_PROCESS_GROUP; @@ -120,7 +152,28 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S AddToInheritListIfValid(startupInfoEx.StartupInfo.hStdError, handlesToInheritSpan, ref handleCount); EnableInheritanceAndAddRef(inheritedHandles, handlesToInheritSpan, ref handleCount, ref handlesToRelease); - BuildProcThreadAttributeList(handlesToInherit, handleCount, ref attributeListBuffer); + } + + if (useExtendedStartupInfo) + { + // Determine the number of attributes we need to set in the proc thread attribute list. + int attributeCount = 0; + if (hasInheritedHandles) + { + attributeCount++; // PROC_THREAD_ATTRIBUTE_HANDLE_LIST + } + if (killOnParentExit) + { + attributeCount++; // PROC_THREAD_ATTRIBUTE_JOB_LIST + } + + BuildProcThreadAttributeList( + hasInheritedHandles ? handlesToInherit : null, + handleCount, + killOnParentExit, + attributeCount, + ref attributeListBuffer, + ref jobHandles); } startupInfoEx.lpAttributeList = attributeListBuffer; @@ -155,6 +208,14 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S Debug.Assert(!hasInheritedHandles, "Inheriting handles is not supported when starting with alternate credentials."); Debug.Assert(startupInfoEx.StartupInfo.cb == sizeof(Interop.Kernel32.STARTUPINFO)); + // When KillOnParentExit is set and we use CreateProcessWithLogonW (which doesn't support + // PROC_THREAD_ATTRIBUTE_JOB_LIST), we create the process suspended, assign it to the job, + // then resume it. + if (killOnParentExit) + { + creationFlags |= Interop.Advapi32.StartupInfoOptions.CREATE_SUSPENDED; + } + commandLine.NullTerminate(); fixed (char* passwordInClearTextPtr = startInfo.PasswordInClearText ?? string.Empty) fixed (char* environmentBlockPtr = environmentBlock) @@ -218,17 +279,47 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S if (!IsInvalidHandle(processInfo.hProcess)) Marshal.InitHandle(procSH, processInfo.hProcess); - if (!IsInvalidHandle(processInfo.hThread)) - Interop.Kernel32.CloseHandle(processInfo.hThread); if (!retVal) { + if (!IsInvalidHandle(processInfo.hThread)) + Interop.Kernel32.CloseHandle(processInfo.hThread); + string nativeErrorMessage = errorCode == Interop.Errors.ERROR_BAD_EXE_FORMAT || errorCode == Interop.Errors.ERROR_EXE_MACHINE_TYPE_MISMATCH ? SR.InvalidApplication : Interop.Kernel32.GetMessage(errorCode); throw ProcessUtils.CreateExceptionForErrorStartingProcess(nativeErrorMessage, errorCode, startInfo.FileName, workingDirectory); } + + // When the process was started suspended for KillOnParentExit with CreateProcessWithLogonW, + // assign it to the job object and then resume the thread. + if (killOnParentExit && startInfo.UserName.Length != 0) + { + Debug.Assert(!IsInvalidHandle(processInfo.hThread), "Thread handle must be valid for suspended process."); + try + { + if (!Interop.Kernel32.AssignProcessToJobObject(s_killOnParentExitJob.Value, processInfo.hProcess)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + } + catch + { + // If we fail to assign to the job, terminate the suspended process. + Interop.Kernel32.TerminateProcess(procSH, -1); + throw; + } + finally + { + Interop.Kernel32.ResumeThread(processInfo.hThread); + Interop.Kernel32.CloseHandle(processInfo.hThread); + } + } + else if (!IsInvalidHandle(processInfo.hThread)) + { + Interop.Kernel32.CloseHandle(processInfo.hThread); + } } catch { @@ -256,6 +347,7 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S Interop.Kernel32.CloseHandle(startupInfoEx.StartupInfo.hStdError); NativeMemory.Free(handlesToInherit); + NativeMemory.Free(jobHandles); if (attributeListBuffer is not null) { @@ -421,15 +513,18 @@ private static void AddToInheritListIfValid(nint handle, Span handlesToInh } /// - /// Creates and populates a PROC_THREAD_ATTRIBUTE_LIST with a PROC_THREAD_ATTRIBUTE_HANDLE_LIST entry. + /// Creates and populates a PROC_THREAD_ATTRIBUTE_LIST with optional PROC_THREAD_ATTRIBUTE_HANDLE_LIST + /// and PROC_THREAD_ATTRIBUTE_JOB_LIST entries. /// private static unsafe void BuildProcThreadAttributeList( IntPtr* handlesToInherit, int handleCount, - ref void* attributeListBuffer) + bool killOnParentExit, + int attributeCount, + ref void* attributeListBuffer, + ref IntPtr* jobHandles) { nuint size = 0; - int attributeCount = handleCount > 0 ? 1 : 0; Interop.Kernel32.InitializeProcThreadAttributeList(null, attributeCount, 0, ref size); attributeListBuffer = NativeMemory.Alloc(size); @@ -439,7 +534,7 @@ private static unsafe void BuildProcThreadAttributeList( throw new Win32Exception(Marshal.GetLastWin32Error()); } - if (handleCount > 0 && !Interop.Kernel32.UpdateProcThreadAttribute( + if (handlesToInherit is not null && handleCount > 0 && !Interop.Kernel32.UpdateProcThreadAttribute( attributeListBuffer, 0, (IntPtr)Interop.Kernel32.PROC_THREAD_ATTRIBUTE_HANDLE_LIST, @@ -450,6 +545,24 @@ private static unsafe void BuildProcThreadAttributeList( { throw new Win32Exception(Marshal.GetLastWin32Error()); } + + if (killOnParentExit) + { + jobHandles = (IntPtr*)NativeMemory.Alloc(1, (nuint)sizeof(IntPtr)); + jobHandles[0] = s_killOnParentExitJob.Value.DangerousGetHandle(); + + if (!Interop.Kernel32.UpdateProcThreadAttribute( + attributeListBuffer, + 0, + (IntPtr)Interop.Kernel32.PROC_THREAD_ATTRIBUTE_JOB_LIST, + jobHandles, + (nuint)sizeof(IntPtr), + null, + null)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + } } private static void EnableInheritanceAndAddRef( diff --git a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx index 3b63f63cc13461..cfc178cbdbf24f 100644 --- a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx +++ b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx @@ -219,6 +219,9 @@ The InheritedHandles property cannot be used with UseShellExecute or UserName. + + The KillOnParentExit property cannot be used with UseShellExecute. + The RedirectStandardInput, RedirectStandardOutput, and RedirectStandardError properties cannot be used by SafeProcessHandle.Start. Use the StandardInputHandle, StandardOutputHandle, and StandardErrorHandle properties. 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..5d5470473add44 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -105,6 +105,8 @@ Link="Common\Interop\Windows\Kernel32\Interop.GetThreadTimes.cs" /> + public IList? InheritedHandles { get; set; } + /// + /// Gets or sets a value indicating whether the child process should be terminated when the parent process exits. + /// + /// + /// + /// When this property is set to , the operating system will automatically terminate + /// the child process when the parent process exits, regardless of whether the parent exits gracefully or crashes. + /// + /// + /// This property cannot be used together with set to . + /// + /// + /// On Windows, this is implemented using Job Objects with the JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE flag. + /// + /// + /// to terminate the child process when the parent exits; otherwise, . The default is . + public bool KillOnParentExit { get; set; } + public Encoding? StandardInputEncoding { get; set; } public Encoding? StandardErrorEncoding { get; set; } @@ -399,6 +417,11 @@ internal void ThrowIfInvalid(out bool anyRedirection, out SafeHandle[]? inherite throw new InvalidOperationException(SR.InheritedHandlesRequiresCreateProcess); } + if (KillOnParentExit && UseShellExecute) + { + throw new InvalidOperationException(SR.KillOnParentExitCannotBeUsedWithUseShellExecute); + } + if (InheritedHandles is not null) { IList list = InheritedHandles; diff --git a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs new file mode 100644 index 00000000000000..850bcbb04ca691 --- /dev/null +++ b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs @@ -0,0 +1,184 @@ +// 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; +using System.Threading; +using Microsoft.DotNet.RemoteExecutor; +using Microsoft.Win32.SafeHandles; +using Xunit; + +namespace System.Diagnostics.Tests +{ + public class KillOnParentExitTests : ProcessTestBase + { + [Fact] + public void KillOnParentExit_DefaultsToFalse() + { + ProcessStartInfo startInfo = new(); + Assert.False(startInfo.KillOnParentExit); + } + + [Fact] + public void KillOnParentExit_CanBeSetToTrue() + { + ProcessStartInfo startInfo = new(); + startInfo.KillOnParentExit = true; + Assert.True(startInfo.KillOnParentExit); + } + + [Fact] + public void KillOnParentExit_WithUseShellExecute_Throws() + { + ProcessStartInfo startInfo = new("hostname") + { + KillOnParentExit = true, + UseShellExecute = true + }; + + Assert.Throws(() => Process.Start(startInfo)); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [PlatformSpecific(TestPlatforms.Windows)] + public void KillOnParentExit_ProcessStartsAndExitsNormally() + { + ProcessStartInfo startInfo = OperatingSystem.IsWindows() + ? new("cmd") { ArgumentList = { "/c", "echo test" }, KillOnParentExit = true } + : new("sh") { ArgumentList = { "-c", "echo test" }, KillOnParentExit = true }; + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(startInfo); + using Process fetchedProcess = Process.GetProcessById(processHandle.ProcessId); + Assert.True(fetchedProcess.WaitForExit(WaitInMS)); + } + + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [PlatformSpecific(TestPlatforms.Windows)] + [InlineData(true)] + [InlineData(false)] + public void KillOnParentExit_KillsTheChild_WhenParentExits(bool enabled) + { + RemoteInvokeOptions remoteInvokeOptions = new() { CheckExitCode = false }; + + using RemoteInvokeHandle remoteHandle = RemoteExecutor.Invoke( + (enabledStr) => + { + ProcessStartInfo processStartInfo = CreateLongRunningStartInfo(); + processStartInfo.KillOnParentExit = bool.Parse(enabledStr); + + using SafeProcessHandle started = SafeProcessHandle.Start(processStartInfo); + + return started.ProcessId; // return grand child pid as exit code + }, + arg: enabled.ToString(), + remoteInvokeOptions); + + remoteHandle.Process.WaitForExit(); + + VerifyProcessIsRunning(enabled, remoteHandle.ExitCode); + } + + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [PlatformSpecific(TestPlatforms.Windows)] + [InlineData(true)] + [InlineData(false)] + public void KillOnParentExit_KillsTheChild_WhenParentIsKilled(bool enabled) + { + RemoteInvokeOptions remoteInvokeOptions = new() { CheckExitCode = false }; + remoteInvokeOptions.StartInfo.RedirectStandardOutput = true; + remoteInvokeOptions.StartInfo.RedirectStandardInput = true; + + using RemoteInvokeHandle remoteHandle = RemoteExecutor.Invoke( + (enabledStr) => + { + ProcessStartInfo processStartInfo = CreateLongRunningStartInfo(); + processStartInfo.KillOnParentExit = bool.Parse(enabledStr); + + using SafeProcessHandle started = SafeProcessHandle.Start(processStartInfo); + Console.WriteLine(started.ProcessId); + + // This will block the child until parent kills it. + _ = Console.ReadLine(); + }, + arg: enabled.ToString(), + remoteInvokeOptions); + + string firstLine = remoteHandle.Process.StandardOutput.ReadLine(); + int grandChildPid = int.Parse(firstLine); + remoteHandle.Process.Kill(); + remoteHandle.Process.WaitForExit(); + + VerifyProcessIsRunning(enabled, grandChildPid); + } + + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [PlatformSpecific(TestPlatforms.Windows)] + [InlineData(true)] + [InlineData(false)] + public void KillOnParentExit_KillsTheChild_WhenParentCrashes(bool enabled) + { + RemoteInvokeOptions remoteInvokeOptions = new() { CheckExitCode = false }; + remoteInvokeOptions.StartInfo.RedirectStandardOutput = true; + remoteInvokeOptions.StartInfo.RedirectStandardError = true; + remoteInvokeOptions.StartInfo.RedirectStandardInput = true; + + using RemoteInvokeHandle remoteHandle = RemoteExecutor.Invoke( + (enabledStr) => + { + ProcessStartInfo processStartInfo = CreateLongRunningStartInfo(); + processStartInfo.KillOnParentExit = bool.Parse(enabledStr); + + using SafeProcessHandle started = SafeProcessHandle.Start(processStartInfo); + Console.WriteLine(started.ProcessId); + + // This will block the child until parent writes input. + _ = Console.ReadLine(); + + // Guaranteed Access Violation - write to null pointer + Marshal.WriteInt32(IntPtr.Zero, 42); + }, + arg: enabled.ToString(), + remoteInvokeOptions); + + string firstLine = remoteHandle.Process.StandardOutput.ReadLine(); + int grandChildPid = int.Parse(firstLine); + remoteHandle.Process.StandardInput.WriteLine("One AccessViolationException please."); + remoteHandle.Process.WaitForExit(); + + VerifyProcessIsRunning(enabled, grandChildPid); + } + + private static ProcessStartInfo CreateLongRunningStartInfo() + { + // Create a process that sleeps for approximately 10 seconds + return OperatingSystem.IsWindows() + ? new("ping") { ArgumentList = { "127.0.0.1", "-n", "11" } } + : new("sleep") { ArgumentList = { "10" } }; + } + + private static void VerifyProcessIsRunning(bool shouldBeKilled, int processId) + { + // Give the OS a moment to clean up + Thread.Sleep(500); + + if (shouldBeKilled) + { + // When KillOnParentExit is enabled, the process should have been terminated. + Assert.Throws(() => Process.GetProcessById(processId)); + } + else + { + // When KillOnParentExit is disabled, the process should still be running. + using Process process = Process.GetProcessById(processId); + try + { + Assert.False(process.HasExited); + } + finally + { + process.Kill(); + process.WaitForExit(); + } + } + } + } +} diff --git a/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj b/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj index 9fe9c4c3f3dd72..487dfe1571c528 100644 --- a/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj +++ b/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj @@ -39,6 +39,7 @@ + From 31f6e6bc4032dbcea98907af6c5bdcc209e2803f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 9 Apr 2026 12:50:47 +0000 Subject: [PATCH 02/23] Address review feedback: defensive UserName check, resume only on happy path, add user credentials test, fix pid reporting via stdout, remove nop UseShellExecute test Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/c4dd95b4-cd8e-4740-83f9-7358444c04ce Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../SafeHandles/SafeProcessHandle.Windows.cs | 6 +- .../tests/KillOnParentExitTests.cs | 158 ++++++++++++++++-- 2 files changed, 146 insertions(+), 18 deletions(-) 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 3415bbc91840f5..07ea963febf472 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 @@ -78,7 +78,7 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S bool killOnParentExit = startInfo.KillOnParentExit; // We need extended startup info when we have inherited handles or when we need to pass // a job list via PROC_THREAD_ATTRIBUTE_JOB_LIST (only for CreateProcess, not CreateProcessWithLogonW). - bool useExtendedStartupInfo = hasInheritedHandles || (killOnParentExit && startInfo.UserName.Length == 0); + bool useExtendedStartupInfo = hasInheritedHandles || (killOnParentExit && string.IsNullOrEmpty(startInfo.UserName)); // When InheritedHandles is set, we use PROC_THREAD_ATTRIBUTE_HANDLE_LIST to restrict inheritance. // For that, we need a reader lock (concurrent starts with different explicit lists are safe). @@ -294,7 +294,7 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S // When the process was started suspended for KillOnParentExit with CreateProcessWithLogonW, // assign it to the job object and then resume the thread. - if (killOnParentExit && startInfo.UserName.Length != 0) + if (killOnParentExit && !string.IsNullOrEmpty(startInfo.UserName)) { Debug.Assert(!IsInvalidHandle(processInfo.hThread), "Thread handle must be valid for suspended process."); try @@ -303,6 +303,7 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S { throw new Win32Exception(Marshal.GetLastWin32Error()); } + Interop.Kernel32.ResumeThread(processInfo.hThread); } catch { @@ -312,7 +313,6 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S } finally { - Interop.Kernel32.ResumeThread(processInfo.hThread); Interop.Kernel32.CloseHandle(processInfo.hThread); } } diff --git a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs index 850bcbb04ca691..70c324343efd2f 100644 --- a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs @@ -1,9 +1,15 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.ComponentModel; +using System.Linq; +using System.Runtime.CompilerServices; using System.Runtime.InteropServices; +using System.Security.AccessControl; +using System.IO; using System.Threading; using Microsoft.DotNet.RemoteExecutor; +using Microsoft.DotNet.XUnitExtensions; using Microsoft.Win32.SafeHandles; using Xunit; @@ -26,18 +32,6 @@ public void KillOnParentExit_CanBeSetToTrue() Assert.True(startInfo.KillOnParentExit); } - [Fact] - public void KillOnParentExit_WithUseShellExecute_Throws() - { - ProcessStartInfo startInfo = new("hostname") - { - KillOnParentExit = true, - UseShellExecute = true - }; - - Assert.Throws(() => Process.Start(startInfo)); - } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] [PlatformSpecific(TestPlatforms.Windows)] public void KillOnParentExit_ProcessStartsAndExitsNormally() @@ -58,6 +52,7 @@ public void KillOnParentExit_ProcessStartsAndExitsNormally() public void KillOnParentExit_KillsTheChild_WhenParentExits(bool enabled) { RemoteInvokeOptions remoteInvokeOptions = new() { CheckExitCode = false }; + remoteInvokeOptions.StartInfo.RedirectStandardOutput = true; using RemoteInvokeHandle remoteHandle = RemoteExecutor.Invoke( (enabledStr) => @@ -66,15 +61,16 @@ public void KillOnParentExit_KillsTheChild_WhenParentExits(bool enabled) processStartInfo.KillOnParentExit = bool.Parse(enabledStr); using SafeProcessHandle started = SafeProcessHandle.Start(processStartInfo); - - return started.ProcessId; // return grand child pid as exit code + Console.WriteLine(started.ProcessId); }, arg: enabled.ToString(), remoteInvokeOptions); + string firstLine = remoteHandle.Process.StandardOutput.ReadLine(); + int grandChildPid = int.Parse(firstLine); remoteHandle.Process.WaitForExit(); - VerifyProcessIsRunning(enabled, remoteHandle.ExitCode); + VerifyProcessIsRunning(enabled, grandChildPid); } [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -147,6 +143,42 @@ public void KillOnParentExit_KillsTheChild_WhenParentCrashes(bool enabled) VerifyProcessIsRunning(enabled, grandChildPid); } + private static bool IsAdmin_IsNotNano_RemoteExecutorIsSupported + => PlatformDetection.IsWindows && PlatformDetection.IsNotWindowsNanoServer + && PlatformDetection.IsPrivilegedProcess && RemoteExecutor.IsSupported; + + [ConditionalFact(typeof(KillOnParentExitTests), nameof(IsAdmin_IsNotNano_RemoteExecutorIsSupported))] + [PlatformSpecific(TestPlatforms.Windows)] + [OuterLoop("Requires admin privileges")] + [ActiveIssue("https://github.com/dotnet/runtime/issues/80019", TestRuntimes.Mono)] + public void KillOnParentExit_WithUserCredentials() + { + using Process longRunning = CreateProcessLong(); + longRunning.StartInfo.KillOnParentExit = true; + longRunning.StartInfo.LoadUserProfile = true; + + using TestProcessState testAccountCleanup = CreateUserAndExecute(longRunning, Setup, Cleanup); + + string username = testAccountCleanup.ProcessAccountName.Split('\\').Last(); + Assert.Equal(username, Helpers.GetProcessUserName(longRunning)); + + void Setup(string username, string workingDirectory) + { + if (PlatformDetection.IsNotWindowsServerCore) + { + SetAccessControl(username, longRunning.StartInfo.FileName, workingDirectory, add: true); + } + } + + void Cleanup(string username, string workingDirectory) + { + if (PlatformDetection.IsNotWindowsServerCore) + { + SetAccessControl(username, longRunning.StartInfo.FileName, workingDirectory, add: false); + } + } + } + private static ProcessStartInfo CreateLongRunningStartInfo() { // Create a process that sleeps for approximately 10 seconds @@ -180,5 +212,101 @@ private static void VerifyProcessIsRunning(bool shouldBeKilled, int processId) } } } + + private static void SetAccessControl(string userName, string filePath, string directoryPath, bool add) + { + FileInfo fileInfo = new FileInfo(filePath); + FileSecurity fileSecurity = fileInfo.GetAccessControl(); + Apply(userName, fileSecurity, FileSystemRights.ReadAndExecute, add); + fileInfo.SetAccessControl(fileSecurity); + + DirectoryInfo directoryInfo = new DirectoryInfo(directoryPath); + DirectorySecurity directorySecurity = directoryInfo.GetAccessControl(); + Apply(userName, directorySecurity, FileSystemRights.Read, add); + directoryInfo.SetAccessControl(directorySecurity); + + static void Apply(string userName, FileSystemSecurity accessControl, FileSystemRights rights, bool add) + { + FileSystemAccessRule fileSystemAccessRule = new FileSystemAccessRule(userName, rights, AccessControlType.Allow); + + if (add) + { + accessControl.AddAccessRule(fileSystemAccessRule); + } + else + { + accessControl.RemoveAccessRule(fileSystemAccessRule); + } + } + } + + private const int ERROR_SHARING_VIOLATION = 32; + + private static TestProcessState CreateUserAndExecute( + Process process, + Action additionalSetup = null, + Action additionalCleanup = null, + [CallerMemberName] string memberName = "") + { + string callerIntials = new string(memberName.Where(c => char.IsUpper(c)).Take(18).ToArray()); + + WindowsTestAccount processAccount = new WindowsTestAccount(string.Concat("d", callerIntials)); + string workingDirectory = string.IsNullOrEmpty(process.StartInfo.WorkingDirectory) + ? Directory.GetCurrentDirectory() + : process.StartInfo.WorkingDirectory; + + additionalSetup?.Invoke(processAccount.AccountName, workingDirectory); + + process.StartInfo.UserName = processAccount.AccountName.Split('\\').Last(); + process.StartInfo.Domain = processAccount.AccountName.Split('\\').First(); + process.StartInfo.PasswordInClearText = processAccount.Password; + + try + { + bool hasStarted = process.Start(); + return new TestProcessState(process, hasStarted, processAccount, workingDirectory, additionalCleanup); + } + catch (Win32Exception ex) when (ex.NativeErrorCode == ERROR_SHARING_VIOLATION) + { + throw new SkipTestException($"{process.StartInfo.FileName} has been locked by some other process"); + } + } + + private class TestProcessState : IDisposable + { + private readonly Process _process; + private readonly bool _hasStarted; + private readonly WindowsTestAccount _processAccount; + private readonly string _workingDirectory; + private readonly Action _additionalCleanup; + + public TestProcessState( + Process process, + bool hasStarted, + WindowsTestAccount processAccount, + string workingDirectory, + Action additionalCleanup) + { + _process = process; + _hasStarted = hasStarted; + _processAccount = processAccount; + _workingDirectory = workingDirectory; + _additionalCleanup = additionalCleanup; + } + + public string ProcessAccountName => _processAccount?.AccountName; + + public void Dispose() + { + if (_hasStarted) + { + _process.Kill(); + Assert.True(_process.WaitForExit(WaitInMS)); + } + + _additionalCleanup?.Invoke(_processAccount?.AccountName, _workingDirectory); + _processAccount?.Dispose(); + } + } } } From b9106235e21f85b29b84aa2b9fe6b0dcc0da581a Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 9 Apr 2026 15:09:09 +0200 Subject: [PATCH 03/23] address my own feedback: - simplify the tests - avoid duplication!! --- .../SafeHandles/SafeProcessHandle.Windows.cs | 65 ++--- .../tests/KillOnParentExitTests.cs | 223 ++++-------------- .../tests/ProcessStartInfoTests.cs | 7 +- 3 files changed, 83 insertions(+), 212 deletions(-) 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 07ea963febf472..0291e989159303 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 @@ -278,48 +278,25 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S } if (!IsInvalidHandle(processInfo.hProcess)) + { Marshal.InitHandle(procSH, processInfo.hProcess); + // When the process was started suspended for KillOnParentExit with CreateProcessWithLogonW, + // assign it to the job object and then resume the thread. + if (killOnParentExit && !string.IsNullOrEmpty(startInfo.UserName)) + { + AssignJobAndResumeThread(processInfo, procSH); + } + } + if (!retVal) { - if (!IsInvalidHandle(processInfo.hThread)) - Interop.Kernel32.CloseHandle(processInfo.hThread); - string nativeErrorMessage = errorCode == Interop.Errors.ERROR_BAD_EXE_FORMAT || errorCode == Interop.Errors.ERROR_EXE_MACHINE_TYPE_MISMATCH ? SR.InvalidApplication : Interop.Kernel32.GetMessage(errorCode); throw ProcessUtils.CreateExceptionForErrorStartingProcess(nativeErrorMessage, errorCode, startInfo.FileName, workingDirectory); } - - // When the process was started suspended for KillOnParentExit with CreateProcessWithLogonW, - // assign it to the job object and then resume the thread. - if (killOnParentExit && !string.IsNullOrEmpty(startInfo.UserName)) - { - Debug.Assert(!IsInvalidHandle(processInfo.hThread), "Thread handle must be valid for suspended process."); - try - { - if (!Interop.Kernel32.AssignProcessToJobObject(s_killOnParentExitJob.Value, processInfo.hProcess)) - { - throw new Win32Exception(Marshal.GetLastWin32Error()); - } - Interop.Kernel32.ResumeThread(processInfo.hThread); - } - catch - { - // If we fail to assign to the job, terminate the suspended process. - Interop.Kernel32.TerminateProcess(procSH, -1); - throw; - } - finally - { - Interop.Kernel32.CloseHandle(processInfo.hThread); - } - } - else if (!IsInvalidHandle(processInfo.hThread)) - { - Interop.Kernel32.CloseHandle(processInfo.hThread); - } } catch { @@ -328,6 +305,9 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S } finally { + if (!IsInvalidHandle(processInfo.hThread)) + Interop.Kernel32.CloseHandle(processInfo.hThread); + // If the provided handle was inheritable, just release the reference we added. // Otherwise if we created a valid duplicate, close it. @@ -614,6 +594,27 @@ private static void DisableInheritanceAndRelease(SafeHandle?[] handlesToRelease) } } + private static void AssignJobAndResumeThread(Interop.Kernel32.PROCESS_INFORMATION processInfo, SafeProcessHandle procSH) + { + Debug.Assert(!IsInvalidHandle(processInfo.hThread), "Thread handle must be valid for suspended process."); + + try + { + if (!Interop.Kernel32.AssignProcessToJobObject(s_killOnParentExitJob.Value, processInfo.hProcess)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + + Interop.Kernel32.ResumeThread(processInfo.hThread); + } + catch + { + // If we fail to assign to the job, terminate the suspended process. + Interop.Kernel32.TerminateProcess(procSH, -1); + throw; + } + } + private int GetProcessIdCore() => Interop.Kernel32.GetProcessId(this); private bool SignalCore(PosixSignal signal) diff --git a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs index 70c324343efd2f..665738a6adc3ce 100644 --- a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs @@ -2,51 +2,59 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.ComponentModel; -using System.Linq; -using System.Runtime.CompilerServices; using System.Runtime.InteropServices; -using System.Security.AccessControl; -using System.IO; using System.Threading; using Microsoft.DotNet.RemoteExecutor; -using Microsoft.DotNet.XUnitExtensions; -using Microsoft.Win32.SafeHandles; using Xunit; namespace System.Diagnostics.Tests { + [PlatformSpecific(TestPlatforms.Windows)] public class KillOnParentExitTests : ProcessTestBase { [Fact] public void KillOnParentExit_DefaultsToFalse() { ProcessStartInfo startInfo = new(); + Assert.False(startInfo.KillOnParentExit); } [Fact] public void KillOnParentExit_CanBeSetToTrue() { - ProcessStartInfo startInfo = new(); - startInfo.KillOnParentExit = true; + ProcessStartInfo startInfo = new() + { + KillOnParentExit = true + }; + Assert.True(startInfo.KillOnParentExit); } + [Fact] + public void KillOnParentExit_WithUseShellExecute_Throws() + { + ProcessStartInfo startInfo = new("dummy") + { + KillOnParentExit = true, + UseShellExecute = true + }; + + Assert.Throws(() => Process.Start(startInfo)); + } + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - [PlatformSpecific(TestPlatforms.Windows)] public void KillOnParentExit_ProcessStartsAndExitsNormally() { - ProcessStartInfo startInfo = OperatingSystem.IsWindows() - ? new("cmd") { ArgumentList = { "/c", "echo test" }, KillOnParentExit = true } - : new("sh") { ArgumentList = { "-c", "echo test" }, KillOnParentExit = true }; + Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); + process.StartInfo.KillOnParentExit = true; + process.Start(); - using SafeProcessHandle processHandle = SafeProcessHandle.Start(startInfo); - using Process fetchedProcess = Process.GetProcessById(processHandle.ProcessId); - Assert.True(fetchedProcess.WaitForExit(WaitInMS)); + Assert.True(process.WaitForExit(WaitInMS)); + Assert.Equal(RemoteExecutor.SuccessExitCode, process.ExitCode); } [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - [PlatformSpecific(TestPlatforms.Windows)] [InlineData(true)] [InlineData(false)] public void KillOnParentExit_KillsTheChild_WhenParentExits(bool enabled) @@ -57,11 +65,11 @@ public void KillOnParentExit_KillsTheChild_WhenParentExits(bool enabled) using RemoteInvokeHandle remoteHandle = RemoteExecutor.Invoke( (enabledStr) => { - ProcessStartInfo processStartInfo = CreateLongRunningStartInfo(); - processStartInfo.KillOnParentExit = bool.Parse(enabledStr); + using Process grandChild = CreateProcessLong(); + grandChild.StartInfo.KillOnParentExit = bool.Parse(enabledStr); - using SafeProcessHandle started = SafeProcessHandle.Start(processStartInfo); - Console.WriteLine(started.ProcessId); + grandChild.Start(); + Console.WriteLine(grandChild.Id); }, arg: enabled.ToString(), remoteInvokeOptions); @@ -74,7 +82,6 @@ public void KillOnParentExit_KillsTheChild_WhenParentExits(bool enabled) } [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - [PlatformSpecific(TestPlatforms.Windows)] [InlineData(true)] [InlineData(false)] public void KillOnParentExit_KillsTheChild_WhenParentIsKilled(bool enabled) @@ -83,14 +90,14 @@ public void KillOnParentExit_KillsTheChild_WhenParentIsKilled(bool enabled) remoteInvokeOptions.StartInfo.RedirectStandardOutput = true; remoteInvokeOptions.StartInfo.RedirectStandardInput = true; - using RemoteInvokeHandle remoteHandle = RemoteExecutor.Invoke( + using RemoteInvokeHandle childHandle = RemoteExecutor.Invoke( (enabledStr) => { - ProcessStartInfo processStartInfo = CreateLongRunningStartInfo(); - processStartInfo.KillOnParentExit = bool.Parse(enabledStr); + using Process grandChild = CreateProcessLong(); + grandChild.StartInfo.KillOnParentExit = bool.Parse(enabledStr); - using SafeProcessHandle started = SafeProcessHandle.Start(processStartInfo); - Console.WriteLine(started.ProcessId); + grandChild.Start(); + Console.WriteLine(grandChild.Id); // This will block the child until parent kills it. _ = Console.ReadLine(); @@ -98,16 +105,15 @@ public void KillOnParentExit_KillsTheChild_WhenParentIsKilled(bool enabled) arg: enabled.ToString(), remoteInvokeOptions); - string firstLine = remoteHandle.Process.StandardOutput.ReadLine(); + string firstLine = childHandle.Process.StandardOutput.ReadLine(); int grandChildPid = int.Parse(firstLine); - remoteHandle.Process.Kill(); - remoteHandle.Process.WaitForExit(); + childHandle.Process.Kill(); + childHandle.Process.WaitForExit(); VerifyProcessIsRunning(enabled, grandChildPid); } [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - [PlatformSpecific(TestPlatforms.Windows)] [InlineData(true)] [InlineData(false)] public void KillOnParentExit_KillsTheChild_WhenParentCrashes(bool enabled) @@ -117,14 +123,14 @@ public void KillOnParentExit_KillsTheChild_WhenParentCrashes(bool enabled) remoteInvokeOptions.StartInfo.RedirectStandardError = true; remoteInvokeOptions.StartInfo.RedirectStandardInput = true; - using RemoteInvokeHandle remoteHandle = RemoteExecutor.Invoke( + using RemoteInvokeHandle childHandle = RemoteExecutor.Invoke( (enabledStr) => { - ProcessStartInfo processStartInfo = CreateLongRunningStartInfo(); - processStartInfo.KillOnParentExit = bool.Parse(enabledStr); + using Process grandChild = CreateProcessLong(); + grandChild.StartInfo.KillOnParentExit = bool.Parse(enabledStr); - using SafeProcessHandle started = SafeProcessHandle.Start(processStartInfo); - Console.WriteLine(started.ProcessId); + grandChild.Start(); + Console.WriteLine(grandChild.Id); // This will block the child until parent writes input. _ = Console.ReadLine(); @@ -135,58 +141,15 @@ public void KillOnParentExit_KillsTheChild_WhenParentCrashes(bool enabled) arg: enabled.ToString(), remoteInvokeOptions); - string firstLine = remoteHandle.Process.StandardOutput.ReadLine(); + string firstLine = childHandle.Process.StandardOutput.ReadLine(); int grandChildPid = int.Parse(firstLine); - remoteHandle.Process.StandardInput.WriteLine("One AccessViolationException please."); - remoteHandle.Process.WaitForExit(); + childHandle.Process.StandardInput.WriteLine("One AccessViolationException please."); + childHandle.Process.WaitForExit(); + Assert.NotEqual(0, childHandle.Process.ExitCode); VerifyProcessIsRunning(enabled, grandChildPid); } - private static bool IsAdmin_IsNotNano_RemoteExecutorIsSupported - => PlatformDetection.IsWindows && PlatformDetection.IsNotWindowsNanoServer - && PlatformDetection.IsPrivilegedProcess && RemoteExecutor.IsSupported; - - [ConditionalFact(typeof(KillOnParentExitTests), nameof(IsAdmin_IsNotNano_RemoteExecutorIsSupported))] - [PlatformSpecific(TestPlatforms.Windows)] - [OuterLoop("Requires admin privileges")] - [ActiveIssue("https://github.com/dotnet/runtime/issues/80019", TestRuntimes.Mono)] - public void KillOnParentExit_WithUserCredentials() - { - using Process longRunning = CreateProcessLong(); - longRunning.StartInfo.KillOnParentExit = true; - longRunning.StartInfo.LoadUserProfile = true; - - using TestProcessState testAccountCleanup = CreateUserAndExecute(longRunning, Setup, Cleanup); - - string username = testAccountCleanup.ProcessAccountName.Split('\\').Last(); - Assert.Equal(username, Helpers.GetProcessUserName(longRunning)); - - void Setup(string username, string workingDirectory) - { - if (PlatformDetection.IsNotWindowsServerCore) - { - SetAccessControl(username, longRunning.StartInfo.FileName, workingDirectory, add: true); - } - } - - void Cleanup(string username, string workingDirectory) - { - if (PlatformDetection.IsNotWindowsServerCore) - { - SetAccessControl(username, longRunning.StartInfo.FileName, workingDirectory, add: false); - } - } - } - - private static ProcessStartInfo CreateLongRunningStartInfo() - { - // Create a process that sleeps for approximately 10 seconds - return OperatingSystem.IsWindows() - ? new("ping") { ArgumentList = { "127.0.0.1", "-n", "11" } } - : new("sleep") { ArgumentList = { "10" } }; - } - private static void VerifyProcessIsRunning(bool shouldBeKilled, int processId) { // Give the OS a moment to clean up @@ -212,101 +175,5 @@ private static void VerifyProcessIsRunning(bool shouldBeKilled, int processId) } } } - - private static void SetAccessControl(string userName, string filePath, string directoryPath, bool add) - { - FileInfo fileInfo = new FileInfo(filePath); - FileSecurity fileSecurity = fileInfo.GetAccessControl(); - Apply(userName, fileSecurity, FileSystemRights.ReadAndExecute, add); - fileInfo.SetAccessControl(fileSecurity); - - DirectoryInfo directoryInfo = new DirectoryInfo(directoryPath); - DirectorySecurity directorySecurity = directoryInfo.GetAccessControl(); - Apply(userName, directorySecurity, FileSystemRights.Read, add); - directoryInfo.SetAccessControl(directorySecurity); - - static void Apply(string userName, FileSystemSecurity accessControl, FileSystemRights rights, bool add) - { - FileSystemAccessRule fileSystemAccessRule = new FileSystemAccessRule(userName, rights, AccessControlType.Allow); - - if (add) - { - accessControl.AddAccessRule(fileSystemAccessRule); - } - else - { - accessControl.RemoveAccessRule(fileSystemAccessRule); - } - } - } - - private const int ERROR_SHARING_VIOLATION = 32; - - private static TestProcessState CreateUserAndExecute( - Process process, - Action additionalSetup = null, - Action additionalCleanup = null, - [CallerMemberName] string memberName = "") - { - string callerIntials = new string(memberName.Where(c => char.IsUpper(c)).Take(18).ToArray()); - - WindowsTestAccount processAccount = new WindowsTestAccount(string.Concat("d", callerIntials)); - string workingDirectory = string.IsNullOrEmpty(process.StartInfo.WorkingDirectory) - ? Directory.GetCurrentDirectory() - : process.StartInfo.WorkingDirectory; - - additionalSetup?.Invoke(processAccount.AccountName, workingDirectory); - - process.StartInfo.UserName = processAccount.AccountName.Split('\\').Last(); - process.StartInfo.Domain = processAccount.AccountName.Split('\\').First(); - process.StartInfo.PasswordInClearText = processAccount.Password; - - try - { - bool hasStarted = process.Start(); - return new TestProcessState(process, hasStarted, processAccount, workingDirectory, additionalCleanup); - } - catch (Win32Exception ex) when (ex.NativeErrorCode == ERROR_SHARING_VIOLATION) - { - throw new SkipTestException($"{process.StartInfo.FileName} has been locked by some other process"); - } - } - - private class TestProcessState : IDisposable - { - private readonly Process _process; - private readonly bool _hasStarted; - private readonly WindowsTestAccount _processAccount; - private readonly string _workingDirectory; - private readonly Action _additionalCleanup; - - public TestProcessState( - Process process, - bool hasStarted, - WindowsTestAccount processAccount, - string workingDirectory, - Action additionalCleanup) - { - _process = process; - _hasStarted = hasStarted; - _processAccount = processAccount; - _workingDirectory = workingDirectory; - _additionalCleanup = additionalCleanup; - } - - public string ProcessAccountName => _processAccount?.AccountName; - - public void Dispose() - { - if (_hasStarted) - { - _process.Kill(); - Assert.True(_process.WaitForExit(WaitInMS)); - } - - _additionalCleanup?.Invoke(_processAccount?.AccountName, _workingDirectory); - _processAccount?.Dispose(); - } - } } } diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs index ef27eacf7f1102..893de4cd62a69f 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs @@ -500,14 +500,17 @@ public void TestWorkingDirectoryPropertyInChildProcess() }, workingDirectory, new RemoteInvokeOptions { StartInfo = psi }).Dispose(); } - [ConditionalFact(typeof(ProcessStartInfoTests), nameof(IsAdmin_IsNotNano_RemoteExecutorIsSupported))] // Nano has no "netapi32.dll", Admin rights are required + [ConditionalTheory(typeof(ProcessStartInfoTests), nameof(IsAdmin_IsNotNano_RemoteExecutorIsSupported))] // Nano has no "netapi32.dll", Admin rights are required [PlatformSpecific(TestPlatforms.Windows)] [OuterLoop("Requires admin privileges")] [ActiveIssue("https://github.com/dotnet/runtime/issues/80019", TestRuntimes.Mono)] - public void TestUserCredentialsPropertiesOnWindows() + [InlineData(true)] + [InlineData(false)] + public void TestUserCredentialsPropertiesOnWindows(bool killOnParentExit) { using Process longRunning = CreateProcessLong(); longRunning.StartInfo.LoadUserProfile = true; + longRunning.StartInfo.KillOnParentExit = killOnParentExit; using TestProcessState testAccountCleanup = CreateUserAndExecute(longRunning, Setup, Cleanup); From cc04598e9b397b93eb05677e5b89c0259e879263 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 9 Apr 2026 16:00:34 +0000 Subject: [PATCH 04/23] Address review feedback: add SupportedOSPlatform("windows"), check ResumeThread return, remove unused using, improve VerifyProcessIsRunning polling Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/06c58cfa-bdef-4f35-9345-9ab03e7dc542 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../ref/System.Diagnostics.Process.cs | 1 + .../SafeHandles/SafeProcessHandle.Windows.cs | 7 ++-- .../System/Diagnostics/ProcessStartInfo.cs | 4 +++ .../tests/KillOnParentExitTests.cs | 33 +++++++++++++++---- 4 files changed, 37 insertions(+), 8 deletions(-) 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 92c57df71264f6..7f8344c3d8f05c 100644 --- a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs +++ b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs @@ -254,6 +254,7 @@ public ProcessStartInfo(string fileName, System.Collections.Generic.IEnumerable< [System.Diagnostics.CodeAnalysis.AllowNullAttribute] public string FileName { get { throw null; } set { } } public System.Collections.Generic.IList? InheritedHandles { get { throw null; } set { } } + [System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")] public bool KillOnParentExit { get { throw null; } set { } } [System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")] public bool LoadUserProfile { get { throw null; } set { } } 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 0291e989159303..9772eb1caed585 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 @@ -605,11 +605,14 @@ private static void AssignJobAndResumeThread(Interop.Kernel32.PROCESS_INFORMATIO throw new Win32Exception(Marshal.GetLastWin32Error()); } - Interop.Kernel32.ResumeThread(processInfo.hThread); + if (Interop.Kernel32.ResumeThread(processInfo.hThread) == -1) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } } catch { - // If we fail to assign to the job, terminate the suspended process. + // If we fail to assign to the job or resume the thread, terminate the process. Interop.Kernel32.TerminateProcess(procSH, -1); throw; } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs index 8ed30d78bd1e75..9010620a2ed235 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs @@ -8,6 +8,7 @@ using System.ComponentModel; using System.Diagnostics.CodeAnalysis; using System.Runtime.InteropServices; +using System.Runtime.Versioning; using System.Text; using Microsoft.Win32.SafeHandles; @@ -258,6 +259,7 @@ public string Arguments /// /// /// to terminate the child process when the parent exits; otherwise, . The default is . + [SupportedOSPlatform("windows")] public bool KillOnParentExit { get; set; } public Encoding? StandardInputEncoding { get; set; } @@ -417,7 +419,9 @@ internal void ThrowIfInvalid(out bool anyRedirection, out SafeHandle[]? inherite throw new InvalidOperationException(SR.InheritedHandlesRequiresCreateProcess); } +#pragma warning disable CA1416 // KillOnParentExit getter works on all platforms; the attribute guards the actual effect if (KillOnParentExit && UseShellExecute) +#pragma warning restore CA1416 { throw new InvalidOperationException(SR.KillOnParentExitCannotBeUsedWithUseShellExecute); } diff --git a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs index 665738a6adc3ce..863da24fb96ba9 100644 --- a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.ComponentModel; using System.Runtime.InteropServices; using System.Threading; using Microsoft.DotNet.RemoteExecutor; @@ -152,16 +151,38 @@ public void KillOnParentExit_KillsTheChild_WhenParentCrashes(bool enabled) private static void VerifyProcessIsRunning(bool shouldBeKilled, int processId) { - // Give the OS a moment to clean up - Thread.Sleep(500); - if (shouldBeKilled) { - // When KillOnParentExit is enabled, the process should have been terminated. - Assert.Throws(() => Process.GetProcessById(processId)); + const int timeoutMilliseconds = 10_000; + long deadline = Environment.TickCount64 + timeoutMilliseconds; + + while (Environment.TickCount64 < deadline) + { + try + { + using Process process = Process.GetProcessById(processId); + if (process.HasExited || process.WaitForExit(100)) + { + return; + } + } + catch (ArgumentException) + { + return; + } + + Thread.Sleep(100); + } + + using Process finalCheck = Process.GetProcessById(processId); + Assert.True(finalCheck.HasExited || finalCheck.WaitForExit(0), + $"Process {processId} was expected to exit within {timeoutMilliseconds}ms."); } else { + // Give the OS a moment to clean up + Thread.Sleep(500); + // When KillOnParentExit is disabled, the process should still be running. using Process process = Process.GetProcessById(processId); try From afab02e1694ca8330c1c876573e475a7e7baec1f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 10 Apr 2026 07:50:52 +0000 Subject: [PATCH 05/23] Add Windows SDK doc link for JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE and remove unnecessary handleCount > 0 check Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/8f476c7f-4f16-4bdf-b34d-93e9e161ecdf Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs | 2 +- .../src/System/Diagnostics/ProcessStartInfo.cs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) 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 9772eb1caed585..2d095f7401353e 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 @@ -514,7 +514,7 @@ private static unsafe void BuildProcThreadAttributeList( throw new Win32Exception(Marshal.GetLastWin32Error()); } - if (handlesToInherit is not null && handleCount > 0 && !Interop.Kernel32.UpdateProcThreadAttribute( + if (handlesToInherit is not null && !Interop.Kernel32.UpdateProcThreadAttribute( attributeListBuffer, 0, (IntPtr)Interop.Kernel32.PROC_THREAD_ATTRIBUTE_HANDLE_LIST, diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs index 9010620a2ed235..36ec108ca1e205 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs @@ -255,7 +255,8 @@ public string Arguments /// This property cannot be used together with set to . /// /// - /// On Windows, this is implemented using Job Objects with the JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE flag. + /// On Windows, this is implemented using Job Objects with the + /// JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE flag. /// /// /// to terminate the child process when the parent exits; otherwise, . The default is . From 309480cbd53088ede63048c75a83e7f5f397c7bc Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 10 Apr 2026 11:25:58 +0200 Subject: [PATCH 06/23] increase test coverage: - combining handle inheritance restriction with KillOnParentExit - combining invalid standard handles, handle inheritance restriction with KillOnParentExit --- .../tests/KillOnParentExitTests.cs | 39 ++++++++++++------- .../tests/ProcessHandlesTests.Windows.cs | 15 ++++--- 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs index 863da24fb96ba9..22480600dd5796 100644 --- a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs @@ -54,23 +54,26 @@ public void KillOnParentExit_ProcessStartsAndExitsNormally() } [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - [InlineData(true)] - [InlineData(false)] - public void KillOnParentExit_KillsTheChild_WhenParentExits(bool enabled) + [InlineData(true, true)] + [InlineData(true, false)] + [InlineData(false, false)] // (false, true) is tested by ProcessHandleTests + public void KillOnParentExit_KillsTheChild_WhenParentExits(bool enabled, bool restrictInheritance) { RemoteInvokeOptions remoteInvokeOptions = new() { CheckExitCode = false }; remoteInvokeOptions.StartInfo.RedirectStandardOutput = true; using RemoteInvokeHandle remoteHandle = RemoteExecutor.Invoke( - (enabledStr) => + (enabledStr, limitInherianceStr) => { using Process grandChild = CreateProcessLong(); grandChild.StartInfo.KillOnParentExit = bool.Parse(enabledStr); + grandChild.StartInfo.InheritedHandles = bool.Parse(limitInherianceStr) ? [] : null; grandChild.Start(); Console.WriteLine(grandChild.Id); }, - arg: enabled.ToString(), + enabled.ToString(), + restrictInheritance.ToString(), remoteInvokeOptions); string firstLine = remoteHandle.Process.StandardOutput.ReadLine(); @@ -81,19 +84,21 @@ public void KillOnParentExit_KillsTheChild_WhenParentExits(bool enabled) } [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - [InlineData(true)] - [InlineData(false)] - public void KillOnParentExit_KillsTheChild_WhenParentIsKilled(bool enabled) + [InlineData(true, true)] + [InlineData(true, false)] + [InlineData(false, false)] // (false, true) is tested by ProcessHandleTests + public void KillOnParentExit_KillsTheChild_WhenParentIsKilled(bool enabled, bool restrictInheritance) { RemoteInvokeOptions remoteInvokeOptions = new() { CheckExitCode = false }; remoteInvokeOptions.StartInfo.RedirectStandardOutput = true; remoteInvokeOptions.StartInfo.RedirectStandardInput = true; using RemoteInvokeHandle childHandle = RemoteExecutor.Invoke( - (enabledStr) => + (enabledStr, limitInherianceStr) => { using Process grandChild = CreateProcessLong(); grandChild.StartInfo.KillOnParentExit = bool.Parse(enabledStr); + grandChild.StartInfo.InheritedHandles = bool.Parse(limitInherianceStr) ? [] : null; grandChild.Start(); Console.WriteLine(grandChild.Id); @@ -101,7 +106,8 @@ public void KillOnParentExit_KillsTheChild_WhenParentIsKilled(bool enabled) // This will block the child until parent kills it. _ = Console.ReadLine(); }, - arg: enabled.ToString(), + enabled.ToString(), + restrictInheritance.ToString(), remoteInvokeOptions); string firstLine = childHandle.Process.StandardOutput.ReadLine(); @@ -113,9 +119,10 @@ public void KillOnParentExit_KillsTheChild_WhenParentIsKilled(bool enabled) } [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - [InlineData(true)] - [InlineData(false)] - public void KillOnParentExit_KillsTheChild_WhenParentCrashes(bool enabled) + [InlineData(true, true)] + [InlineData(true, false)] + [InlineData(false, false)] // (false, true) is tested by ProcessHandleTests + public void KillOnParentExit_KillsTheChild_WhenParentCrashes(bool enabled, bool restrictInheritance) { RemoteInvokeOptions remoteInvokeOptions = new() { CheckExitCode = false }; remoteInvokeOptions.StartInfo.RedirectStandardOutput = true; @@ -123,10 +130,11 @@ public void KillOnParentExit_KillsTheChild_WhenParentCrashes(bool enabled) remoteInvokeOptions.StartInfo.RedirectStandardInput = true; using RemoteInvokeHandle childHandle = RemoteExecutor.Invoke( - (enabledStr) => + (enabledStr, limitInherianceStr) => { using Process grandChild = CreateProcessLong(); grandChild.StartInfo.KillOnParentExit = bool.Parse(enabledStr); + grandChild.StartInfo.InheritedHandles = bool.Parse(limitInherianceStr) ? [] : null; grandChild.Start(); Console.WriteLine(grandChild.Id); @@ -137,7 +145,8 @@ public void KillOnParentExit_KillsTheChild_WhenParentCrashes(bool enabled) // Guaranteed Access Violation - write to null pointer Marshal.WriteInt32(IntPtr.Zero, 42); }, - arg: enabled.ToString(), + enabled.ToString(), + restrictInheritance.ToString(), remoteInvokeOptions); string firstLine = childHandle.Process.StandardOutput.ReadLine(); diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.Windows.cs index 0734dcb66b6593..7aea1e1e33043e 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.Windows.cs @@ -31,11 +31,13 @@ public void ProcessStartedWithInvalidHandles_ConsoleReportsInvalidHandles() } [Theory] - [InlineData(false)] - [InlineData(true)] - public void ProcessStartedWithInvalidHandles_CanStartChildProcessWithDerivedInvalidHandles(bool restrictHandles) + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public void ProcessStartedWithInvalidHandles_CanStartChildProcessWithDerivedInvalidHandles(bool restrictHandles, bool killOnParentExit) { - using Process process = CreateProcess(arg => + using Process process = CreateProcess((inheritanceArg, killArg) => { using (Process childProcess = CreateProcess(() => { @@ -46,7 +48,8 @@ public void ProcessStartedWithInvalidHandles_CanStartChildProcessWithDerivedInva return RemoteExecutor.SuccessExitCode; })) { - childProcess.StartInfo.InheritedHandles = bool.Parse(arg) ? [] : null; + childProcess.StartInfo.InheritedHandles = bool.Parse(inheritanceArg) ? [] : null; + childProcess.StartInfo.KillOnParentExit = bool.Parse(killArg); childProcess.Start(); try @@ -59,7 +62,7 @@ public void ProcessStartedWithInvalidHandles_CanStartChildProcessWithDerivedInva childProcess.Kill(); } } - }, restrictHandles.ToString()); + }, restrictHandles.ToString(), killOnParentExit.ToString()); Assert.Equal(RemoteExecutor.SuccessExitCode, RunWithInvalidHandles(process.StartInfo)); } From f0f971491fd525534c59a74e2fd38f0aecf2a1f9 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 10 Apr 2026 12:29:59 +0200 Subject: [PATCH 07/23] fix a bug discovered by the tests: when all standard handles are invalid, there is no need to use PROC_THREAD_ATTRIBUTE_HANDLE_LIST --- .../SafeHandles/SafeProcessHandle.Windows.cs | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) 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 2d095f7401353e..e46648b84e7c9c 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 @@ -74,16 +74,16 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S // Inheritable copies of the child handles for CreateProcess bool stdinRefAdded = false, stdoutRefAdded = false, stderrRefAdded = false; - bool hasInheritedHandles = inheritedHandles is not null; + bool restrictInheritedHandles = inheritedHandles is not null; bool killOnParentExit = startInfo.KillOnParentExit; // We need extended startup info when we have inherited handles or when we need to pass // a job list via PROC_THREAD_ATTRIBUTE_JOB_LIST (only for CreateProcess, not CreateProcessWithLogonW). - bool useExtendedStartupInfo = hasInheritedHandles || (killOnParentExit && string.IsNullOrEmpty(startInfo.UserName)); + bool useExtendedStartupInfo = restrictInheritedHandles || (killOnParentExit && string.IsNullOrEmpty(startInfo.UserName)); // When InheritedHandles is set, we use PROC_THREAD_ATTRIBUTE_HANDLE_LIST to restrict inheritance. // For that, we need a reader lock (concurrent starts with different explicit lists are safe). // When InheritedHandles is not set, we use the existing approach with a writer lock. - if (hasInheritedHandles) + if (restrictInheritedHandles) { ProcessUtils.s_processStartLock.EnterReadLock(); } @@ -140,7 +140,7 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S // When InheritedHandles is set, build a PROC_THREAD_ATTRIBUTE_HANDLE_LIST to restrict // inheritance to only the explicitly specified handles. int handleCount = 0; - if (hasInheritedHandles) + if (restrictInheritedHandles) { int maxHandleCount = 3 + inheritedHandles!.Length; handlesToInherit = (IntPtr*)NativeMemory.Alloc((nuint)maxHandleCount, (nuint)sizeof(IntPtr)); @@ -158,7 +158,10 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S { // Determine the number of attributes we need to set in the proc thread attribute list. int attributeCount = 0; - if (hasInheritedHandles) + + // When InheritedHandles is set but handleCount is 0 (all standard handles are invalid), + // we pass bInheritHandles=false to CreateProcess and there is no need to use PROC_THREAD_ATTRIBUTE_HANDLE_LIST. + if (restrictInheritedHandles && handleCount > 0) { attributeCount++; // PROC_THREAD_ATTRIBUTE_HANDLE_LIST } @@ -168,7 +171,7 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S } BuildProcThreadAttributeList( - hasInheritedHandles ? handlesToInherit : null, + handlesToInherit, handleCount, killOnParentExit, attributeCount, @@ -205,7 +208,7 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S // CreateProcessWithLogonW does not support STARTUPINFOEX. CreateProcessWithTokenW docs mention STARTUPINFOEX, // but they don't mention that EXTENDED_STARTUPINFO_PRESENT is not supported anyway. // CreateProcessAsUserW supports both, but it's too restrictive and simply different than CreateProcessWithLogonW in many ways. - Debug.Assert(!hasInheritedHandles, "Inheriting handles is not supported when starting with alternate credentials."); + Debug.Assert(!restrictInheritedHandles, "Inheriting handles is not supported when starting with alternate credentials."); Debug.Assert(startupInfoEx.StartupInfo.cb == sizeof(Interop.Kernel32.STARTUPINFO)); // When KillOnParentExit is set and we use CreateProcessWithLogonW (which doesn't support @@ -259,7 +262,8 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S { // When InheritedHandles is set but handleCount is 0 (e.g. empty list, no stdio), // pass false to prevent all inheritable handles from leaking to the child. - bool bInheritHandles = !hasInheritedHandles || handleCount > 0; + bool bInheritHandles = !restrictInheritedHandles || handleCount > 0; + retVal = Interop.Kernel32.CreateProcess( null, // we don't need this since all the info is in commandLine commandLinePtr, // pointer to the command line string @@ -340,7 +344,7 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S DisableInheritanceAndRelease(handlesToRelease); } - if (hasInheritedHandles) + if (restrictInheritedHandles) { ProcessUtils.s_processStartLock.ExitReadLock(); } @@ -514,7 +518,7 @@ private static unsafe void BuildProcThreadAttributeList( throw new Win32Exception(Marshal.GetLastWin32Error()); } - if (handlesToInherit is not null && !Interop.Kernel32.UpdateProcThreadAttribute( + if (handleCount > 0 && !Interop.Kernel32.UpdateProcThreadAttribute( attributeListBuffer, 0, (IntPtr)Interop.Kernel32.PROC_THREAD_ATTRIBUTE_HANDLE_LIST, From 7f887bd9430c18b2041dd0a34a11d6dd8457cc14 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 10 Apr 2026 12:44:38 +0200 Subject: [PATCH 08/23] Don't create (and upload) a memory dump for this test, as AV is intentional and expected --- .../System.Diagnostics.Process/tests/KillOnParentExitTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs index 22480600dd5796..58aaed778ca701 100644 --- a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs @@ -128,6 +128,8 @@ public void KillOnParentExit_KillsTheChild_WhenParentCrashes(bool enabled, bool remoteInvokeOptions.StartInfo.RedirectStandardOutput = true; remoteInvokeOptions.StartInfo.RedirectStandardError = true; remoteInvokeOptions.StartInfo.RedirectStandardInput = true; + // Don't create (and upload) a memory dump for this test, as AV is intentional and expected. + remoteInvokeOptions.StartInfo.Environment["HELIX_WORKITEM_UPLOAD_ROOT"] = null; using RemoteInvokeHandle childHandle = RemoteExecutor.Invoke( (enabledStr, limitInherianceStr) => From ede5e33b9186b6771ef730a1dfc3762478bbd61d Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 10 Apr 2026 12:52:39 +0200 Subject: [PATCH 09/23] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../System.Diagnostics.Process/tests/KillOnParentExitTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs index 58aaed778ca701..b0f598113d4788 100644 --- a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs @@ -63,11 +63,11 @@ public void KillOnParentExit_KillsTheChild_WhenParentExits(bool enabled, bool re remoteInvokeOptions.StartInfo.RedirectStandardOutput = true; using RemoteInvokeHandle remoteHandle = RemoteExecutor.Invoke( - (enabledStr, limitInherianceStr) => + (enabledStr, limitInheritanceStr) => { using Process grandChild = CreateProcessLong(); grandChild.StartInfo.KillOnParentExit = bool.Parse(enabledStr); - grandChild.StartInfo.InheritedHandles = bool.Parse(limitInherianceStr) ? [] : null; + grandChild.StartInfo.InheritedHandles = bool.Parse(limitInheritanceStr) ? [] : null; grandChild.Start(); Console.WriteLine(grandChild.Id); From 9bb0ef450b81696b58a76c480efdeaa1bcbfdbc1 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 10 Apr 2026 14:50:10 +0200 Subject: [PATCH 10/23] Apply suggestions from code review --- .../tests/KillOnParentExitTests.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs index b0f598113d4788..692994715315b5 100644 --- a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs @@ -45,7 +45,7 @@ public void KillOnParentExit_WithUseShellExecute_Throws() [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] public void KillOnParentExit_ProcessStartsAndExitsNormally() { - Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); + using Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); process.StartInfo.KillOnParentExit = true; process.Start(); @@ -94,11 +94,11 @@ public void KillOnParentExit_KillsTheChild_WhenParentIsKilled(bool enabled, bool remoteInvokeOptions.StartInfo.RedirectStandardInput = true; using RemoteInvokeHandle childHandle = RemoteExecutor.Invoke( - (enabledStr, limitInherianceStr) => + (enabledStr, limitInheritanceStr) => { using Process grandChild = CreateProcessLong(); grandChild.StartInfo.KillOnParentExit = bool.Parse(enabledStr); - grandChild.StartInfo.InheritedHandles = bool.Parse(limitInherianceStr) ? [] : null; + grandChild.StartInfo.InheritedHandles = bool.Parse(limitInheritanceStr) ? [] : null; grandChild.Start(); Console.WriteLine(grandChild.Id); @@ -132,11 +132,11 @@ public void KillOnParentExit_KillsTheChild_WhenParentCrashes(bool enabled, bool remoteInvokeOptions.StartInfo.Environment["HELIX_WORKITEM_UPLOAD_ROOT"] = null; using RemoteInvokeHandle childHandle = RemoteExecutor.Invoke( - (enabledStr, limitInherianceStr) => + (enabledStr, limitInheritanceStr) => { using Process grandChild = CreateProcessLong(); grandChild.StartInfo.KillOnParentExit = bool.Parse(enabledStr); - grandChild.StartInfo.InheritedHandles = bool.Parse(limitInherianceStr) ? [] : null; + grandChild.StartInfo.InheritedHandles = bool.Parse(limitInheritanceStr) ? [] : null; grandChild.Start(); Console.WriteLine(grandChild.Id); From 7b0e6525dfc347b8529eb90ac4b1df3216d66c98 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 10 Apr 2026 16:22:59 +0200 Subject: [PATCH 11/23] make the tests less flaky --- .../tests/KillOnParentExitTests.cs | 108 ++++++++---------- 1 file changed, 50 insertions(+), 58 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs index 692994715315b5..303e58b405775d 100644 --- a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs @@ -31,6 +31,7 @@ public void KillOnParentExit_CanBeSetToTrue() } [Fact] + [SkipOnPlatform(TestPlatforms.iOS | TestPlatforms.tvOS, "Not supported on iOS and tvOS.")] public void KillOnParentExit_WithUseShellExecute_Throws() { ProcessStartInfo startInfo = new("dummy") @@ -61,8 +62,9 @@ public void KillOnParentExit_KillsTheChild_WhenParentExits(bool enabled, bool re { RemoteInvokeOptions remoteInvokeOptions = new() { CheckExitCode = false }; remoteInvokeOptions.StartInfo.RedirectStandardOutput = true; + remoteInvokeOptions.StartInfo.RedirectStandardInput = true; - using RemoteInvokeHandle remoteHandle = RemoteExecutor.Invoke( + using RemoteInvokeHandle childHandle = RemoteExecutor.Invoke( (enabledStr, limitInheritanceStr) => { using Process grandChild = CreateProcessLong(); @@ -71,16 +73,31 @@ public void KillOnParentExit_KillsTheChild_WhenParentExits(bool enabled, bool re grandChild.Start(); Console.WriteLine(grandChild.Id); + + // This will block the child until parent provides input. + _ = Console.ReadLine(); }, enabled.ToString(), restrictInheritance.ToString(), remoteInvokeOptions); - string firstLine = remoteHandle.Process.StandardOutput.ReadLine(); - int grandChildPid = int.Parse(firstLine); - remoteHandle.Process.WaitForExit(); + int grandChildPid = int.Parse(childHandle.Process.StandardOutput.ReadLine()); + + // Obtain a Process instance before the child exits to avoid PID reuse issues. + using Process grandchild = Process.GetProcessById(grandChildPid); - VerifyProcessIsRunning(enabled, grandChildPid); + try + { + childHandle.Process.StandardInput.WriteLine("You can exit now."); + + Assert.True(childHandle.Process.WaitForExit(WaitInMS)); + // Use shorter wait time when the process is expected to survive + Assert.Equal(enabled, grandchild.WaitForExit(enabled ? WaitInMS : 300)); + } + finally + { + grandchild.Kill(); + } } [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -110,12 +127,24 @@ public void KillOnParentExit_KillsTheChild_WhenParentIsKilled(bool enabled, bool restrictInheritance.ToString(), remoteInvokeOptions); - string firstLine = childHandle.Process.StandardOutput.ReadLine(); - int grandChildPid = int.Parse(firstLine); - childHandle.Process.Kill(); - childHandle.Process.WaitForExit(); + int grandChildPid = int.Parse(childHandle.Process.StandardOutput.ReadLine()); + + // Obtain a Process instance before the child is killed to avoid PID reuse issues. + using Process grandchild = Process.GetProcessById(grandChildPid); - VerifyProcessIsRunning(enabled, grandChildPid); + try + { + childHandle.Process.Kill(); + + Assert.True(childHandle.Process.WaitForExit(WaitInMS)); + Assert.NotEqual(0, childHandle.Process.ExitCode); + // Use shorter wait time when the process is expected to survive + Assert.Equal(enabled, grandchild.WaitForExit(enabled ? WaitInMS : 300)); + } + finally + { + grandchild.Kill(); + } } [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -151,60 +180,23 @@ public void KillOnParentExit_KillsTheChild_WhenParentCrashes(bool enabled, bool restrictInheritance.ToString(), remoteInvokeOptions); - string firstLine = childHandle.Process.StandardOutput.ReadLine(); - int grandChildPid = int.Parse(firstLine); - childHandle.Process.StandardInput.WriteLine("One AccessViolationException please."); - childHandle.Process.WaitForExit(); - Assert.NotEqual(0, childHandle.Process.ExitCode); + int grandChildPid = int.Parse(childHandle.Process.StandardOutput.ReadLine()); - VerifyProcessIsRunning(enabled, grandChildPid); - } + // Obtain a Process instance before the child crashes to avoid PID reuse issues. + using Process grandchild = Process.GetProcessById(grandChildPid); - private static void VerifyProcessIsRunning(bool shouldBeKilled, int processId) - { - if (shouldBeKilled) + try { - const int timeoutMilliseconds = 10_000; - long deadline = Environment.TickCount64 + timeoutMilliseconds; + childHandle.Process.StandardInput.WriteLine("One AccessViolationException please."); - while (Environment.TickCount64 < deadline) - { - try - { - using Process process = Process.GetProcessById(processId); - if (process.HasExited || process.WaitForExit(100)) - { - return; - } - } - catch (ArgumentException) - { - return; - } - - Thread.Sleep(100); - } - - using Process finalCheck = Process.GetProcessById(processId); - Assert.True(finalCheck.HasExited || finalCheck.WaitForExit(0), - $"Process {processId} was expected to exit within {timeoutMilliseconds}ms."); + Assert.True(childHandle.Process.WaitForExit(WaitInMS)); + Assert.NotEqual(0, childHandle.Process.ExitCode); + // Use shorter wait time when the process is expected to survive + Assert.Equal(enabled, grandchild.WaitForExit(enabled ? WaitInMS : 300)); } - else + finally { - // Give the OS a moment to clean up - Thread.Sleep(500); - - // When KillOnParentExit is disabled, the process should still be running. - using Process process = Process.GetProcessById(processId); - try - { - Assert.False(process.HasExited); - } - finally - { - process.Kill(); - process.WaitForExit(); - } + grandchild.Kill(); } } } From c432cd583443aa41b3c55f7c4df895676cfad0c4 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 10 Apr 2026 18:36:21 +0200 Subject: [PATCH 12/23] refactor the code: move all logic related to extended startup info into one method --- .../SafeHandles/SafeProcessHandle.Windows.cs | 158 +++++++++--------- 1 file changed, 80 insertions(+), 78 deletions(-) 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 e46648b84e7c9c..721b4ba8240d9c 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 @@ -76,11 +76,10 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S bool stdinRefAdded = false, stdoutRefAdded = false, stderrRefAdded = false; bool restrictInheritedHandles = inheritedHandles is not null; bool killOnParentExit = startInfo.KillOnParentExit; - // We need extended startup info when we have inherited handles or when we need to pass - // a job list via PROC_THREAD_ATTRIBUTE_JOB_LIST (only for CreateProcess, not CreateProcessWithLogonW). - bool useExtendedStartupInfo = restrictInheritedHandles || (killOnParentExit && string.IsNullOrEmpty(startInfo.UserName)); + bool logon = !string.IsNullOrEmpty(startInfo.UserName); - // When InheritedHandles is set, we use PROC_THREAD_ATTRIBUTE_HANDLE_LIST to restrict inheritance. + // When InheritedHandles is set, we use PROC_THREAD_ATTRIBUTE_HANDLE_LIST to restrict inheritance + // or pass bInheritHandles=false when there are no valid handles to inherit. // For that, we need a reader lock (concurrent starts with different explicit lists are safe). // When InheritedHandles is not set, we use the existing approach with a writer lock. if (restrictInheritedHandles) @@ -103,7 +102,7 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S try { - startupInfoEx.StartupInfo.cb = useExtendedStartupInfo ? sizeof(Interop.Kernel32.STARTUPINFOEX) : sizeof(Interop.Kernel32.STARTUPINFO); + startupInfoEx.StartupInfo.cb = sizeof(Interop.Kernel32.STARTUPINFO); ProcessUtils.DuplicateAsInheritableIfNeeded(stdinHandle, ref startupInfoEx.StartupInfo.hStdInput, ref stdinRefAdded); ProcessUtils.DuplicateAsInheritableIfNeeded(stdoutHandle, ref startupInfoEx.StartupInfo.hStdOutput, ref stdoutRefAdded); @@ -119,7 +118,7 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S } // set up the creation flags parameter - int creationFlags = useExtendedStartupInfo ? Interop.Kernel32.EXTENDED_STARTUPINFO_PRESENT : 0; + int creationFlags = 0; if (startInfo.CreateNoWindow) creationFlags |= Interop.Advapi32.StartupInfoOptions.CREATE_NO_WINDOW; if (startInfo.CreateNewProcessGroup) creationFlags |= Interop.Advapi32.StartupInfoOptions.CREATE_NEW_PROCESS_GROUP; @@ -137,54 +136,22 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S workingDirectory = null; } - // When InheritedHandles is set, build a PROC_THREAD_ATTRIBUTE_HANDLE_LIST to restrict - // inheritance to only the explicitly specified handles. - int handleCount = 0; - if (restrictInheritedHandles) - { - int maxHandleCount = 3 + inheritedHandles!.Length; - handlesToInherit = (IntPtr*)NativeMemory.Alloc((nuint)maxHandleCount, (nuint)sizeof(IntPtr)); - Span handlesToInheritSpan = new Span(handlesToInherit, maxHandleCount); + // By default, all handles are inherited. + bool bInheritHandles = true; - // Add valid effective stdio handles (already made inheritable via DuplicateAsInheritableIfNeeded) - AddToInheritListIfValid(startupInfoEx.StartupInfo.hStdInput, handlesToInheritSpan, ref handleCount); - AddToInheritListIfValid(startupInfoEx.StartupInfo.hStdOutput, handlesToInheritSpan, ref handleCount); - AddToInheritListIfValid(startupInfoEx.StartupInfo.hStdError, handlesToInheritSpan, ref handleCount); - - EnableInheritanceAndAddRef(inheritedHandles, handlesToInheritSpan, ref handleCount, ref handlesToRelease); - } - - if (useExtendedStartupInfo) + // Extended Startup Info can be configured only for the non-logon path + if (!logon) { - // Determine the number of attributes we need to set in the proc thread attribute list. - int attributeCount = 0; - - // When InheritedHandles is set but handleCount is 0 (all standard handles are invalid), - // we pass bInheritHandles=false to CreateProcess and there is no need to use PROC_THREAD_ATTRIBUTE_HANDLE_LIST. - if (restrictInheritedHandles && handleCount > 0) - { - attributeCount++; // PROC_THREAD_ATTRIBUTE_HANDLE_LIST - } - if (killOnParentExit) - { - attributeCount++; // PROC_THREAD_ATTRIBUTE_JOB_LIST - } - - BuildProcThreadAttributeList( - handlesToInherit, - handleCount, - killOnParentExit, - attributeCount, - ref attributeListBuffer, + ConfigureExtendedStartupInfo(inheritedHandles, killOnParentExit, + ref startupInfoEx, ref creationFlags, ref attributeListBuffer, + ref handlesToInherit, ref handlesToRelease, ref bInheritHandles, ref jobHandles); } - startupInfoEx.lpAttributeList = attributeListBuffer; - bool retVal; int errorCode = 0; - if (startInfo.UserName.Length != 0) + if (logon) { if (startInfo.Password != null && startInfo.PasswordInClearText != null) { @@ -210,6 +177,7 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S // CreateProcessAsUserW supports both, but it's too restrictive and simply different than CreateProcessWithLogonW in many ways. Debug.Assert(!restrictInheritedHandles, "Inheriting handles is not supported when starting with alternate credentials."); Debug.Assert(startupInfoEx.StartupInfo.cb == sizeof(Interop.Kernel32.STARTUPINFO)); + Debug.Assert((creationFlags & Interop.Kernel32.EXTENDED_STARTUPINFO_PRESENT) == 0); // When KillOnParentExit is set and we use CreateProcessWithLogonW (which doesn't support // PROC_THREAD_ATTRIBUTE_JOB_LIST), we create the process suspended, assign it to the job, @@ -260,10 +228,6 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S fixed (char* environmentBlockPtr = environmentBlock) fixed (char* commandLinePtr = &commandLine.GetPinnableReference()) { - // When InheritedHandles is set but handleCount is 0 (e.g. empty list, no stdio), - // pass false to prevent all inheritable handles from leaking to the child. - bool bInheritHandles = !restrictInheritedHandles || handleCount > 0; - retVal = Interop.Kernel32.CreateProcess( null, // we don't need this since all the info is in commandLine commandLinePtr, // pointer to the command line string @@ -287,7 +251,7 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S // When the process was started suspended for KillOnParentExit with CreateProcessWithLogonW, // assign it to the job object and then resume the thread. - if (killOnParentExit && !string.IsNullOrEmpty(startInfo.UserName)) + if (killOnParentExit && logon) { AssignJobAndResumeThread(processInfo, procSH); } @@ -496,21 +460,61 @@ private static void AddToInheritListIfValid(nint handle, Span handlesToInh handlesToInherit[handleCount++] = handle; } - /// - /// Creates and populates a PROC_THREAD_ATTRIBUTE_LIST with optional PROC_THREAD_ATTRIBUTE_HANDLE_LIST - /// and PROC_THREAD_ATTRIBUTE_JOB_LIST entries. - /// - private static unsafe void BuildProcThreadAttributeList( - IntPtr* handlesToInherit, - int handleCount, - bool killOnParentExit, - int attributeCount, - ref void* attributeListBuffer, - ref IntPtr* jobHandles) + private static unsafe void ConfigureExtendedStartupInfo(SafeHandle[]? inheritedHandles, bool killOnParentExit, + ref Interop.Kernel32.STARTUPINFOEX startupInfoEx, ref int creationFlags, ref void* attributeListBuffer, + ref nint* handlesToInherit, ref SafeHandle?[]? handlesToRelease, ref bool bInheritHandles, + ref nint* jobHandles) { + if (inheritedHandles is null && !killOnParentExit) + { + return; + } + + // Determine the number of attributes we need to set in the proc thread attribute list. + int attributeCount = 0; + + int handleCount = 0; + if (inheritedHandles is not null) + { + int maxHandleCount = 3 + inheritedHandles.Length; + handlesToInherit = (IntPtr*)NativeMemory.Alloc((nuint)maxHandleCount, (nuint)sizeof(IntPtr)); + Span handlesToInheritSpan = new Span(handlesToInherit, maxHandleCount); + + // Add valid effective stdio handles (already made inheritable via DuplicateAsInheritableIfNeeded) + AddToInheritListIfValid(startupInfoEx.StartupInfo.hStdInput, handlesToInheritSpan, ref handleCount); + AddToInheritListIfValid(startupInfoEx.StartupInfo.hStdOutput, handlesToInheritSpan, ref handleCount); + AddToInheritListIfValid(startupInfoEx.StartupInfo.hStdError, handlesToInheritSpan, ref handleCount); + + EnableInheritanceAndAddRef(inheritedHandles, handlesToInheritSpan, ref handleCount, ref handlesToRelease); + + if (handleCount == 0) + { + // When InheritedHandles is set but handleCount is 0 (e.g. all standard handles are invalid), + // pass false to prevent all inheritable handles from leaking to the child. + bInheritHandles = false; + } + else + { + attributeCount++; // PROC_THREAD_ATTRIBUTE_HANDLE_LIST + } + } + + if (killOnParentExit) + { + jobHandles = (IntPtr*)NativeMemory.Alloc(1, (nuint)sizeof(IntPtr)); + jobHandles[0] = s_killOnParentExitJob.Value.DangerousGetHandle(); + + attributeCount++; // PROC_THREAD_ATTRIBUTE_JOB_LIST + } + + if (attributeCount == 0) + { + Debug.Assert(!bInheritHandles); + return; + } + nuint size = 0; Interop.Kernel32.InitializeProcThreadAttributeList(null, attributeCount, 0, ref size); - attributeListBuffer = NativeMemory.Alloc(size); if (!Interop.Kernel32.InitializeProcThreadAttributeList(attributeListBuffer, attributeCount, 0, ref size)) @@ -530,23 +534,21 @@ private static unsafe void BuildProcThreadAttributeList( throw new Win32Exception(Marshal.GetLastWin32Error()); } - if (killOnParentExit) + if (killOnParentExit && !Interop.Kernel32.UpdateProcThreadAttribute( + attributeListBuffer, + 0, + (IntPtr)Interop.Kernel32.PROC_THREAD_ATTRIBUTE_JOB_LIST, + jobHandles, + (nuint)sizeof(IntPtr), + null, + null)) { - jobHandles = (IntPtr*)NativeMemory.Alloc(1, (nuint)sizeof(IntPtr)); - jobHandles[0] = s_killOnParentExitJob.Value.DangerousGetHandle(); - - if (!Interop.Kernel32.UpdateProcThreadAttribute( - attributeListBuffer, - 0, - (IntPtr)Interop.Kernel32.PROC_THREAD_ATTRIBUTE_JOB_LIST, - jobHandles, - (nuint)sizeof(IntPtr), - null, - null)) - { - throw new Win32Exception(Marshal.GetLastWin32Error()); - } + throw new Win32Exception(Marshal.GetLastWin32Error()); } + + creationFlags |= Interop.Kernel32.EXTENDED_STARTUPINFO_PRESENT; + startupInfoEx.StartupInfo.cb = sizeof(Interop.Kernel32.STARTUPINFOEX); + startupInfoEx.lpAttributeList = attributeListBuffer; } private static void EnableInheritanceAndAddRef( From bbab88ff8bfbc344eb7201e9707b8a07727d57e2 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Fri, 10 Apr 2026 21:06:29 -0700 Subject: [PATCH 13/23] Update src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../System.Diagnostics.Process/tests/KillOnParentExitTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs index 303e58b405775d..222cffbaa37c74 100644 --- a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Runtime.InteropServices; -using System.Threading; using Microsoft.DotNet.RemoteExecutor; using Xunit; From baea4895a8336aaa81401da6ca3542ceb247991f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 11 Apr 2026 07:04:11 +0000 Subject: [PATCH 14/23] Move ResumeThread P/Invoke to dedicated file and remove redundant SkipOnPlatform attribute Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/d0fe3867-dd26-469a-9b34-4dc3b623b741 Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com> --- .../Interop/Windows/Kernel32/Interop.JobObjects.cs | 3 --- .../Windows/Kernel32/Interop.ResumeThread.cs | 14 ++++++++++++++ .../src/System.Diagnostics.Process.csproj | 2 ++ .../tests/KillOnParentExitTests.cs | 1 - 4 files changed, 16 insertions(+), 4 deletions(-) create mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ResumeThread.cs diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.JobObjects.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.JobObjects.cs index dc618f38262e64..72d0171f7af895 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.JobObjects.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.JobObjects.cs @@ -71,9 +71,6 @@ internal struct JOBOBJECT_EXTENDED_LIMIT_INFORMATION [return: MarshalAs(UnmanagedType.Bool)] internal static partial bool AssignProcessToJobObject(SafeJobHandle hJob, IntPtr hProcess); - [LibraryImport(Libraries.Kernel32, SetLastError = true)] - internal static partial int ResumeThread(IntPtr hThread); - internal const int PROC_THREAD_ATTRIBUTE_JOB_LIST = 0x0002000D; } } diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ResumeThread.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ResumeThread.cs new file mode 100644 index 00000000000000..0495f60a564f86 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ResumeThread.cs @@ -0,0 +1,14 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.InteropServices; + +internal static partial class Interop +{ + internal static partial class Kernel32 + { + [LibraryImport(Libraries.Kernel32, SetLastError = true)] + internal static partial int ResumeThread(IntPtr hThread); + } +} 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 5d5470473add44..2c1c9e0be2eb23 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -107,6 +107,8 @@ Link="Common\Interop\Windows\Kernel32\Interop.CreateProcess.cs" /> + Date: Sat, 11 Apr 2026 07:35:58 +0000 Subject: [PATCH 15/23] Use nuint for numerical fields in job object structs and fix processInfo.hProcess vs procSH consistency Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/09cb9d8c-1881-4dc2-80ac-f52f2a02112e Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com> --- .../Windows/Kernel32/Interop.JobObjects.cs | 16 ++++++++-------- .../SafeHandles/SafeProcessHandle.Windows.cs | 10 +++++----- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.JobObjects.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.JobObjects.cs index 72d0171f7af895..534c551443d3f6 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.JobObjects.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.JobObjects.cs @@ -44,10 +44,10 @@ internal struct JOBOBJECT_BASIC_LIMIT_INFORMATION internal long PerProcessUserTimeLimit; internal long PerJobUserTimeLimit; internal uint LimitFlags; - internal UIntPtr MinimumWorkingSetSize; - internal UIntPtr MaximumWorkingSetSize; + internal nuint MinimumWorkingSetSize; + internal nuint MaximumWorkingSetSize; internal uint ActiveProcessLimit; - internal UIntPtr Affinity; + internal nuint Affinity; internal uint PriorityClass; internal uint SchedulingClass; } @@ -57,10 +57,10 @@ internal struct JOBOBJECT_EXTENDED_LIMIT_INFORMATION { internal JOBOBJECT_BASIC_LIMIT_INFORMATION BasicLimitInformation; internal IO_COUNTERS IoInfo; - internal UIntPtr ProcessMemoryLimit; - internal UIntPtr JobMemoryLimit; - internal UIntPtr PeakProcessMemoryUsed; - internal UIntPtr PeakJobMemoryUsed; + internal nuint ProcessMemoryLimit; + internal nuint JobMemoryLimit; + internal nuint PeakProcessMemoryUsed; + internal nuint PeakJobMemoryUsed; } [LibraryImport(Libraries.Kernel32, SetLastError = true)] @@ -69,7 +69,7 @@ internal struct JOBOBJECT_EXTENDED_LIMIT_INFORMATION [LibraryImport(Libraries.Kernel32, SetLastError = true)] [return: MarshalAs(UnmanagedType.Bool)] - internal static partial bool AssignProcessToJobObject(SafeJobHandle hJob, IntPtr hProcess); + internal static partial bool AssignProcessToJobObject(SafeJobHandle hJob, SafeProcessHandle hProcess); internal const int PROC_THREAD_ATTRIBUTE_JOB_LIST = 0x0002000D; } 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 bdd7ce8f231e67..f3615785243bfb 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 @@ -254,7 +254,7 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S // assign it to the job object and then resume the thread. if (killOnParentExit && logon) { - AssignJobAndResumeThread(processInfo, procSH); + AssignJobAndResumeThread(processInfo.hThread, procSH); } } @@ -601,18 +601,18 @@ private static void DisableInheritanceAndRelease(SafeHandle?[] handlesToRelease) } } - private static void AssignJobAndResumeThread(Interop.Kernel32.PROCESS_INFORMATION processInfo, SafeProcessHandle procSH) + private static void AssignJobAndResumeThread(IntPtr hThread, SafeProcessHandle procSH) { - Debug.Assert(!IsInvalidHandle(processInfo.hThread), "Thread handle must be valid for suspended process."); + Debug.Assert(hThread != IntPtr.Zero, "Thread handle must be valid for suspended process."); try { - if (!Interop.Kernel32.AssignProcessToJobObject(s_killOnParentExitJob.Value, processInfo.hProcess)) + if (!Interop.Kernel32.AssignProcessToJobObject(s_killOnParentExitJob.Value, procSH)) { throw new Win32Exception(Marshal.GetLastWin32Error()); } - if (Interop.Kernel32.ResumeThread(processInfo.hThread) == -1) + if (Interop.Kernel32.ResumeThread(hThread) == -1) { throw new Win32Exception(Marshal.GetLastWin32Error()); } From 04d67a92276ec16fab4396823a3de8d72e2f99a4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 11 Apr 2026 07:50:22 +0000 Subject: [PATCH 16/23] Change ResumeThread return type to uint (matches DWORD) and remove ineffective HELIX env var Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/a6c175e9-c76c-45ff-978c-cf037751d56b Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com> --- .../Common/src/Interop/Windows/Kernel32/Interop.ResumeThread.cs | 2 +- .../Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs | 2 +- .../System.Diagnostics.Process/tests/KillOnParentExitTests.cs | 2 -- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ResumeThread.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ResumeThread.cs index 0495f60a564f86..9da3a07bf2f451 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ResumeThread.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ResumeThread.cs @@ -9,6 +9,6 @@ internal static partial class Interop internal static partial class Kernel32 { [LibraryImport(Libraries.Kernel32, SetLastError = true)] - internal static partial int ResumeThread(IntPtr hThread); + internal static partial uint ResumeThread(IntPtr hThread); } } 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 f3615785243bfb..896f0d2b42361d 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 @@ -612,7 +612,7 @@ private static void AssignJobAndResumeThread(IntPtr hThread, SafeProcessHandle p throw new Win32Exception(Marshal.GetLastWin32Error()); } - if (Interop.Kernel32.ResumeThread(hThread) == -1) + if (Interop.Kernel32.ResumeThread(hThread) == 0xFFFFFFFF) { throw new Win32Exception(Marshal.GetLastWin32Error()); } diff --git a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs index 4b1286a4ed0b85..37f31b0831732c 100644 --- a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs @@ -155,8 +155,6 @@ public void KillOnParentExit_KillsTheChild_WhenParentCrashes(bool enabled, bool remoteInvokeOptions.StartInfo.RedirectStandardOutput = true; remoteInvokeOptions.StartInfo.RedirectStandardError = true; remoteInvokeOptions.StartInfo.RedirectStandardInput = true; - // Don't create (and upload) a memory dump for this test, as AV is intentional and expected. - remoteInvokeOptions.StartInfo.Environment["HELIX_WORKITEM_UPLOAD_ROOT"] = null; using RemoteInvokeHandle childHandle = RemoteExecutor.Invoke( (enabledStr, limitInheritanceStr) => From 20559e0ec04e0d308810041cad103978a2f88bcb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 11 Apr 2026 08:01:31 +0000 Subject: [PATCH 17/23] Use IsInvalidHandle for thread handle assertion (checks both 0 and -1) Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/fbffe836-d31a-44ba-a420-e7f63c24df12 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 896f0d2b42361d..1ce9c95bc26d4a 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 @@ -603,7 +603,7 @@ private static void DisableInheritanceAndRelease(SafeHandle?[] handlesToRelease) private static void AssignJobAndResumeThread(IntPtr hThread, SafeProcessHandle procSH) { - Debug.Assert(hThread != IntPtr.Zero, "Thread handle must be valid for suspended process."); + Debug.Assert(!IsInvalidHandle(hThread), "Thread handle must be valid for suspended process."); try { From 35b273ce8085fa1adf90026ca2a57cba0365934f Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Sat, 11 Apr 2026 10:31:58 +0200 Subject: [PATCH 18/23] try to avoid creating a memory dump --- .../tests/KillOnParentExitTests.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs index 37f31b0831732c..770dd4379f62f3 100644 --- a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs @@ -149,6 +149,7 @@ public void KillOnParentExit_KillsTheChild_WhenParentIsKilled(bool enabled, bool [InlineData(true, true)] [InlineData(true, false)] [InlineData(false, false)] // (false, true) is tested by ProcessHandleTests + [OuterLoop("May create a memory dump")] public void KillOnParentExit_KillsTheChild_WhenParentCrashes(bool enabled, bool restrictInheritance) { RemoteInvokeOptions remoteInvokeOptions = new() { CheckExitCode = false }; @@ -156,6 +157,11 @@ public void KillOnParentExit_KillsTheChild_WhenParentCrashes(bool enabled, bool remoteInvokeOptions.StartInfo.RedirectStandardError = true; remoteInvokeOptions.StartInfo.RedirectStandardInput = true; + // RemoteExecutor creates a memory dump when the child process does not exit on time. + // Until we have a dedicated API to control this behavior, + // we can disable dump creation by setting the upload root to null. + remoteInvokeOptions.StartInfo.Environment["HELIX_WORKITEM_UPLOAD_ROOT"] = null; + using RemoteInvokeHandle childHandle = RemoteExecutor.Invoke( (enabledStr, limitInheritanceStr) => { From 8beb0267f96dc4904ece068e98cdf048d9196b8b Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Sat, 11 Apr 2026 10:46:31 +0200 Subject: [PATCH 19/23] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../System.Diagnostics.Process/tests/KillOnParentExitTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs index 770dd4379f62f3..cb16474135bb0d 100644 --- a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs @@ -154,7 +154,6 @@ public void KillOnParentExit_KillsTheChild_WhenParentCrashes(bool enabled, bool { RemoteInvokeOptions remoteInvokeOptions = new() { CheckExitCode = false }; remoteInvokeOptions.StartInfo.RedirectStandardOutput = true; - remoteInvokeOptions.StartInfo.RedirectStandardError = true; remoteInvokeOptions.StartInfo.RedirectStandardInput = true; // RemoteExecutor creates a memory dump when the child process does not exit on time. From c5b7d5904e5bd41192b5da7ebc0c141c440d58b5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 11 Apr 2026 14:19:57 +0000 Subject: [PATCH 20/23] Replace local ResumeThread P/Invoke in ProcessHandlesTests with shared Interop.Kernel32.ResumeThread Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/e8bb36bc-5c71-4723-a32c-17e87d762370 Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com> --- .../tests/ProcessHandlesTests.Windows.cs | 5 +---- .../tests/System.Diagnostics.Process.Tests.csproj | 2 ++ 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.Windows.cs index 3be363eee92233..b8955d8391efe0 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.Windows.cs @@ -157,7 +157,7 @@ private unsafe int RunWithInvalidHandles(ProcessStartInfo startInfo) // As soon as SafeProcessHandle.WaitForExit* are implemented (#126293), we can use them instead. using Process process = Process.GetProcessById(processInfo.dwProcessId); - if (ResumeThread(processInfo.hThread) == -1) + if (Interop.Kernel32.ResumeThread(processInfo.hThread) == 0xFFFFFFFF) { throw new Win32Exception(); } @@ -183,9 +183,6 @@ private unsafe int RunWithInvalidHandles(ProcessStartInfo startInfo) } } - [LibraryImport(Interop.Libraries.Kernel32)] - private static partial int ResumeThread(nint hThread); - private static unsafe string GetSafeFileHandleId(SafeFileHandle handle) { const int MaxPath = 32_767; diff --git a/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj b/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj index cf86449ac47e83..5c345d299a1ac8 100644 --- a/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj +++ b/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj @@ -71,6 +71,8 @@ Link="Common\Interop\Windows\Kernel32\Interop.SetConsoleCtrlHandler.cs" /> + From 84ca617dab5f5f0478ddac85b89b7bffb949ac39 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 11 Apr 2026 14:21:01 +0000 Subject: [PATCH 21/23] Remove unused System.Runtime.InteropServices using after removing local ResumeThread P/Invoke Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/e8bb36bc-5c71-4723-a32c-17e87d762370 Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com> --- .../tests/ProcessHandlesTests.Windows.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.Windows.cs index b8955d8391efe0..92d2c7bfde2fbe 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.Windows.cs @@ -5,7 +5,6 @@ using System.ComponentModel; using System.IO; using System.IO.Pipes; -using System.Runtime.InteropServices; using System.Threading.Tasks; using Microsoft.DotNet.RemoteExecutor; using Microsoft.DotNet.XUnitExtensions; From 96e280489539bcdc7561973b0503d5b022040956 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 11 Apr 2026 17:16:04 +0000 Subject: [PATCH 22/23] Remove defensive HELIX_WORKITEM_UPLOAD_ROOT env var from crash test Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/f3703368-dbc7-4226-8258-1bb793d93faf Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../tests/KillOnParentExitTests.cs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs index cb16474135bb0d..34a100e7e80bbf 100644 --- a/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs @@ -156,11 +156,6 @@ public void KillOnParentExit_KillsTheChild_WhenParentCrashes(bool enabled, bool remoteInvokeOptions.StartInfo.RedirectStandardOutput = true; remoteInvokeOptions.StartInfo.RedirectStandardInput = true; - // RemoteExecutor creates a memory dump when the child process does not exit on time. - // Until we have a dedicated API to control this behavior, - // we can disable dump creation by setting the upload root to null. - remoteInvokeOptions.StartInfo.Environment["HELIX_WORKITEM_UPLOAD_ROOT"] = null; - using RemoteInvokeHandle childHandle = RemoteExecutor.Invoke( (enabledStr, limitInheritanceStr) => { From f497166baed158b3db40332f35029ca454730fc8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 11 Apr 2026 19:34:27 +0000 Subject: [PATCH 23/23] Address jkotas feedback: remove unused enum, unnecessary early return, unnecessary assert, reduce ConfigureExtendedStartupInfo args Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/2dd25b7c-da4a-4ed0-a9b3-cc6561f14aeb Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Windows/Kernel32/Interop.JobObjects.cs | 1 - .../SafeHandles/SafeProcessHandle.Windows.cs | 27 +++++++++---------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.JobObjects.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.JobObjects.cs index 534c551443d3f6..31300bc6adc5e7 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.JobObjects.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.JobObjects.cs @@ -23,7 +23,6 @@ public SafeJobHandle() : base(true) { } internal enum JOBOBJECTINFOCLASS { - JobObjectBasicLimitInformation = 2, JobObjectExtendedLimitInformation = 9 } 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 1ce9c95bc26d4a..29e915a0803c0b 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 @@ -143,10 +143,15 @@ internal static unsafe SafeProcessHandle StartCore(ProcessStartInfo startInfo, S // Extended Startup Info can be configured only for the non-logon path if (!logon) { - ConfigureExtendedStartupInfo(inheritedHandles, killOnParentExit, - ref startupInfoEx, ref creationFlags, ref attributeListBuffer, + if (ConfigureExtendedStartupInfo(inheritedHandles, killOnParentExit, + in startupInfoEx, ref attributeListBuffer, ref handlesToInherit, ref handlesToRelease, ref bInheritHandles, - ref jobHandles); + ref jobHandles)) + { + creationFlags |= Interop.Kernel32.EXTENDED_STARTUPINFO_PRESENT; + startupInfoEx.StartupInfo.cb = sizeof(Interop.Kernel32.STARTUPINFOEX); + startupInfoEx.lpAttributeList = attributeListBuffer; + } } bool retVal; @@ -461,16 +466,11 @@ private static void AddToInheritListIfValid(nint handle, Span handlesToInh handlesToInherit[handleCount++] = handle; } - private static unsafe void ConfigureExtendedStartupInfo(SafeHandle[]? inheritedHandles, bool killOnParentExit, - ref Interop.Kernel32.STARTUPINFOEX startupInfoEx, ref int creationFlags, ref void* attributeListBuffer, + private static unsafe bool ConfigureExtendedStartupInfo(SafeHandle[]? inheritedHandles, bool killOnParentExit, + in Interop.Kernel32.STARTUPINFOEX startupInfoEx, ref void* attributeListBuffer, ref nint* handlesToInherit, ref SafeHandle?[]? handlesToRelease, ref bool bInheritHandles, ref nint* jobHandles) { - if (inheritedHandles is null && !killOnParentExit) - { - return; - } - // Determine the number of attributes we need to set in the proc thread attribute list. int attributeCount = 0; @@ -510,8 +510,7 @@ private static unsafe void ConfigureExtendedStartupInfo(SafeHandle[]? inheritedH if (attributeCount == 0) { - Debug.Assert(!bInheritHandles); - return; + return false; } nuint size = 0; @@ -547,9 +546,7 @@ private static unsafe void ConfigureExtendedStartupInfo(SafeHandle[]? inheritedH throw new Win32Exception(Marshal.GetLastWin32Error()); } - creationFlags |= Interop.Kernel32.EXTENDED_STARTUPINFO_PRESENT; - startupInfoEx.StartupInfo.cb = sizeof(Interop.Kernel32.STARTUPINFOEX); - startupInfoEx.lpAttributeList = attributeListBuffer; + return true; } private static void EnableInheritanceAndAddRef(