From 8d4df788064ee6374d7ae9fc2ab8ec4eb30a1799 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BD=C5=AFrek?= <188900745+mrek-msft@users.noreply.github.com> Date: Thu, 26 Feb 2026 13:22:34 +0100 Subject: [PATCH 01/22] Register CtrlHandler only once on Windows to prevent AB/BA deadlock between runtime and kernelbase. --- .../PosixSignalRegistration.Windows.cs | 27 ++++++++----------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs index b176506f042af6..0e14e8eb4511ae 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs @@ -9,6 +9,7 @@ namespace System.Runtime.InteropServices public sealed partial class PosixSignalRegistration { private static readonly Dictionary> s_registrations = new(); + private static bool s_isCtrlHandlerRegisteredOnce; private static unsafe PosixSignalRegistration Register(PosixSignal signal, Action handler) { @@ -23,13 +24,14 @@ private static unsafe PosixSignalRegistration Register(PosixSignal signal, Actio var token = new Token(signal, signo, handler); var registration = new PosixSignalRegistration(token); + bool registerCtrlHandler = false; lock (s_registrations) { - if (s_registrations.Count == 0 && - !Interop.Kernel32.SetConsoleCtrlHandler(&HandlerRoutine, Add: true)) + if (s_registrations.Count == 0 && !s_isCtrlHandlerRegisteredOnce) { - throw Win32Marshal.GetExceptionForLastWin32Error(); + s_isCtrlHandlerRegisteredOnce = true; + registerCtrlHandler = true; } if (!s_registrations.TryGetValue(signo, out List? tokens)) @@ -40,6 +42,12 @@ private static unsafe PosixSignalRegistration Register(PosixSignal signal, Actio tokens.Add(token); } + if (registerCtrlHandler && + !Interop.Kernel32.SetConsoleCtrlHandler(&HandlerRoutine, Add: true)) + { + throw Win32Marshal.GetExceptionForLastWin32Error(); + } + return registration; } @@ -58,19 +66,6 @@ private unsafe void Unregister() { s_registrations.Remove(token.SigNo); } - - if (s_registrations.Count == 0 && - !Interop.Kernel32.SetConsoleCtrlHandler(&HandlerRoutine, Add: false)) - { - // Ignore errors due to the handler no longer being registered; this can happen, for example, with - // direct use of Alloc/Attach/FreeConsole which result in the table of control handlers being reset. - // Throw for everything else. - int error = Marshal.GetLastPInvokeError(); - if (error != Interop.Errors.ERROR_INVALID_PARAMETER) - { - throw Win32Marshal.GetExceptionForWin32Error(error); - } - } } } } From 346a77b7dfd56e284880b7efc9c0b1bcc61ff17f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BD=C5=AFrek?= <188900745+mrek-msft@users.noreply.github.com> Date: Thu, 26 Feb 2026 14:41:30 +0100 Subject: [PATCH 02/22] Reregister in case of Free/Attach/AllocConsole directly used. --- .../PosixSignalRegistration.Windows.cs | 48 +++++++++++++++---- 1 file changed, 38 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs index 0e14e8eb4511ae..503803fc75f1df 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs @@ -9,7 +9,7 @@ namespace System.Runtime.InteropServices public sealed partial class PosixSignalRegistration { private static readonly Dictionary> s_registrations = new(); - private static bool s_isCtrlHandlerRegisteredOnce; + private static bool s_isCtrlHandlerRegistering; private static unsafe PosixSignalRegistration Register(PosixSignal signal, Action handler) { @@ -24,28 +24,56 @@ private static unsafe PosixSignalRegistration Register(PosixSignal signal, Actio var token = new Token(signal, signo, handler); var registration = new PosixSignalRegistration(token); - bool registerCtrlHandler = false; + bool registerCtrlHandler = false; lock (s_registrations) { - if (s_registrations.Count == 0 && !s_isCtrlHandlerRegisteredOnce) + if (s_registrations.Count == 0 && !s_isCtrlHandlerRegistering) { - s_isCtrlHandlerRegisteredOnce = true; registerCtrlHandler = true; + s_isCtrlHandlerRegistering = true; } + } - if (!s_registrations.TryGetValue(signo, out List? tokens)) + if (registerCtrlHandler) + { + // User may reset registrations externally by direct calls of Free/Attach/AllocConsole. + // We do not know if it is currently registered or not. To prevent duplicate + // registration, we try unregister existing one first. + if (!Interop.Kernel32.SetConsoleCtrlHandler(&HandlerRoutine, Add: false)) { - s_registrations[signo] = tokens = new List(); + // Returns ERROR_INVALID_PARAMETER if it was not registered. Throw for everything else. + int error = Marshal.GetLastPInvokeError(); + if (error != Interop.Errors.ERROR_INVALID_PARAMETER) + { + lock (s_registrations) + { + s_isCtrlHandlerRegistering = false; + } + throw Win32Marshal.GetExceptionForWin32Error(error); + } } - tokens.Add(token); + if (!Interop.Kernel32.SetConsoleCtrlHandler(&HandlerRoutine, Add: true)) + { + lock (s_registrations) + { + s_isCtrlHandlerRegistering = false; + } + throw Win32Marshal.GetExceptionForLastWin32Error(); + } } - if (registerCtrlHandler && - !Interop.Kernel32.SetConsoleCtrlHandler(&HandlerRoutine, Add: true)) + lock (s_registrations) { - throw Win32Marshal.GetExceptionForLastWin32Error(); + s_isCtrlHandlerRegistering = false; + + if (!s_registrations.TryGetValue(signo, out List? tokens)) + { + s_registrations[signo] = tokens = new List(); + } + + tokens.Add(token); } return registration; From 0bcf834da0069c75f4a23b3c1d1e050682c8ae9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BD=C5=AFrek?= <188900745+mrek-msft@users.noreply.github.com> Date: Fri, 6 Mar 2026 12:57:30 +0100 Subject: [PATCH 03/22] Rework locking mechanism to handle possible races --- .../PosixSignalRegistration.Windows.cs | 131 ++++++++++-------- 1 file changed, 70 insertions(+), 61 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs index 503803fc75f1df..7e163707e30d3a 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs @@ -9,7 +9,16 @@ namespace System.Runtime.InteropServices public sealed partial class PosixSignalRegistration { private static readonly Dictionary> s_registrations = new(); - private static bool s_isCtrlHandlerRegistering; + + /// + /// Lock object for critical section ensurring that only one of register and unregister happen at time. + /// + private static readonly object s_oneOfRegisterUnregisterLock = new(); + + /// + /// Lock object for critical section ensuring that only one of unregister and callback execution happen at time. + /// + private static readonly object s_oneOfUnregisterExecuteLock = new(); private static unsafe PosixSignalRegistration Register(PosixSignal signal, Action handler) { @@ -25,55 +34,47 @@ private static unsafe PosixSignalRegistration Register(PosixSignal signal, Actio var token = new Token(signal, signo, handler); var registration = new PosixSignalRegistration(token); - bool registerCtrlHandler = false; - lock (s_registrations) + lock (s_oneOfRegisterUnregisterLock) { - if (s_registrations.Count == 0 && !s_isCtrlHandlerRegistering) + bool registerCtrlHandler = false; + lock (s_registrations) { - registerCtrlHandler = true; - s_isCtrlHandlerRegistering = true; + if (s_registrations.Count == 0) + { + registerCtrlHandler = true; + } } - } - if (registerCtrlHandler) - { - // User may reset registrations externally by direct calls of Free/Attach/AllocConsole. - // We do not know if it is currently registered or not. To prevent duplicate - // registration, we try unregister existing one first. - if (!Interop.Kernel32.SetConsoleCtrlHandler(&HandlerRoutine, Add: false)) + if (registerCtrlHandler) { - // Returns ERROR_INVALID_PARAMETER if it was not registered. Throw for everything else. - int error = Marshal.GetLastPInvokeError(); - if (error != Interop.Errors.ERROR_INVALID_PARAMETER) + // User may reset registrations externally by direct calls of Free/Attach/AllocConsole. + // We do not know if it is currently registered or not. To prevent duplicate + // registration, we try unregister existing one first. + if (!Interop.Kernel32.SetConsoleCtrlHandler(&HandlerRoutine, Add: false)) { - lock (s_registrations) + // Returns ERROR_INVALID_PARAMETER if it was not registered. Throw for everything else. + int error = Marshal.GetLastPInvokeError(); + if (error != Interop.Errors.ERROR_INVALID_PARAMETER) { - s_isCtrlHandlerRegistering = false; + throw Win32Marshal.GetExceptionForWin32Error(error); } - throw Win32Marshal.GetExceptionForWin32Error(error); } - } - if (!Interop.Kernel32.SetConsoleCtrlHandler(&HandlerRoutine, Add: true)) - { - lock (s_registrations) + if (!Interop.Kernel32.SetConsoleCtrlHandler(&HandlerRoutine, Add: true)) { - s_isCtrlHandlerRegistering = false; + throw Win32Marshal.GetExceptionForLastWin32Error(); } - throw Win32Marshal.GetExceptionForLastWin32Error(); } - } - - lock (s_registrations) - { - s_isCtrlHandlerRegistering = false; - if (!s_registrations.TryGetValue(signo, out List? tokens)) + lock (s_registrations) { - s_registrations[signo] = tokens = new List(); - } + if (!s_registrations.TryGetValue(signo, out List? tokens)) + { + s_registrations[signo] = tokens = new List(); + } - tokens.Add(token); + tokens.Add(token); + } } return registration; @@ -81,18 +82,24 @@ private static unsafe PosixSignalRegistration Register(PosixSignal signal, Actio private unsafe void Unregister() { - lock (s_registrations) + lock (s_oneOfUnregisterExecuteLock) { - if (_token is Token token) + lock (s_oneOfRegisterUnregisterLock) { - _token = null; - - if (s_registrations.TryGetValue(token.SigNo, out List? tokens)) + lock (s_registrations) { - tokens.Remove(token); - if (tokens.Count == 0) + if (_token is Token token) { - s_registrations.Remove(token.SigNo); + _token = null; + + if (s_registrations.TryGetValue(token.SigNo, out List? tokens)) + { + tokens.Remove(token); + if (tokens.Count == 0) + { + s_registrations.Remove(token.SigNo); + } + } } } } @@ -103,32 +110,34 @@ private unsafe void Unregister() private static Interop.BOOL HandlerRoutine(int dwCtrlType) { Token[]? tokens = null; - - lock (s_registrations) + lock (s_oneOfUnregisterExecuteLock) { - if (s_registrations.TryGetValue(dwCtrlType, out List? registrations)) + lock (s_registrations) { - tokens = new Token[registrations.Count]; - registrations.CopyTo(tokens); + if (s_registrations.TryGetValue(dwCtrlType, out List? registrations)) + { + tokens = new Token[registrations.Count]; + registrations.CopyTo(tokens); + } } - } - if (tokens is null) - { - return Interop.BOOL.FALSE; - } + if (tokens is null) + { + return Interop.BOOL.FALSE; + } - var context = new PosixSignalContext(0); + var context = new PosixSignalContext(0); - // Iterate through the tokens in reverse order to match the order of registration. - for (int i = tokens.Length - 1; i >= 0; i--) - { - Token token = tokens[i]; - context.Signal = token.Signal; - token.Handler(context); - } + // Iterate through the tokens in reverse order to match the order of registration. + for (int i = tokens.Length - 1; i >= 0; i--) + { + Token token = tokens[i]; + context.Signal = token.Signal; + token.Handler(context); + } - return context.Cancel ? Interop.BOOL.TRUE : Interop.BOOL.FALSE; + return context.Cancel ? Interop.BOOL.TRUE : Interop.BOOL.FALSE; + } } } } From adfcccb9b170fb366d23bca600b1387f99d25466 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BD=C5=AFrek?= <188900745+mrek-msft@users.noreply.github.com> Date: Mon, 9 Mar 2026 12:58:23 +0100 Subject: [PATCH 04/22] Undo s_oneOfUnregisterExecuteLock --- .../PosixSignalRegistration.Windows.cs | 64 ++++++++----------- 1 file changed, 27 insertions(+), 37 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs index 7e163707e30d3a..f7ace235cd3208 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs @@ -15,10 +15,6 @@ public sealed partial class PosixSignalRegistration /// private static readonly object s_oneOfRegisterUnregisterLock = new(); - /// - /// Lock object for critical section ensuring that only one of unregister and callback execution happen at time. - /// - private static readonly object s_oneOfUnregisterExecuteLock = new(); private static unsafe PosixSignalRegistration Register(PosixSignal signal, Action handler) { @@ -82,23 +78,20 @@ private static unsafe PosixSignalRegistration Register(PosixSignal signal, Actio private unsafe void Unregister() { - lock (s_oneOfUnregisterExecuteLock) + lock (s_oneOfRegisterUnregisterLock) { - lock (s_oneOfRegisterUnregisterLock) + lock (s_registrations) { - lock (s_registrations) + if (_token is Token token) { - if (_token is Token token) - { - _token = null; + _token = null; - if (s_registrations.TryGetValue(token.SigNo, out List? tokens)) + if (s_registrations.TryGetValue(token.SigNo, out List? tokens)) + { + tokens.Remove(token); + if (tokens.Count == 0) { - tokens.Remove(token); - if (tokens.Count == 0) - { - s_registrations.Remove(token.SigNo); - } + s_registrations.Remove(token.SigNo); } } } @@ -110,34 +103,31 @@ private unsafe void Unregister() private static Interop.BOOL HandlerRoutine(int dwCtrlType) { Token[]? tokens = null; - lock (s_oneOfUnregisterExecuteLock) + lock (s_registrations) { - lock (s_registrations) + if (s_registrations.TryGetValue(dwCtrlType, out List? registrations)) { - if (s_registrations.TryGetValue(dwCtrlType, out List? registrations)) - { - tokens = new Token[registrations.Count]; - registrations.CopyTo(tokens); - } - } - - if (tokens is null) - { - return Interop.BOOL.FALSE; + tokens = new Token[registrations.Count]; + registrations.CopyTo(tokens); } + } - var context = new PosixSignalContext(0); + if (tokens is null) + { + return Interop.BOOL.FALSE; + } - // Iterate through the tokens in reverse order to match the order of registration. - for (int i = tokens.Length - 1; i >= 0; i--) - { - Token token = tokens[i]; - context.Signal = token.Signal; - token.Handler(context); - } + var context = new PosixSignalContext(0); - return context.Cancel ? Interop.BOOL.TRUE : Interop.BOOL.FALSE; + // Iterate through the tokens in reverse order to match the order of registration. + for (int i = tokens.Length - 1; i >= 0; i--) + { + Token token = tokens[i]; + context.Signal = token.Signal; + token.Handler(context); } + + return context.Cancel ? Interop.BOOL.TRUE : Interop.BOOL.FALSE; } } } From d9ab11079bb9f98939e262846acf3d5661826c19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BD=C5=AFrek?= <188900745+mrek-msft@users.noreply.github.com> Date: Mon, 9 Mar 2026 13:00:47 +0100 Subject: [PATCH 05/22] Whitespace changes cleanup --- .../Runtime/InteropServices/PosixSignalRegistration.Windows.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs index f7ace235cd3208..ee875d576d081e 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs @@ -15,7 +15,6 @@ public sealed partial class PosixSignalRegistration /// private static readonly object s_oneOfRegisterUnregisterLock = new(); - private static unsafe PosixSignalRegistration Register(PosixSignal signal, Action handler) { int signo = signal switch @@ -103,6 +102,7 @@ private unsafe void Unregister() private static Interop.BOOL HandlerRoutine(int dwCtrlType) { Token[]? tokens = null; + lock (s_registrations) { if (s_registrations.TryGetValue(dwCtrlType, out List? registrations)) From 1a9751fa12a99d333d0c36c38cd151a1638914a8 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Mon, 9 Mar 2026 09:41:31 -0700 Subject: [PATCH 06/22] Update src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../Runtime/InteropServices/PosixSignalRegistration.Windows.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs index ee875d576d081e..a15429938f12b3 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs @@ -11,7 +11,7 @@ public sealed partial class PosixSignalRegistration private static readonly Dictionary> s_registrations = new(); /// - /// Lock object for critical section ensurring that only one of register and unregister happen at time. + /// Lock object for critical section ensuring that only one of register and unregister happens at a time. /// private static readonly object s_oneOfRegisterUnregisterLock = new(); From e2184ec2ef274231ed91ebdc400e650ae99de120 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BD=C5=AFrek?= <188900745+mrek-msft@users.noreply.github.com> Date: Thu, 12 Mar 2026 13:30:13 +0100 Subject: [PATCH 07/22] Remove register/unregister lock and rename --- .../PosixSignalRegistration.Windows.cs | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs index ee875d576d081e..535787890b6e43 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs @@ -13,7 +13,7 @@ public sealed partial class PosixSignalRegistration /// /// Lock object for critical section ensurring that only one of register and unregister happen at time. /// - private static readonly object s_oneOfRegisterUnregisterLock = new(); + private static readonly object s_registerLock = new(); private static unsafe PosixSignalRegistration Register(PosixSignal signal, Action handler) { @@ -29,7 +29,7 @@ private static unsafe PosixSignalRegistration Register(PosixSignal signal, Actio var token = new Token(signal, signo, handler); var registration = new PosixSignalRegistration(token); - lock (s_oneOfRegisterUnregisterLock) + lock (s_registerLock) { bool registerCtrlHandler = false; lock (s_registrations) @@ -77,21 +77,18 @@ private static unsafe PosixSignalRegistration Register(PosixSignal signal, Actio private unsafe void Unregister() { - lock (s_oneOfRegisterUnregisterLock) + lock (s_registrations) { - lock (s_registrations) + if (_token is Token token) { - if (_token is Token token) - { - _token = null; + _token = null; - if (s_registrations.TryGetValue(token.SigNo, out List? tokens)) + if (s_registrations.TryGetValue(token.SigNo, out List? tokens)) + { + tokens.Remove(token); + if (tokens.Count == 0) { - tokens.Remove(token); - if (tokens.Count == 0) - { - s_registrations.Remove(token.SigNo); - } + s_registrations.Remove(token.SigNo); } } } From 44e676323f148ebeafa1825c323ce36da02830f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BD=C5=AFrek?= <188900745+mrek-msft@users.noreply.github.com> Date: Thu, 12 Mar 2026 13:45:04 +0100 Subject: [PATCH 08/22] Capture and use fixed address of HandlerRoutine --- .../PosixSignalRegistration.Windows.cs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs index 535787890b6e43..0e1b3df0004c11 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs @@ -3,10 +3,11 @@ using System.Collections.Generic; using System.IO; +using static Interop; namespace System.Runtime.InteropServices { - public sealed partial class PosixSignalRegistration + public sealed unsafe partial class PosixSignalRegistration { private static readonly Dictionary> s_registrations = new(); @@ -15,7 +16,13 @@ public sealed partial class PosixSignalRegistration /// private static readonly object s_registerLock = new(); - private static unsafe PosixSignalRegistration Register(PosixSignal signal, Action handler) + /// + /// Runtime can generate multiple addresses to same function. For registering and unregistering allways + /// the same instance, we capture it in this statics. + /// + private static readonly delegate* unmanaged s_HandlerRoutineAddr = &HandlerRoutine; + + private static PosixSignalRegistration Register(PosixSignal signal, Action handler) { int signo = signal switch { @@ -45,7 +52,7 @@ private static unsafe PosixSignalRegistration Register(PosixSignal signal, Actio // User may reset registrations externally by direct calls of Free/Attach/AllocConsole. // We do not know if it is currently registered or not. To prevent duplicate // registration, we try unregister existing one first. - if (!Interop.Kernel32.SetConsoleCtrlHandler(&HandlerRoutine, Add: false)) + if (!Interop.Kernel32.SetConsoleCtrlHandler(s_HandlerRoutineAddr, Add: false)) { // Returns ERROR_INVALID_PARAMETER if it was not registered. Throw for everything else. int error = Marshal.GetLastPInvokeError(); @@ -55,7 +62,7 @@ private static unsafe PosixSignalRegistration Register(PosixSignal signal, Actio } } - if (!Interop.Kernel32.SetConsoleCtrlHandler(&HandlerRoutine, Add: true)) + if (!Interop.Kernel32.SetConsoleCtrlHandler(s_HandlerRoutineAddr, Add: true)) { throw Win32Marshal.GetExceptionForLastWin32Error(); } @@ -75,7 +82,7 @@ private static unsafe PosixSignalRegistration Register(PosixSignal signal, Actio return registration; } - private unsafe void Unregister() + private void Unregister() { lock (s_registrations) { From d27d330a384e46e4408f467713d166861d96e651 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BD=C5=AFrek?= <188900745+mrek-msft@users.noreply.github.com> Date: Thu, 12 Mar 2026 16:34:13 +0100 Subject: [PATCH 09/22] Adding test for testing that deadlock do not happen whne unregistering from handler --- .../tests/ProcessTests.cs | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs index d89f422a2065d1..1bbe42c9c4e7c5 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs @@ -161,6 +161,98 @@ public void TestCreateNewProcessGroup_HandlerReceivesExpectedSignal(PosixSignal } } + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [MemberData(nameof(SignalTestData))] + public void SignalHandler_CanDisposeInHandler(PosixSignal signal) + { + const string PosixSignalRegistrationCreatedMessage = "PosixSignalRegistration created..."; + const string PosixSignalHandlerStartedMessage = "PosixSignalHandlerFinishedMessage created..."; + const string PosixSignalHandlerDisposedMessage = "PosixSignalHandlerDisposedMessage created..."; + + var remoteInvokeOptions = new RemoteInvokeOptions { CheckExitCode = false }; + remoteInvokeOptions.StartInfo.RedirectStandardOutput = true; + if (OperatingSystem.IsWindows()) + { + remoteInvokeOptions.StartInfo.CreateNewProcessGroup = true; + } + + using RemoteInvokeHandle remoteHandle = RemoteExecutor.Invoke( + (signalStr) => + { + PosixSignal expectedSignal = Enum.Parse(signalStr); + using ManualResetEvent receivedSignalEvent = new ManualResetEvent(false); + ReEnableCtrlCHandlerIfNeeded(expectedSignal); + + PosixSignalRegistration? p = null; + p = PosixSignalRegistration.Create(expectedSignal, (ctx) => + { + Console.WriteLine(PosixSignalHandlerStartedMessage); + Assert.Equal(expectedSignal, ctx.Signal); + + Assert.NotNull(p); + p?.Dispose(); + + // Used for checking that Dispose returned and did not stuck + Console.WriteLine(PosixSignalHandlerDisposedMessage); + + receivedSignalEvent.Set(); + ctx.Cancel = true; + }); + + Console.WriteLine(PosixSignalRegistrationCreatedMessage); + + // Wait for singal which unregisters itself + Assert.True(receivedSignalEvent.WaitOne(WaitInMS)); + + // Wait for second signal which should temrinate process by default system handler + Thread.Sleep(1000); + + // If we did not terminated yet, return failure exit code + return -1; + }, + arg: $"{signal}", + remoteInvokeOptions); + + while (!remoteHandle.Process.StandardOutput.ReadLine().EndsWith(PosixSignalRegistrationCreatedMessage)) + { + Thread.Sleep(20); + } + + try + { + SendSignal(signal, remoteHandle.Process.Id); + + while (!remoteHandle.Process.StandardOutput.ReadLine().EndsWith(PosixSignalHandlerStartedMessage)) + { + Thread.Sleep(20); + } + + while (!remoteHandle.Process.StandardOutput.ReadLine().EndsWith(PosixSignalHandlerDisposedMessage)) + { + Thread.Sleep(20); + } + + SendSignal(signal, remoteHandle.Process.Id); + + Assert.True(remoteHandle.Process.WaitForExit(WaitInMS)); + Assert.True(remoteHandle.Process.StandardOutput.EndOfStream); + if (OperatingSystem.IsWindows()) + { + Assert.Equal(unchecked((int)0xC000013A), remoteHandle.Process.ExitCode); // STATUS_CONTROL_C_EXIT + } + else + { + Assert.Equal(0, remoteHandle.Process.ExitCode); + } + } + finally + { + // If sending the signal fails, we want to kill the process ASAP + // to prevent RemoteExecutor's timeout from hiding it. + remoteHandle.Process.Kill(); + } + } + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] [InlineData(-2)] [InlineData((long)int.MaxValue + 1)] From b369a3c522e58afceedf98b0dc7f7c59a0a0773a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BD=C5=AFrek?= <188900745+mrek-msft@users.noreply.github.com> Date: Wed, 11 Mar 2026 15:06:51 +0100 Subject: [PATCH 10/22] Dispose handlers immidiately, do not wait for finialize time --- .../src/Internal/ConsoleLifetime.netcoreapp.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting/src/Internal/ConsoleLifetime.netcoreapp.cs b/src/libraries/Microsoft.Extensions.Hosting/src/Internal/ConsoleLifetime.netcoreapp.cs index c5be9291ffd44c..71c72b3564f1ec 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/src/Internal/ConsoleLifetime.netcoreapp.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/src/Internal/ConsoleLifetime.netcoreapp.cs @@ -38,11 +38,7 @@ private void HandleWindowsShutdown(PosixSignalContext context) // for SIGTERM on Windows we must block this thread until the application is finished // otherwise the process will be killed immediately on return from this handler - // don't allow Dispose to unregister handlers, since Windows has a lock that prevents the unregistration while this handler is running - // just leak these, since the process is exiting - _sigIntRegistration = null; - _sigQuitRegistration = null; - _sigTermRegistration = null; + UnregisterShutdownHandlers(); ApplicationLifetime.StopApplication(); From c4250f37c0cbe6afb986cca12da6dab1341208ee Mon Sep 17 00:00:00 2001 From: MichalZ <188900745+mrek-msft@users.noreply.github.com> Date: Fri, 13 Mar 2026 11:22:36 +0100 Subject: [PATCH 11/22] Fix typo in comment Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs index 1bbe42c9c4e7c5..e056734407ac8a 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs @@ -201,7 +201,7 @@ public void SignalHandler_CanDisposeInHandler(PosixSignal signal) Console.WriteLine(PosixSignalRegistrationCreatedMessage); - // Wait for singal which unregisters itself + // Wait for signal which unregisters itself Assert.True(receivedSignalEvent.WaitOne(WaitInMS)); // Wait for second signal which should temrinate process by default system handler From 43b751ff8f7f3e265c0e61d9016cf340a3a1a500 Mon Sep 17 00:00:00 2001 From: MichalZ <188900745+mrek-msft@users.noreply.github.com> Date: Fri, 13 Mar 2026 11:23:08 +0100 Subject: [PATCH 12/22] Fix misleading comment on lock object. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../Runtime/InteropServices/PosixSignalRegistration.Windows.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs index 83405206c916c9..60cea0c7b14908 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs @@ -12,7 +12,7 @@ public sealed unsafe partial class PosixSignalRegistration private static readonly Dictionary> s_registrations = new(); /// - /// Lock object for critical section ensuring that only one of register and unregister happens at a time. + /// Lock object used to serialize registration changes that affect calls to SetConsoleCtrlHandler. /// private static readonly object s_registerLock = new(); From 481abee17e101da19e43ba4c5a162f39328da912 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BD=C5=AFrek?= <188900745+mrek-msft@users.noreply.github.com> Date: Fri, 13 Mar 2026 11:43:27 +0100 Subject: [PATCH 13/22] Move unsafe from partial class to members needing it. --- .../InteropServices/PosixSignalRegistration.Windows.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs index 60cea0c7b14908..6ba087404bd78b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs @@ -7,7 +7,7 @@ namespace System.Runtime.InteropServices { - public sealed unsafe partial class PosixSignalRegistration + public sealed partial class PosixSignalRegistration { private static readonly Dictionary> s_registrations = new(); @@ -20,9 +20,9 @@ public sealed unsafe partial class PosixSignalRegistration /// Runtime can generate multiple addresses to same function. For registering and unregistering allways /// the same instance, we capture it in this statics. /// - private static readonly delegate* unmanaged s_HandlerRoutineAddr = &HandlerRoutine; + private static readonly unsafe delegate* unmanaged s_HandlerRoutineAddr = &HandlerRoutine; - private static PosixSignalRegistration Register(PosixSignal signal, Action handler) + private static unsafe PosixSignalRegistration Register(PosixSignal signal, Action handler) { int signo = signal switch { From a7ed82c8a01a84346a218af22f26e5b772931f2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BD=C5=AFrek?= <188900745+mrek-msft@users.noreply.github.com> Date: Fri, 13 Mar 2026 11:44:09 +0100 Subject: [PATCH 14/22] Fix typo in comment --- src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs index e056734407ac8a..32e75223f91161 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs @@ -204,7 +204,7 @@ public void SignalHandler_CanDisposeInHandler(PosixSignal signal) // Wait for signal which unregisters itself Assert.True(receivedSignalEvent.WaitOne(WaitInMS)); - // Wait for second signal which should temrinate process by default system handler + // Wait for second signal which should terminate process by default system handler Thread.Sleep(1000); // If we did not terminated yet, return failure exit code From 5af264ae7ca204329f71a83297de05bdc15a67aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BD=C5=AFrek?= <188900745+mrek-msft@users.noreply.github.com> Date: Fri, 13 Mar 2026 11:49:15 +0100 Subject: [PATCH 15/22] revert introduced using static. --- .../Runtime/InteropServices/PosixSignalRegistration.Windows.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs index 6ba087404bd78b..c664359ea35a23 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs @@ -3,7 +3,6 @@ using System.Collections.Generic; using System.IO; -using static Interop; namespace System.Runtime.InteropServices { @@ -20,7 +19,7 @@ public sealed partial class PosixSignalRegistration /// Runtime can generate multiple addresses to same function. For registering and unregistering allways /// the same instance, we capture it in this statics. /// - private static readonly unsafe delegate* unmanaged s_HandlerRoutineAddr = &HandlerRoutine; + private static readonly unsafe delegate* unmanaged s_HandlerRoutineAddr = &HandlerRoutine; private static unsafe PosixSignalRegistration Register(PosixSignal signal, Action handler) { From 5e591980f24ebc6de4288e35a644b497bad2a5af Mon Sep 17 00:00:00 2001 From: MichalZ <188900745+mrek-msft@users.noreply.github.com> Date: Fri, 13 Mar 2026 11:50:17 +0100 Subject: [PATCH 16/22] Improve comment grammar. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs index e056734407ac8a..1b4e32f443dc05 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs @@ -192,7 +192,7 @@ public void SignalHandler_CanDisposeInHandler(PosixSignal signal) Assert.NotNull(p); p?.Dispose(); - // Used for checking that Dispose returned and did not stuck + // Used for checking that Dispose returned and did not get stuck Console.WriteLine(PosixSignalHandlerDisposedMessage); receivedSignalEvent.Set(); From 20822897daec4fc69207f398f6317fab214fb25a Mon Sep 17 00:00:00 2001 From: MichalZ <188900745+mrek-msft@users.noreply.github.com> Date: Fri, 13 Mar 2026 11:52:21 +0100 Subject: [PATCH 17/22] Fix wrong const message Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs index 1b4e32f443dc05..671b08e9c17c9a 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs @@ -166,7 +166,7 @@ public void TestCreateNewProcessGroup_HandlerReceivesExpectedSignal(PosixSignal public void SignalHandler_CanDisposeInHandler(PosixSignal signal) { const string PosixSignalRegistrationCreatedMessage = "PosixSignalRegistration created..."; - const string PosixSignalHandlerStartedMessage = "PosixSignalHandlerFinishedMessage created..."; + const string PosixSignalHandlerStartedMessage = "PosixSignalHandlerStartedMessage created..."; const string PosixSignalHandlerDisposedMessage = "PosixSignalHandlerDisposedMessage created..."; var remoteInvokeOptions = new RemoteInvokeOptions { CheckExitCode = false }; From 781813fa59cbbc33ee29ac6dbedefdaf4c09c512 Mon Sep 17 00:00:00 2001 From: MichalZ <188900745+mrek-msft@users.noreply.github.com> Date: Fri, 13 Mar 2026 11:53:09 +0100 Subject: [PATCH 18/22] Improve comment grammar Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../InteropServices/PosixSignalRegistration.Windows.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs index 60cea0c7b14908..637bd60c9f0ad6 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs @@ -17,8 +17,8 @@ public sealed unsafe partial class PosixSignalRegistration private static readonly object s_registerLock = new(); /// - /// Runtime can generate multiple addresses to same function. For registering and unregistering allways - /// the same instance, we capture it in this statics. + /// Runtime can generate multiple addresses to the same function. To ensure that registering and unregistering always + /// use the same instance, we capture it in this static field. /// private static readonly delegate* unmanaged s_HandlerRoutineAddr = &HandlerRoutine; From 54be37cecb56f17f0e2d8778f2ede3796506a62a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BD=C5=AFrek?= <188900745+mrek-msft@users.noreply.github.com> Date: Fri, 13 Mar 2026 11:54:21 +0100 Subject: [PATCH 19/22] Fix wrong naming convetion --- .../InteropServices/PosixSignalRegistration.Windows.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs index c664359ea35a23..88376b2d5f58e4 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs @@ -19,7 +19,7 @@ public sealed partial class PosixSignalRegistration /// Runtime can generate multiple addresses to same function. For registering and unregistering allways /// the same instance, we capture it in this statics. /// - private static readonly unsafe delegate* unmanaged s_HandlerRoutineAddr = &HandlerRoutine; + private static readonly unsafe delegate* unmanaged s_handlerRoutineAddr = &HandlerRoutine; private static unsafe PosixSignalRegistration Register(PosixSignal signal, Action handler) { @@ -51,7 +51,7 @@ private static unsafe PosixSignalRegistration Register(PosixSignal signal, Actio // User may reset registrations externally by direct calls of Free/Attach/AllocConsole. // We do not know if it is currently registered or not. To prevent duplicate // registration, we try unregister existing one first. - if (!Interop.Kernel32.SetConsoleCtrlHandler(s_HandlerRoutineAddr, Add: false)) + if (!Interop.Kernel32.SetConsoleCtrlHandler(s_handlerRoutineAddr, Add: false)) { // Returns ERROR_INVALID_PARAMETER if it was not registered. Throw for everything else. int error = Marshal.GetLastPInvokeError(); @@ -61,7 +61,7 @@ private static unsafe PosixSignalRegistration Register(PosixSignal signal, Actio } } - if (!Interop.Kernel32.SetConsoleCtrlHandler(s_HandlerRoutineAddr, Add: true)) + if (!Interop.Kernel32.SetConsoleCtrlHandler(s_handlerRoutineAddr, Add: true)) { throw Win32Marshal.GetExceptionForLastWin32Error(); } From 25cdb3c5344b68e991eefd03be1ad97fdaf89fff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BD=C5=AFrek?= <188900745+mrek-msft@users.noreply.github.com> Date: Fri, 13 Mar 2026 12:04:17 +0100 Subject: [PATCH 20/22] Improve remote proces emmited messages --- .../System.Diagnostics.Process/tests/ProcessTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs index dc08cabccba808..5039f1663a0ed7 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs @@ -165,9 +165,9 @@ public void TestCreateNewProcessGroup_HandlerReceivesExpectedSignal(PosixSignal [MemberData(nameof(SignalTestData))] public void SignalHandler_CanDisposeInHandler(PosixSignal signal) { - const string PosixSignalRegistrationCreatedMessage = "PosixSignalRegistration created..."; - const string PosixSignalHandlerStartedMessage = "PosixSignalHandlerStartedMessage created..."; - const string PosixSignalHandlerDisposedMessage = "PosixSignalHandlerDisposedMessage created..."; + const string PosixSignalRegistrationCreatedMessage = "PosixSignalRegistration created."; + const string PosixSignalHandlerStartedMessage = "PosixSignalRegistration handler started."; + const string PosixSignalHandlerDisposedMessage = "PosixSignalRegistration disposed."; var remoteInvokeOptions = new RemoteInvokeOptions { CheckExitCode = false }; remoteInvokeOptions.StartInfo.RedirectStandardOutput = true; From cc720e3a9c12941dc7606827eae1f01158eb347e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BD=C5=AFrek?= <188900745+mrek-msft@users.noreply.github.com> Date: Fri, 13 Mar 2026 12:34:16 +0100 Subject: [PATCH 21/22] Comments improvements. --- .../InteropServices/PosixSignalRegistration.Windows.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs index 51fb1f6185baee..e5fbd3ceee16de 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs @@ -11,13 +11,14 @@ public sealed partial class PosixSignalRegistration private static readonly Dictionary> s_registrations = new(); /// - /// Lock object used to serialize registration changes that affect calls to SetConsoleCtrlHandler. + /// Serializes concurrent calls to make the emptiness check and + /// the subsequent token insertion atomic. /// private static readonly object s_registerLock = new(); /// - /// Runtime can generate multiple addresses to the same function. To ensure that registering and unregistering always - /// use the same instance, we capture it in this static field. + /// Runtime can generate multiple addresses to the same function. To ensure that registering and + /// unregistering always use the same instance, we capture it in this static field. /// private static readonly unsafe delegate* unmanaged s_handlerRoutineAddr = &HandlerRoutine; @@ -46,6 +47,9 @@ private static unsafe PosixSignalRegistration Register(PosixSignal signal, Actio } } + // All SetConsoleCtrlHandler calls must happen outside s_registrations locked section + // otherwise we risk AB/BA deadlock between it and internal critical section in OS. + if (registerCtrlHandler) { // User may reset registrations externally by direct calls of Free/Attach/AllocConsole. From af28e8f1d64ba93cb4f3afd45858e5f0b96df08e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BD=C5=AFrek?= <188900745+mrek-msft@users.noreply.github.com> Date: Mon, 16 Mar 2026 11:52:14 +0100 Subject: [PATCH 22/22] Move waiting for console message to separate method, add timeout. Change asseritng return code on linux. --- .../tests/ProcessTests.cs | 57 ++++++++++++------- 1 file changed, 37 insertions(+), 20 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs index 5039f1663a0ed7..8ccd4682d31b80 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs @@ -81,6 +81,33 @@ private void AssertNonZeroAllZeroDarwin(long value) } } + private static void AssertRemoteProcessStandardOutputLine(RemoteInvokeHandle remoteHandle, string expectedMessage, int timeout) + { + using CancellationTokenSource cts = new(); + + Task readTask = remoteHandle.Process.StandardOutput.ReadLineAsync(cts.Token).AsTask(); + Task timeoutTask = Task.Delay(timeout, cts.Token); + + Task finishedTask = Task.WhenAny(readTask, timeoutTask).ConfigureAwait(false).GetAwaiter().GetResult(); + cts.Cancel(); // cancel the slower one + + if (finishedTask == readTask) + { + if (readTask.Result != null) + { + Assert.Equal(expectedMessage, readTask.Result); + } + else + { + throw new XunitException($"StandardOutput was closed before observing expected message '{expectedMessage}'"); + } + } + else + { + throw new XunitException($"Expected message '{expectedMessage}' was not observed on remote process within specified time."); + } + } + public static IEnumerable SignalTestData() { if (OperatingSystem.IsWindows()) @@ -141,10 +168,7 @@ public void TestCreateNewProcessGroup_HandlerReceivesExpectedSignal(PosixSignal arg: $"{signal}", remoteInvokeOptions); - while (!remoteHandle.Process.StandardOutput.ReadLine().EndsWith(PosixSignalRegistrationCreatedMessage)) - { - Thread.Sleep(20); - } + AssertRemoteProcessStandardOutputLine(remoteHandle, PosixSignalRegistrationCreatedMessage, WaitInMS); try { @@ -162,7 +186,9 @@ public void TestCreateNewProcessGroup_HandlerReceivesExpectedSignal(PosixSignal } [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - [MemberData(nameof(SignalTestData))] + // Limited only to signals which terminates process by default, and are supported on all platfroms by SendSignal + [InlineData(PosixSignal.SIGINT)] + [InlineData(PosixSignal.SIGQUIT)] public void SignalHandler_CanDisposeInHandler(PosixSignal signal) { const string PosixSignalRegistrationCreatedMessage = "PosixSignalRegistration created."; @@ -205,7 +231,7 @@ public void SignalHandler_CanDisposeInHandler(PosixSignal signal) Assert.True(receivedSignalEvent.WaitOne(WaitInMS)); // Wait for second signal which should terminate process by default system handler - Thread.Sleep(1000); + Thread.Sleep(WaitInMS); // If we did not terminated yet, return failure exit code return -1; @@ -213,24 +239,15 @@ public void SignalHandler_CanDisposeInHandler(PosixSignal signal) arg: $"{signal}", remoteInvokeOptions); - while (!remoteHandle.Process.StandardOutput.ReadLine().EndsWith(PosixSignalRegistrationCreatedMessage)) - { - Thread.Sleep(20); - } try { - SendSignal(signal, remoteHandle.Process.Id); + AssertRemoteProcessStandardOutputLine(remoteHandle, PosixSignalRegistrationCreatedMessage, WaitInMS); - while (!remoteHandle.Process.StandardOutput.ReadLine().EndsWith(PosixSignalHandlerStartedMessage)) - { - Thread.Sleep(20); - } + SendSignal(signal, remoteHandle.Process.Id); - while (!remoteHandle.Process.StandardOutput.ReadLine().EndsWith(PosixSignalHandlerDisposedMessage)) - { - Thread.Sleep(20); - } + AssertRemoteProcessStandardOutputLine(remoteHandle, PosixSignalHandlerStartedMessage, WaitInMS); + AssertRemoteProcessStandardOutputLine(remoteHandle, PosixSignalHandlerDisposedMessage, WaitInMS); SendSignal(signal, remoteHandle.Process.Id); @@ -242,7 +259,7 @@ public void SignalHandler_CanDisposeInHandler(PosixSignal signal) } else { - Assert.Equal(0, remoteHandle.Process.ExitCode); + Assert.Equal(128 + (int)signal, remoteHandle.Process.ExitCode); } } finally