From 80c6a19278e796aaab9bf0451256e5e09d08ae5e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Apr 2026 07:33:51 +0000 Subject: [PATCH 1/4] Initial plan From a397d0283c1c85e03a24d440584cb0109c88521e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Apr 2026 08:23:26 +0000 Subject: [PATCH 2/4] Fix race condition: set _canceled under _gate lock in ProcessWaitState The test ProcessSafeHandle_WaitForExitOrKillOnCancellationAsync_KillsOnCancellation was failing intermittently because _canceled was being set outside the _gate lock, while ChildReaped reads it under the lock to build ProcessExitStatus. This caused a race where ChildReaped could read _canceled as false before the cancellation callback set it to true. Changes: - Add Cancel() method to SafeProcessHandle (platform-specific implementations) - On Unix, Cancel() delegates to ProcessWaitState.Cancel() which acquires _gate before setting _canceled, ensuring atomicity with ChildReaped - On Windows, Cancel() wraps the existing SignalCore + Canceled property set - Remove volatile from _canceled field since it's now protected by the lock - Update all call sites to use Cancel() instead of directly setting Canceled Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/d23310ac-167d-4a45-a389-45918504a964 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Win32/SafeHandles/SafeProcessHandle.Unix.cs | 9 +++------ .../Win32/SafeHandles/SafeProcessHandle.Windows.cs | 11 ++++++++--- .../Win32/SafeHandles/SafeProcessHandle.cs | 2 +- .../src/System/Diagnostics/ProcessWaitState.Unix.cs | 13 ++++++++++++- 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs index 1b3c2e9918e3d6..9074ba9f6deb98 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs @@ -56,12 +56,9 @@ public int ProcessId } } - /// - /// Sets a value indicating whether the process has been terminated due to timeout or cancellation. - /// - private bool Canceled + private void Cancel() { - set => GetWaitState()._canceled = value; + GetWaitState().Cancel(this); } protected override bool ReleaseHandle() @@ -132,7 +129,7 @@ private ProcessExitStatus WaitForExitOrKillOnTimeoutCore(int milliseconds) if (!waitState.WaitForExit(milliseconds)) { - waitState._canceled = SignalCore(PosixSignal.SIGKILL); + waitState.Cancel(this); waitState.WaitForExit(Timeout.Infinite); } 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 fe53e2c07de417..8fa6bea697099d 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 @@ -46,6 +46,13 @@ public int ProcessId /// private bool Canceled { get; set; } + private void Cancel() + { +#pragma warning disable CA1416 // PosixSignal.SIGKILL is supported on Windows via SignalCore + Canceled = SignalCore(PosixSignal.SIGKILL); +#pragma warning restore CA1416 + } + protected override bool ReleaseHandle() { return Interop.Kernel32.CloseHandle(handle); @@ -673,9 +680,7 @@ private ProcessExitStatus WaitForExitOrKillOnTimeoutCore(int milliseconds) using Interop.Kernel32.ProcessWaitHandle processWaitHandle = new(this); if (!processWaitHandle.WaitOne(milliseconds)) { -#pragma warning disable CA1416 // PosixSignal.SIGKILL is supported on Windows via SignalCore - Canceled = SignalCore(PosixSignal.SIGKILL); -#pragma warning restore CA1416 + Cancel(); processWaitHandle.WaitOne(Timeout.Infinite); } diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs index fb5de007dbe4b5..2a2b4c4cad2743 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs @@ -343,7 +343,7 @@ public async Task WaitForExitOrKillOnCancellationAsync(Cancel var (handle, tcs) = ((SafeProcessHandle, TaskCompletionSource))state!; try { - handle.Canceled = handle.SignalCore(PosixSignal.SIGKILL); + handle.Cancel(); } catch (Exception ex) { diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs index 90bda5bcf07f1c..4834751f08a63c 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs @@ -5,6 +5,7 @@ using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; +using Microsoft.Win32.SafeHandles; namespace System.Diagnostics { @@ -213,7 +214,17 @@ internal void ReleaseRef() /// Associated process is a child that can use the terminal. private readonly bool _usesTerminal; /// A value indicating whether the process has been terminated due to timeout or cancellation. - internal volatile bool _canceled; + internal bool _canceled; + + internal void Cancel(SafeProcessHandle process) + { + lock (_gate) + { +#pragma warning disable CA1416 // Signal is not supported on iOS/tvOS but Cancel is only called from supported platforms + _canceled = process.Signal(PosixSignal.SIGKILL); +#pragma warning restore CA1416 + } + } /// An in-progress or completed wait operation. /// A completed task does not mean the process has exited. From 8c30a44005cacd166e21a4ab18631ded831e2b42 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Apr 2026 10:13:36 +0000 Subject: [PATCH 3/4] Make _canceled field private in ProcessWaitState Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/22fb6e3e-d0e3-4cf8-91b3-5b227dbbc8a4 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/ProcessWaitState.Unix.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs index 4834751f08a63c..c5a0b6d1ede77b 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs @@ -214,7 +214,7 @@ internal void ReleaseRef() /// Associated process is a child that can use the terminal. private readonly bool _usesTerminal; /// A value indicating whether the process has been terminated due to timeout or cancellation. - internal bool _canceled; + private bool _canceled; internal void Cancel(SafeProcessHandle process) { From 6bc725192e38b3304cee46c5267380ac321948d9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Apr 2026 11:21:16 +0000 Subject: [PATCH 4/4] Revert lock-based approach; set _canceled before SignalCore instead Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/e3d7eda5-6eea-46ce-8caf-8dbe57c1b899 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Win32/SafeHandles/SafeProcessHandle.Unix.cs | 10 +++++++--- .../Win32/SafeHandles/SafeProcessHandle.Windows.cs | 12 ++++-------- .../Win32/SafeHandles/SafeProcessHandle.cs | 3 ++- .../src/System/Diagnostics/ProcessWaitState.Unix.cs | 13 +------------ 4 files changed, 14 insertions(+), 24 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs index 9074ba9f6deb98..0b48b5e9d1e191 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs @@ -56,9 +56,12 @@ public int ProcessId } } - private void Cancel() + /// + /// Sets a value indicating whether the process has been terminated due to timeout or cancellation. + /// + private bool Canceled { - GetWaitState().Cancel(this); + set => GetWaitState()._canceled = value; } protected override bool ReleaseHandle() @@ -129,7 +132,8 @@ private ProcessExitStatus WaitForExitOrKillOnTimeoutCore(int milliseconds) if (!waitState.WaitForExit(milliseconds)) { - waitState.Cancel(this); + waitState._canceled = true; + SignalCore(PosixSignal.SIGKILL); waitState.WaitForExit(Timeout.Infinite); } 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 8fa6bea697099d..6410a78817e170 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 @@ -46,13 +46,6 @@ public int ProcessId /// private bool Canceled { get; set; } - private void Cancel() - { -#pragma warning disable CA1416 // PosixSignal.SIGKILL is supported on Windows via SignalCore - Canceled = SignalCore(PosixSignal.SIGKILL); -#pragma warning restore CA1416 - } - protected override bool ReleaseHandle() { return Interop.Kernel32.CloseHandle(handle); @@ -680,7 +673,10 @@ private ProcessExitStatus WaitForExitOrKillOnTimeoutCore(int milliseconds) using Interop.Kernel32.ProcessWaitHandle processWaitHandle = new(this); if (!processWaitHandle.WaitOne(milliseconds)) { - Cancel(); +#pragma warning disable CA1416 // PosixSignal.SIGKILL is supported on Windows via SignalCore + Canceled = true; + SignalCore(PosixSignal.SIGKILL); +#pragma warning restore CA1416 processWaitHandle.WaitOne(Timeout.Infinite); } diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs index 2a2b4c4cad2743..1ea7ecfd94cbd0 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs @@ -343,7 +343,8 @@ public async Task WaitForExitOrKillOnCancellationAsync(Cancel var (handle, tcs) = ((SafeProcessHandle, TaskCompletionSource))state!; try { - handle.Cancel(); + handle.Canceled = true; + handle.SignalCore(PosixSignal.SIGKILL); } catch (Exception ex) { diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs index c5a0b6d1ede77b..b2c8445a3b9006 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs @@ -5,7 +5,6 @@ using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; -using Microsoft.Win32.SafeHandles; namespace System.Diagnostics { @@ -214,17 +213,7 @@ internal void ReleaseRef() /// Associated process is a child that can use the terminal. private readonly bool _usesTerminal; /// A value indicating whether the process has been terminated due to timeout or cancellation. - private bool _canceled; - - internal void Cancel(SafeProcessHandle process) - { - lock (_gate) - { -#pragma warning disable CA1416 // Signal is not supported on iOS/tvOS but Cancel is only called from supported platforms - _canceled = process.Signal(PosixSignal.SIGKILL); -#pragma warning restore CA1416 - } - } + internal bool _canceled; /// An in-progress or completed wait operation. /// A completed task does not mean the process has exited.