Avoid throwing exceptions in Process.KillTree during process enumeration (#123865)

Fixes #121279

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: jkotas <[email protected]>
Co-authored-by: stephentoub <[email protected]>

Author: 3 people

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs

@@ -328,13 +328,65 @@ private bool WaitForInputIdleCore(int milliseconds)
         /// </remarks>
         private bool IsParentOf(Process possibleChild)
         {
-            try
+            // Use non-throwing helpers to avoid first-chance exceptions during enumeration.
+            // This is critical for performance when a debugger is attached.
+            if (!TryGetStartTime(out DateTime myStartTime) ||
+                !possibleChild.TryGetStartTime(out DateTime childStartTime) ||
+                !possibleChild.TryGetParentProcessId(out int childParentId))
             {
-                return StartTime < possibleChild.StartTime && Id == possibleChild.ParentProcessId;
+                return false;
             }
-            catch (Exception e) when (IsProcessInvalidException(e))
+
+            return myStartTime < childStartTime && Id == childParentId;
+        }
+
+        /// <summary>
+        /// Try to get the process start time without throwing exceptions.
+        /// </summary>
+        private bool TryGetStartTime(out DateTime startTime)
+        {
+            startTime = default;
+            using (SafeProcessHandle handle = GetProcessHandle(Interop.Advapi32.ProcessOptions.PROCESS_QUERY_LIMITED_INFORMATION, false))
             {
-                return false;
+                if (handle.IsInvalid)
+                {
+                    return false;
+                }
+
+                ProcessThreadTimes processTimes = new ProcessThreadTimes();
+                if (!Interop.Kernel32.GetProcessTimes(handle,
+                    out processTimes._create, out processTimes._exit,
+                    out processTimes._kernel, out processTimes._user))
+                {
+                    return false;
+                }
+
+                startTime = processTimes.StartTime;
+                return true;
+            }
+        }
+
+        /// <summary>
+        /// Try to get the parent process ID without throwing exceptions.
+        /// </summary>
+        private unsafe bool TryGetParentProcessId(out int parentProcessId)
+        {
+            parentProcessId = 0;
+            using (SafeProcessHandle handle = GetProcessHandle(Interop.Advapi32.ProcessOptions.PROCESS_QUERY_LIMITED_INFORMATION, false))
+            {
+                if (handle.IsInvalid)
+                {
+                    return false;
+                }
+
+                Interop.NtDll.PROCESS_BASIC_INFORMATION info;
+                if (Interop.NtDll.NtQueryInformationProcess(handle, Interop.NtDll.ProcessBasicInformation, &info, (uint)sizeof(Interop.NtDll.PROCESS_BASIC_INFORMATION), out _) != 0)
+                {
+                    return false;
+                }
+
+                parentProcessId = (int)info.InheritedFromUniqueProcessId;
+                return true;
             }
         }
 
@@ -359,14 +411,20 @@ private unsafe int ParentProcessId
 
         private bool Equals(Process process)
         {
-            try
+            // Check IDs first since they're cheap to compare and will fail most of the time.
+            if (Id != process.Id)
             {
-                return Id == process.Id && StartTime == process.StartTime;
+                return false;
             }
-            catch (Exception e) when (IsProcessInvalidException(e))
+
+            // Use non-throwing helper to avoid first-chance exceptions during enumeration.
+            if (!TryGetStartTime(out DateTime myStartTime) ||
+                !process.TryGetStartTime(out DateTime otherStartTime))
             {
                 return false;
             }
+
+            return myStartTime == otherStartTime;
         }
 
         private List<Exception>? KillTree()

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Windows.cs

@@ -236,7 +236,7 @@ static unsafe ProcessManager()
             }
         }
 
-        public static SafeProcessHandle OpenProcess(int processId, int access, bool throwIfExited)
+        public static SafeProcessHandle OpenProcess(int processId, int access, bool throwOnError)
         {
             SafeProcessHandle processHandle = Interop.Kernel32.OpenProcess(access, false, processId);
             int result = Marshal.GetLastWin32Error();
@@ -247,24 +247,21 @@ public static SafeProcessHandle OpenProcess(int processId, int access, bool thro
 
             processHandle.Dispose();
 
+            if (!throwOnError)
+            {
+                return SafeProcessHandle.InvalidHandle;
+            }
+
             if (processId == 0)
             {
                 throw new Win32Exception(Interop.Errors.ERROR_ACCESS_DENIED);
             }
 
-            // If the handle is invalid because the process has exited, only throw an exception if throwIfExited is true.
-            // Assume the process is still running if the error was ERROR_ACCESS_DENIED for better performance
-            if (result != Interop.Errors.ERROR_ACCESS_DENIED && !IsProcessRunning(processId))
+            if (!IsProcessRunning(processId))
             {
-                if (throwIfExited)
-                {
-                    throw new InvalidOperationException(SR.Format(SR.ProcessHasExited, processId.ToString()));
-                }
-                else
-                {
-                    return SafeProcessHandle.InvalidHandle;
-                }
+                throw new InvalidOperationException(SR.Format(SR.ProcessHasExited, processId.ToString()));
             }
+
             throw new Win32Exception(result);
         }
 

src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs

@@ -5,6 +5,8 @@
 using System.ComponentModel;
 using System.IO;
 using System.Runtime.InteropServices;
+using System.Threading;
+using Microsoft.DotNet.RemoteExecutor;
 using Microsoft.DotNet.XUnitExtensions;
 using Xunit;
 
@@ -55,5 +57,98 @@ private static unsafe void ReEnableCtrlCHandlerIfNeeded(PosixSignal signal)
                 }
             }
         }
+
+        [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
+        public void Kill_EntireProcessTree_MinimalExceptions()
+        {
+            // This test validates that Kill(true) doesn't throw excessive exceptions internally
+            // during process enumeration, which causes severe performance degradation with debugger attached.
+            // See https://github.com/dotnet/runtime/issues/121279
+            
+            int exceptionCount = 0;
+            EventHandler<System.Runtime.ExceptionServices.FirstChanceExceptionEventArgs> handler = 
+                (sender, e) => Interlocked.Increment(ref exceptionCount);
+            
+            AppDomain.CurrentDomain.FirstChanceException += handler;
+            
+            try
+            {
+                // Create a process tree based on the repro from the issue:
+                // Main process spawns child1, child1 spawns child2
+                RemoteInvokeHandle handle = RemoteExecutor.Invoke(CreateProcessTreeAndWait);
+                Process parentProcess = handle.Process;
+
+                try
+                {
+                    // Wait a bit to ensure the process tree is established
+                    Thread.Sleep(1500);
+                    
+                    // Reset the counter before killing
+                    exceptionCount = 0;
+                    
+                    // Kill the entire process tree
+                    parentProcess.Kill(entireProcessTree: true);
+                    
+                    // Wait for process to exit
+                    Assert.True(parentProcess.WaitForExit(Helpers.PassingTestTimeoutMilliseconds));
+                    
+                    // The fix should ensure that we don't throw exceptions for each system process
+                    // that we can't access during enumeration. Allow up to 5 exceptions for edge cases
+                    // (e.g., processes that exit during enumeration, rare error conditions)
+                    Assert.True(exceptionCount <= 5, 
+                        $"Expected no more than 5 exceptions during Kill(true), but got {exceptionCount}");
+                }
+                finally
+                {
+                    try
+                    {
+                        if (!parentProcess.HasExited)
+                        {
+                            parentProcess.Kill();
+                        }
+                        handle.Dispose();
+                    }
+                    catch
+                    {
+                        // Best effort cleanup
+                    }
+                }
+            }
+            finally
+            {
+                AppDomain.CurrentDomain.FirstChanceException -= handler;
+            }
+        }
+
+        private static int CreateProcessTreeAndWait()
+        {
+            // This mimics the repro code from the issue
+            // Create child1 which will create child2
+            ProcessStartInfo psi = new ProcessStartInfo
+            {
+                FileName = RemoteExecutor.HostRunner,
+                Arguments = $"exec \"{RemoteExecutor.Path}\" {typeof(ProcessTests).Assembly.Location} {nameof(SleepForever)}",
+                UseShellExecute = false,
+                RedirectStandardOutput = true,
+                RedirectStandardError = true,
+            };
+
+            using (Process child1 = Process.Start(psi))
+            {
+                // Wait a bit to ensure child1 is running
+                Thread.Sleep(500);
+                
+                // Keep this process alive
+                Thread.Sleep(Timeout.Infinite);
+            }
+            
+            return RemoteExecutor.SuccessExitCode;
+        }
+
+        private static int SleepForever()
+        {
+            Thread.Sleep(Timeout.Infinite);
+            return RemoteExecutor.SuccessExitCode;
+        }
     }
 }