diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.PosixSignal.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.PosixSignal.cs index b42b5138ec622d..65e3ca35f4844c 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.PosixSignal.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.PosixSignal.cs @@ -18,7 +18,7 @@ internal static partial class Sys internal static extern void DisablePosixSignalHandling(int signal); [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_HandleNonCanceledPosixSignal")] - internal static extern bool HandleNonCanceledPosixSignal(int signal, int handlersDisposed); + internal static extern void HandleNonCanceledPosixSignal(int signal); [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetPlatformSignalNumber")] [SuppressGCTransition] diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index 4c4017f6547463..ee6ccd833c0de9 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -233,7 +233,7 @@ static void SignalHandler(int sig, siginfo_t* siginfo, void* context) } } -int32_t SystemNative_HandleNonCanceledPosixSignal(int32_t signalCode, int32_t handlersDisposed) +void SystemNative_HandleNonCanceledPosixSignal(int32_t signalCode) { switch (signalCode) { @@ -276,11 +276,6 @@ int32_t SystemNative_HandleNonCanceledPosixSignal(int32_t signalCode, int32_t ha // Original handler doesn't do anything. break; } - if (handlersDisposed && g_hasPosixSignalRegistrations[signalCode - 1]) - { - // New handlers got registered. - return 0; - } // Restore and invoke the original handler. pthread_mutex_lock(&lock); { @@ -293,7 +288,6 @@ int32_t SystemNative_HandleNonCanceledPosixSignal(int32_t signalCode, int32_t ha kill(getpid(), signalCode); break; } - return 1; } // Entrypoint for the thread that handles signals where our handling @@ -385,7 +379,7 @@ static void* SignalHandlerLoop(void* arg) if (!usePosixSignalHandler) { - SystemNative_HandleNonCanceledPosixSignal(signalCode, 0); + SystemNative_HandleNonCanceledPosixSignal(signalCode); } } } diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.h b/src/libraries/Native/Unix/System.Native/pal_signal.h index 138f37835a8ca6..146fdabbfceb14 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.h +++ b/src/libraries/Native/Unix/System.Native/pal_signal.h @@ -75,7 +75,7 @@ PALEXPORT void SystemNative_DisablePosixSignalHandling(int signalCode); /** * Performs the default runtime action for a non-canceled PosixSignal. */ -PALEXPORT int32_t SystemNative_HandleNonCanceledPosixSignal(int32_t signalCode, int32_t handlersDisposed); +PALEXPORT void SystemNative_HandleNonCanceledPosixSignal(int32_t signalCode); typedef void (*ConsoleSigTtouHandler)(void); diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.PlatformNotSupported.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.PlatformNotSupported.cs index 3ccc169b1a84b3..e726194c0c0e7f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.PlatformNotSupported.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.PlatformNotSupported.cs @@ -10,16 +10,9 @@ public sealed partial class PosixSignalRegistration private PosixSignalRegistration() { } [DynamicDependency("#ctor")] // Prevent the private ctor and the IDisposable implementation from getting linked away - public static partial PosixSignalRegistration Create(PosixSignal signal, Action handler) - { - if (handler is null) - { - throw new ArgumentNullException(nameof(handler)); - } - + private static PosixSignalRegistration Register(PosixSignal signal, Action handler) => throw new PlatformNotSupportedException(); - } - public partial void Dispose() { } + private void Unregister() { } } } 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 f01b2de0130385..d91dd1c3a8dc75 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 @@ -2,265 +2,138 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Diagnostics; using System.Threading; namespace System.Runtime.InteropServices { public sealed partial class PosixSignalRegistration { - private static volatile bool s_initialized; - private static readonly Dictionary?>> s_registrations = new(); + private static readonly Dictionary> s_registrations = Initialize(); - private readonly Action _handler; - private readonly PosixSignal _signal; - private readonly int _signo; - private bool _registered; - private readonly object _gate = new object(); - - private PosixSignalRegistration(PosixSignal signal, int signo, Action handler) - { - _signal = signal; - _signo = signo; - _handler = handler; - } - - public static partial PosixSignalRegistration Create(PosixSignal signal, Action handler) + private static unsafe Dictionary> Initialize() { - if (handler == null) + if (!Interop.Sys.InitializeTerminalAndSignalHandling()) { - throw new ArgumentNullException(nameof(handler)); + Interop.CheckIo(-1); } - int signo = Interop.Sys.GetPlatformSignalNumber(signal); - if (signo == 0) - { - throw new PlatformNotSupportedException(); - } + Interop.Sys.SetPosixSignalHandler(&OnPosixSignal); - PosixSignalRegistration registration = new PosixSignalRegistration(signal, signo, handler); - registration.Register(); - return registration; + return new Dictionary>(); } - private unsafe void Register() + private static PosixSignalRegistration Register(PosixSignal signal, Action handler) { - if (!s_initialized) + int signo = Interop.Sys.GetPlatformSignalNumber(signal); + if (signo == 0) { - if (!Interop.Sys.InitializeTerminalAndSignalHandling()) - { - // We can't use Win32Exception because that causes a cycle with - // Microsoft.Win32.Primitives. - Interop.CheckIo(-1); - } - - Interop.Sys.SetPosixSignalHandler(&OnPosixSignal); - s_initialized = true; + throw new PlatformNotSupportedException(); } + var token = new Token(signal, signo, handler); + var registration = new PosixSignalRegistration(token); + lock (s_registrations) { - if (!s_registrations.TryGetValue(_signo, out List?>? signalRegistrations)) + if (!s_registrations.TryGetValue(signo, out HashSet? tokens)) { - signalRegistrations = new List?>(); - s_registrations.Add(_signo, signalRegistrations); + s_registrations[signo] = tokens = new HashSet(); } - if (signalRegistrations.Count == 0) + if (tokens.Count == 0 && + !Interop.Sys.EnablePosixSignalHandling(signo)) { - if (!Interop.Sys.EnablePosixSignalHandling(_signo)) - { - // We can't use Win32Exception because that causes a cycle with - // Microsoft.Win32.Primitives. - Interop.CheckIo(-1); - } + Interop.CheckIo(-1); } - signalRegistrations.Add(new WeakReference(this)); + tokens.Add(token); } - _registered = true; + return registration; } - private bool CallHandler(PosixSignalContext context) + private void Unregister() { - lock (_gate) + lock (s_registrations) { - if (_registered) + if (_token is Token token) { - _handler(context); - return true; - } + _token = null; - return false; + if (s_registrations.TryGetValue(token.SigNo, out HashSet? tokens)) + { + tokens.Remove(token); + if (tokens.Count == 0) + { + s_registrations.Remove(token.SigNo); + Interop.Sys.DisablePosixSignalHandling(token.SigNo); + } + } + } } } [UnmanagedCallersOnly] private static int OnPosixSignal(int signo, PosixSignal signal) { - PosixSignalRegistration?[]? registrations = GetRegistrations(signo); - if (registrations != null) - { - // This is called on the native signal handling thread. We need to move to another thread so - // signal handling is not blocked. Otherwise we may get deadlocked when the handler depends - // on work triggered from the signal handling thread. - - // For terminate/interrupt signals we use a dedicated Thread - // in case the ThreadPool is saturated. - bool useDedicatedThread = signal == PosixSignal.SIGINT || - signal == PosixSignal.SIGQUIT || - signal == PosixSignal.SIGTERM; + Token[]? tokens = null; - if (useDedicatedThread) - { - Thread handlerThread = new Thread(HandleSignal) - { - IsBackground = true, - Name = ".NET Signal Handler" - }; - handlerThread.UnsafeStart((signo, registrations)); - } - else + lock (s_registrations) + { + if (s_registrations.TryGetValue(signo, out HashSet? registrations)) { - ThreadPool.UnsafeQueueUserWorkItem(HandleSignal, (signo, registrations)); + tokens = new Token[registrations.Count]; + registrations.CopyTo(tokens); } - - return 1; } - return 0; - } - - private static PosixSignalRegistration?[]? GetRegistrations(int signo) - { - lock (s_registrations) + if (tokens is null) { - if (s_registrations.TryGetValue(signo, out List?>? signalRegistrations)) - { - if (signalRegistrations.Count != 0) - { - var registrations = new PosixSignalRegistration?[signalRegistrations.Count]; - bool hasRegistrations = false; - bool pruneWeakReferences = false; - - for (int i = 0; i < signalRegistrations.Count; i++) - { - if (signalRegistrations[i]!.TryGetTarget(out PosixSignalRegistration? registration)) - { - registrations[i] = registration; - hasRegistrations = true; - } - else - { - // WeakReference no longer holds an object. PosixSignalRegistration got finalized. - signalRegistrations[i] = null; - pruneWeakReferences = true; - } - } - - if (pruneWeakReferences) - { - signalRegistrations.RemoveAll(item => item is null); - } - - if (hasRegistrations) - { - return registrations; - } - else - { - Interop.Sys.DisablePosixSignalHandling(signo); - } - } - } - return null; + return 0; } - } - private static void HandleSignal(object? state) - { - HandleSignal(((int, PosixSignalRegistration?[]))state!); - } + Debug.Assert(tokens.Length != 0); - private static void HandleSignal((int signo, PosixSignalRegistration?[]? registrations) state) - { - do + // This is called on the native signal handling thread. We need to move to another thread so + // signal handling is not blocked. Otherwise we may get deadlocked when the handler depends + // on work triggered from the signal handling thread. + switch (signal) { - bool handlersCalled = false; - if (state.registrations != null) - { - PosixSignalContext ctx = new(0); - foreach (PosixSignalRegistration? registration in state.registrations) - { - if (registration != null) - { - // Different values for PosixSignal map to the same signo. - // Match the PosixSignal value used when registering. - ctx.Signal = registration._signal; - if (registration.CallHandler(ctx)) - { - handlersCalled = true; - } - } - } - - if (ctx.Cancel) + case PosixSignal.SIGINT: + case PosixSignal.SIGQUIT: + case PosixSignal.SIGTERM: + // For terminate/interrupt signals we use a dedicated Thread in case the ThreadPool is saturated. + new Thread(HandleSignal) { - return; - } - } + IsBackground = true, + Name = ".NET Signal Handler" + }.UnsafeStart((signo, tokens)); + break; - if (Interop.Sys.HandleNonCanceledPosixSignal(state.signo, handlersCalled ? 0 : 1)) - { - return; - } + default: + ThreadPool.UnsafeQueueUserWorkItem(HandleSignal, (signo, tokens)); + break; + } - // HandleNonCanceledPosixSignal returns false when handlers got registered. - state.registrations = GetRegistrations(state.signo); - } while (true); - } + return 1; - public partial void Dispose() - { - if (_registered) + static void HandleSignal(object? state) { - lock (s_registrations) - { - List?> signalRegistrations = s_registrations[_signo]; - bool pruneWeakReferences = false; - for (int i = 0; i < signalRegistrations.Count; i++) - { - if (signalRegistrations[i]!.TryGetTarget(out PosixSignalRegistration? registration)) - { - if (ReferenceEquals(this, registration)) - { - signalRegistrations.RemoveAt(i); - break; - } - } - else - { - // WeakReference no longer holds an object. PosixSignalRegistration got finalized. - signalRegistrations[i] = null; - pruneWeakReferences = true; - } - } + (int signo, Token[] tokens) = ((int, Token[]))state!; - if (pruneWeakReferences) - { - signalRegistrations.RemoveAll(item => item is null); - } - - if (signalRegistrations.Count == 0) - { - Interop.Sys.DisablePosixSignalHandling(_signo); - } + PosixSignalContext ctx = new(0); + foreach (Token token in tokens) + { + // Different values for PosixSignal map to the same signo. + // Match the PosixSignal value used when registering. + ctx.Signal = token.Signal; + token.Handler(ctx); } - // Synchronize with _handler invocations. - lock (_gate) + if (!ctx.Cancel) { - _registered = false; + 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 d3e5405053e56c..a50fca2bbee906 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 @@ -6,59 +6,51 @@ namespace System.Runtime.InteropServices { - public sealed unsafe partial class PosixSignalRegistration + public sealed partial class PosixSignalRegistration { - private static readonly HashSet s_handlers = new(); + private static readonly HashSet s_registrations = new(); - private Token? _token; - - private PosixSignalRegistration(Token token) => _token = token; - - private static object SyncObj => s_handlers; - - public static partial PosixSignalRegistration Create(PosixSignal signal, Action handler) + private static unsafe PosixSignalRegistration Register(PosixSignal signal, Action handler) { - if (handler is null) + switch (signal) { - throw new ArgumentNullException(nameof(handler)); + case PosixSignal.SIGINT: + case PosixSignal.SIGQUIT: + case PosixSignal.SIGTERM: + case PosixSignal.SIGHUP: + break; + + default: + throw new PlatformNotSupportedException(); } - lock (SyncObj) - { - switch (signal) - { - case PosixSignal.SIGINT: - case PosixSignal.SIGQUIT: - case PosixSignal.SIGTERM: - case PosixSignal.SIGHUP: - break; - - default: - throw new PlatformNotSupportedException(); - } + var token = new Token(signal, handler); + var registration = new PosixSignalRegistration(token); - if (s_handlers.Count == 0 && + lock (s_registrations) + { + if (s_registrations.Count == 0 && !Interop.Kernel32.SetConsoleCtrlHandler(&HandlerRoutine, Add: true)) { throw Win32Marshal.GetExceptionForLastWin32Error(); } - var token = new Token(signal, handler); - s_handlers.Add(token); - return new PosixSignalRegistration(token); + s_registrations.Add(token); } + + return registration; } - public partial void Dispose() + private unsafe void Unregister() { - lock (SyncObj) + lock (s_registrations) { if (_token is Token token) { _token = null; - s_handlers.Remove(token); - if (s_handlers.Count == 0 && + s_registrations.Remove(token); + if (s_registrations.Count == 0 && !Interop.Kernel32.SetConsoleCtrlHandler(&HandlerRoutine, Add: false)) { throw Win32Marshal.GetExceptionForLastWin32Error(); @@ -94,9 +86,9 @@ private static Interop.BOOL HandlerRoutine(int dwCtrlType) } List? tokens = null; - lock (SyncObj) + lock (s_registrations) { - foreach (Token token in s_handlers) + foreach (Token token in s_registrations) { if (token.Signal == signal) { @@ -118,17 +110,5 @@ private static Interop.BOOL HandlerRoutine(int dwCtrlType) return context.Cancel ? Interop.BOOL.TRUE : Interop.BOOL.FALSE; } - - private sealed class Token - { - public Token(PosixSignal signal, Action handler) - { - Signal = signal; - Handler = handler; - } - - public PosixSignal Signal { get; } - public Action Handler { get; } - } } } 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 fb2c675acb8b14..811d75ddd51822 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 @@ -9,6 +9,13 @@ namespace System.Runtime.InteropServices /// Handles a . public sealed partial class PosixSignalRegistration : IDisposable { + /// The state associated with this registration. + /// + /// This is separate from the registration instance so that this token may be stored + /// in a statically rooted table, with a finalizer on the registration able to remove it. + /// + private Token? _token; + /// Registers a that is invoked when the occurs. /// The signal to register for. /// The handler that gets invoked. @@ -28,11 +35,48 @@ public sealed partial class PosixSignalRegistration : IDisposable [UnsupportedOSPlatform("ios")] [UnsupportedOSPlatform("maccatalyst")] [UnsupportedOSPlatform("tvos")] - public static partial PosixSignalRegistration Create(PosixSignal signal, Action handler); + public static PosixSignalRegistration Create(PosixSignal signal, Action handler) + { + if (handler is null) + { + throw new ArgumentNullException(nameof(handler)); + } + + return Register(signal, handler); + } + + /// Initializes the registration to wrap the specified token. + private PosixSignalRegistration(Token token) => _token = token; /// Unregister the handler. - public partial void Dispose(); + public void Dispose() + { + Unregister(); + GC.SuppressFinalize(this); + } + + /// Unregister the handler. + ~PosixSignalRegistration() => Unregister(); + + /// The state associated with a registration. + private sealed class Token + { + public Token(PosixSignal signal, Action handler) + { + Signal = signal; + Handler = handler; + } + + public Token(PosixSignal signal, int sigNo, Action handler) + { + Signal = signal; + Handler = handler; + SigNo = sigNo; + } - ~PosixSignalRegistration() => Dispose(); + public PosixSignal Signal { get; } + public Action Handler { get; } + public int SigNo { get; } + } } }