diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs index 89c2eb45ac852f..bd5a09dfe48fdc 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs @@ -9,9 +9,9 @@ namespace System.Runtime.InteropServices { public sealed partial class PosixSignalRegistration { - private static readonly Dictionary> s_registrations = Initialize(); + private static readonly Dictionary> s_registrations = Initialize(); - private static unsafe Dictionary> Initialize() + private static unsafe Dictionary> Initialize() { if (!Interop.Sys.InitializeTerminalAndSignalHandling()) { @@ -20,7 +20,7 @@ private static unsafe Dictionary> Initialize() Interop.Sys.SetPosixSignalHandler(&OnPosixSignal); - return new Dictionary>(); + return new Dictionary>(); } private static PosixSignalRegistration Register(PosixSignal signal, Action handler) @@ -36,9 +36,9 @@ private static PosixSignalRegistration Register(PosixSignal signal, Action? tokens)) + if (!s_registrations.TryGetValue(signo, out List? tokens)) { - s_registrations[signo] = tokens = new HashSet(); + s_registrations[signo] = tokens = new List(); } if (tokens.Count == 0 && @@ -61,7 +61,7 @@ private void Unregister() { _token = null; - if (s_registrations.TryGetValue(token.SigNo, out HashSet? tokens)) + if (s_registrations.TryGetValue(token.SigNo, out List? tokens)) { tokens.Remove(token); if (tokens.Count == 0) @@ -81,7 +81,7 @@ private static int OnPosixSignal(int signo, PosixSignal signal) lock (s_registrations) { - if (s_registrations.TryGetValue(signo, out HashSet? registrations)) + if (s_registrations.TryGetValue(signo, out List? registrations)) { tokens = new Token[registrations.Count]; registrations.CopyTo(tokens); @@ -122,16 +122,19 @@ static void HandleSignal(object? state) { (int signo, Token[] tokens) = ((int, Token[]))state!; - PosixSignalContext ctx = new(0); - foreach (Token token in tokens) + PosixSignalContext context = new(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]; // Different values for PosixSignal map to the same signo. // Match the PosixSignal value used when registering. - ctx.Signal = token.Signal; - token.Handler(ctx); + context.Signal = token.Signal; + token.Handler(context); } - if (!ctx.Cancel) + if (!context.Cancel) { Interop.Sys.HandleNonCanceledPosixSignal(signo); } 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 7eea717c55d42f..b176506f042af6 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 @@ -8,23 +8,20 @@ namespace System.Runtime.InteropServices { public sealed partial class PosixSignalRegistration { - private static readonly HashSet s_registrations = new(); + private static readonly Dictionary> s_registrations = new(); private static unsafe PosixSignalRegistration Register(PosixSignal signal, Action handler) { - switch (signal) + int signo = signal switch { - case PosixSignal.SIGINT: - case PosixSignal.SIGQUIT: - case PosixSignal.SIGTERM: - case PosixSignal.SIGHUP: - break; - - default: - throw new PlatformNotSupportedException(); - } - - var token = new Token(signal, handler); + PosixSignal.SIGINT => Interop.Kernel32.CTRL_C_EVENT, + PosixSignal.SIGQUIT => Interop.Kernel32.CTRL_BREAK_EVENT, + PosixSignal.SIGTERM => Interop.Kernel32.CTRL_SHUTDOWN_EVENT, + PosixSignal.SIGHUP => Interop.Kernel32.CTRL_CLOSE_EVENT, + _ => throw new PlatformNotSupportedException() + }; + + var token = new Token(signal, signo, handler); var registration = new PosixSignalRegistration(token); lock (s_registrations) @@ -35,7 +32,12 @@ private static unsafe PosixSignalRegistration Register(PosixSignal signal, Actio throw Win32Marshal.GetExceptionForLastWin32Error(); } - s_registrations.Add(token); + if (!s_registrations.TryGetValue(signo, out List? tokens)) + { + s_registrations[signo] = tokens = new List(); + } + + tokens.Add(token); } return registration; @@ -49,17 +51,25 @@ private unsafe void Unregister() { _token = null; - s_registrations.Remove(token); - if (s_registrations.Count == 0 && - !Interop.Kernel32.SetConsoleCtrlHandler(&HandlerRoutine, Add: false)) + if (s_registrations.TryGetValue(token.SigNo, out List? tokens)) { - // 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) + tokens.Remove(token); + if (tokens.Count == 0) { - throw Win32Marshal.GetExceptionForWin32Error(error); + 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); + } } } } @@ -69,38 +79,14 @@ private unsafe void Unregister() [UnmanagedCallersOnly] private static Interop.BOOL HandlerRoutine(int dwCtrlType) { - PosixSignal signal; - switch (dwCtrlType) - { - case Interop.Kernel32.CTRL_C_EVENT: - signal = PosixSignal.SIGINT; - break; + Token[]? tokens = null; - case Interop.Kernel32.CTRL_BREAK_EVENT: - signal = PosixSignal.SIGQUIT; - break; - - case Interop.Kernel32.CTRL_SHUTDOWN_EVENT: - signal = PosixSignal.SIGTERM; - break; - - case Interop.Kernel32.CTRL_CLOSE_EVENT: - signal = PosixSignal.SIGHUP; - break; - - default: - return Interop.BOOL.FALSE; - } - - List? tokens = null; lock (s_registrations) { - foreach (Token token in s_registrations) + if (s_registrations.TryGetValue(dwCtrlType, out List? registrations)) { - if (token.Signal == signal) - { - (tokens ??= new List()).Add(token); - } + tokens = new Token[registrations.Count]; + registrations.CopyTo(tokens); } } @@ -109,10 +95,14 @@ private static Interop.BOOL HandlerRoutine(int dwCtrlType) return Interop.BOOL.FALSE; } - var context = new PosixSignalContext(signal); - foreach (Token handler in tokens) + 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--) { - handler.Handler(context); + Token token = tokens[i]; + context.Signal = token.Signal; + token.Handler(context); } return context.Cancel ? Interop.BOOL.TRUE : Interop.BOOL.FALSE; diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.cs index 6cec67330022e2..a804ae68486cbc 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.cs @@ -24,6 +24,7 @@ public sealed partial class PosixSignalRegistration : IDisposable /// is not supported by the platform. /// An error occurred while setting up the signal handling or while installing the handler for the specified signal. /// + /// The handlers are executed in reverse order of their registration. /// Raw values can be provided for on Unix by casting them to . /// Default handling of the signal can be canceled through . /// and can be canceled on both diff --git a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs index 2aabb129722237..9a4b9f37a8b3d0 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs @@ -125,6 +125,41 @@ public void SignalHandlerWorksForSecondRegistration() } } + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotMobile))] + public void SignalHandlersCalledInReverseOrder() + { + PosixSignal signal = PosixSignal.SIGCONT; + bool secondHandlerCalled = false; + + using SemaphoreSlim semaphore = new(0); + using var first = PosixSignalRegistration.Create(signal, ctx => + { + Assert.Equal(signal, ctx.Signal); + + // Ensure signal doesn't cause the process to terminate. + ctx.Cancel = true; + + Assert.True(secondHandlerCalled); + + semaphore.Release(); + }); + + using var second = PosixSignalRegistration.Create(signal, ctx => + { + Assert.Equal(signal, ctx.Signal); + + // Ensure signal doesn't cause the process to terminate. + ctx.Cancel = true; + + Assert.False(secondHandlerCalled); + secondHandlerCalled = true; + }); + + kill(signal); + bool entered = semaphore.Wait(SuccessTimeout); + Assert.True(entered); + } + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotMobile))] public void SignalHandlerNotCalledWhenDisposed() {