From 11a94bbf72122721c83c0d4beed8591f3a22cc10 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 9 Jun 2021 15:09:04 +0200 Subject: [PATCH 01/65] Implement PosixSignal API --- .../Unix/System.Native/Interop.PosixSignal.cs | 22 ++ .../System.Native/Interop.RegisterForCtrlC.cs | 24 --- .../Interop.RestoreAndHandleCtrl.cs | 13 -- .../Native/Unix/System.Native/entrypoints.c | 6 +- .../Native/Unix/System.Native/pal_signal.c | 202 +++++++++++++----- .../Native/Unix/System.Native/pal_signal.h | 39 ++-- .../System.Console/src/System.Console.csproj | 7 - .../System.Console/src/System/Console.cs | 3 +- .../src/System/ConsolePal.Unix.cs | 38 +--- .../ref/System.Runtime.InteropServices.cs | 20 ++ .../src/System.Runtime.InteropServices.csproj | 20 +- .../Runtime/InteropServices/PosixSignal.cs | 14 ++ .../InteropServices/PosixSignalContext.cs | 24 +++ .../PosixSignalRegistration.Unix.cs | 144 +++++++++++++ .../PosixSignalRegistration.cs | 18 ++ 15 files changed, 447 insertions(+), 147 deletions(-) create mode 100644 src/libraries/Common/src/Interop/Unix/System.Native/Interop.PosixSignal.cs delete mode 100644 src/libraries/Common/src/Interop/Unix/System.Native/Interop.RegisterForCtrlC.cs delete mode 100644 src/libraries/Common/src/Interop/Unix/System.Native/Interop.RestoreAndHandleCtrl.cs create mode 100644 src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignal.cs create mode 100644 src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalContext.cs create mode 100644 src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs create mode 100644 src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.cs 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 new file mode 100644 index 0000000000000..dfeb44469c9d7 --- /dev/null +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.PosixSignal.cs @@ -0,0 +1,22 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.InteropServices; + +internal static partial class Interop +{ + internal static partial class Sys + { + [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_RegisterForPosixSignal")] + [SuppressGCTransition] + internal static extern unsafe bool RegisterForPosixSignal(PosixSignal signal, delegate* unmanaged handler); + + [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_UnregisterForPosixSignal")] + [SuppressGCTransition] + internal static extern void UnregisterForPosixSignal(PosixSignal signal); + + [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_HandlePosixSignal")] + [SuppressGCTransition] + internal static extern void HandlePosixSignal(PosixSignal signal); + } +} diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.RegisterForCtrlC.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.RegisterForCtrlC.cs deleted file mode 100644 index cb06c67651c0c..0000000000000 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.RegisterForCtrlC.cs +++ /dev/null @@ -1,24 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Runtime.InteropServices; - -internal static partial class Interop -{ - internal static partial class Sys - { - internal enum CtrlCode - { - Interrupt = 0, - Break = 1 - } - - [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_RegisterForCtrl")] - [SuppressGCTransition] - internal static extern unsafe void RegisterForCtrl(delegate* unmanaged handler); - - [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_UnregisterForCtrl")] - [SuppressGCTransition] - internal static extern void UnregisterForCtrl(); - } -} diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.RestoreAndHandleCtrl.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.RestoreAndHandleCtrl.cs deleted file mode 100644 index 927c2d2706833..0000000000000 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.RestoreAndHandleCtrl.cs +++ /dev/null @@ -1,13 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Runtime.InteropServices; - -internal static partial class Interop -{ - internal static partial class Sys - { - [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_RestoreAndHandleCtrl")] - internal static extern void RestoreAndHandleCtrl(CtrlCode ctrlCode); - } -} diff --git a/src/libraries/Native/Unix/System.Native/entrypoints.c b/src/libraries/Native/Unix/System.Native/entrypoints.c index cf6485ecf8430..e2bcdd1ba297f 100644 --- a/src/libraries/Native/Unix/System.Native/entrypoints.c +++ b/src/libraries/Native/Unix/System.Native/entrypoints.c @@ -216,10 +216,7 @@ static const Entry s_sysNative[] = DllImportEntry(SystemNative_GetOSArchitecture) DllImportEntry(SystemNative_GetProcessArchitecture) DllImportEntry(SystemNative_SearchPath) - DllImportEntry(SystemNative_RegisterForCtrl) - DllImportEntry(SystemNative_UnregisterForCtrl) DllImportEntry(SystemNative_RegisterForSigChld) - DllImportEntry(SystemNative_RestoreAndHandleCtrl) DllImportEntry(SystemNative_SetTerminalInvalidationHandler) DllImportEntry(SystemNative_InitializeTerminalAndSignalHandling) DllImportEntry(SystemNative_SNPrintF) @@ -251,6 +248,9 @@ static const Entry s_sysNative[] = DllImportEntry(SystemNative_PWrite) DllImportEntry(SystemNative_PReadV) DllImportEntry(SystemNative_PWriteV) + DllImportEntry(SystemNative_RegisterForPosixSignal) + DllImportEntry(SystemNative_UnregisterForPosixSignal) + DllImportEntry(SystemNative_HandlePosixSignal) }; EXTERN_C const void* SystemResolveDllImport(const char* name); diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index dfe30adb9e89f..4ad2274579d3e 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -17,23 +17,105 @@ #include static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER; -static struct sigaction g_origSigIntHandler, g_origSigQuitHandler; // saved signal handlers for ctrl handling -static struct sigaction g_origSigContHandler, g_origSigChldHandler; // saved signal handlers for reinitialization -static struct sigaction g_origSigWinchHandler; // saved signal handlers for SIGWINCH -static volatile CtrlCallback g_ctrlCallback = NULL; // Callback invoked for SIGINT/SIGQUIT -static volatile TerminalInvalidationCallback g_terminalInvalidationCallback = NULL; // Callback invoked for SIGCHLD/SIGCONT/SIGWINCH -static volatile SigChldCallback g_sigChldCallback = NULL; // Callback invoked for SIGCHLD + +// Saved signal handlers +static struct sigaction g_origSigIntHandler; +static struct sigaction g_origSigQuitHandler; +static struct sigaction g_origSigIntHandler; +static struct sigaction g_origSigTermHandler; +static struct sigaction g_origSigContHandler; +static struct sigaction g_origSigChldHandler; +static struct sigaction g_origSigWinchHandler; +static struct sigaction g_origSigHupHandler; + +// Callback invoked for SIGCHLD/SIGCONT/SIGWINCH +static volatile TerminalInvalidationCallback g_terminalInvalidationCallback = NULL; +// Callback invoked for SIGCHLD +static volatile SigChldCallback g_sigChldCallback = NULL; +// Callback invoked for PosixSignal handling. +static PosixSignalHandler g_posixSignalHandler = NULL; + static int g_signalPipe[2] = {-1, -1}; // Pipe used between signal handler and worker +static volatile bool g_signalHasRegistrations[PosixSignalCount]; + +static bool TryConvertSignalCodeToPosixSignal(int signalCode, PosixSignal* posixSignal) +{ + assert(posixSignal != NULL); + + switch (signalCode) + { + case SIGHUP: + *posixSignal = PosixSignalSIGHUP; + return true; + + case SIGINT: + *posixSignal = PosixSignalSIGINT; + return true; + + case SIGQUIT: + *posixSignal = PosixSignalSIGQUIT; + return true; + + case SIGTERM: + *posixSignal = PosixSignalSIGTERM; + return true; + + case SIGCHLD: + *posixSignal = PosixSignalSIGCHLD; + return true; + + default: + *posixSignal = signalCode; + return false; + } +} + +static bool TryConvertPosixSignalToSignalCode(PosixSignal signal, int* signalCode) +{ + assert(signalCode != NULL); + + switch (signal) + { + case PosixSignalSIGHUP: + *signalCode = SIGHUP; + return true; + + case PosixSignalSIGINT: + *signalCode = SIGINT; + return true; + + case PosixSignalSIGQUIT: + *signalCode = SIGQUIT; + return true; + + case PosixSignalSIGTERM: + *signalCode = SIGTERM; + return true; + + case PosixSignalSIGCHLD: + *signalCode = SIGCHLD; + return true; + + case PosixSignalCount: + break; + } + + *signalCode = signal; + return false; +} + static struct sigaction* OrigActionFor(int sig) { switch (sig) { - case SIGINT: return &g_origSigIntHandler; + case SIGINT: return &g_origSigIntHandler; + case SIGTERM: return &g_origSigTermHandler; case SIGQUIT: return &g_origSigQuitHandler; case SIGCONT: return &g_origSigContHandler; case SIGCHLD: return &g_origSigChldHandler; case SIGWINCH: return &g_origSigWinchHandler; + case SIGHUP: return &g_origSigHupHandler; } assert(false); @@ -106,21 +188,7 @@ static void* SignalHandlerLoop(void* arg) } } - if (signalCode == SIGQUIT || signalCode == SIGINT) - { - // We're now handling SIGQUIT and SIGINT. Invoke the callback, if we have one. - CtrlCallback callback = g_ctrlCallback; - CtrlCode ctrlCode = signalCode == SIGQUIT ? Break : Interrupt; - if (callback != NULL) - { - callback(ctrlCode); - } - else - { - SystemNative_RestoreAndHandleCtrl(ctrlCode); - } - } - else if (signalCode == SIGCHLD) + if (signalCode == SIGCHLD) { // When the original disposition is SIG_IGN, children that terminated did not become zombies. // Since we overwrote the disposition, we have become responsible for reaping those processes. @@ -154,13 +222,25 @@ static void* SignalHandlerLoop(void* arg) } else if (signalCode == SIGCONT) { + // TODO: should this be cancelable? #ifdef HAS_CONSOLE_SIGNALS ReinitializeTerminal(); #endif } - else if (signalCode != SIGWINCH) + + PosixSignal signal; + if (TryConvertSignalCodeToPosixSignal(signalCode, &signal)) { - assert_msg(false, "invalid signalCode", (int)signalCode); + bool hasManagedHandler = g_signalHasRegistrations[-signal]; + if (hasManagedHandler) + { + assert(g_posixSignalHandler != NULL); + hasManagedHandler = g_posixSignalHandler(signal) != 0; + } + if (!hasManagedHandler) + { + SystemNative_HandlePosixSignal(signal); + } } } } @@ -175,29 +255,6 @@ static void CloseSignalHandlingPipe() g_signalPipe[1] = -1; } -void SystemNative_RegisterForCtrl(CtrlCallback callback) -{ - assert(callback != NULL); - assert(g_ctrlCallback == NULL); - g_ctrlCallback = callback; -} - -void SystemNative_UnregisterForCtrl() -{ - assert(g_ctrlCallback != NULL); - g_ctrlCallback = NULL; -} - -void SystemNative_RestoreAndHandleCtrl(CtrlCode ctrlCode) -{ - int signalCode = ctrlCode == Break ? SIGQUIT : SIGINT; -#ifdef HAS_CONSOLE_SIGNALS - UninitializeTerminal(); -#endif - sigaction(signalCode, OrigActionFor(signalCode), NULL); - kill(getpid(), signalCode); -} - void SystemNative_SetTerminalInvalidationHandler(TerminalInvalidationCallback callback) { assert(callback != NULL); @@ -309,10 +366,14 @@ int32_t InitializeSignalHandlingCore() } // Finally, register our signal handlers - // We don't handle ignored SIGINT/SIGQUIT signals. If we'd setup a handler, our child - // processes would reset to the default on exec causing them to terminate on these signals. + // We don't handle ignored SIGINT/SIGQUIT/SIGHUP/SIGTERM signals. If we'd + // setup a handler, our child processes would reset to the default on exec + // causing them to terminate on these signals. InstallSignalHandler(SIGINT , /* skipWhenSigIgn */ true); InstallSignalHandler(SIGQUIT, /* skipWhenSigIgn */ true); + InstallSignalHandler(SIGHUP, /* skipWhenSigIgn */ true); + InstallSignalHandler(SIGTERM, /* skipWhenSigIgn */ true); + InstallSignalHandler(SIGCONT, /* skipWhenSigIgn */ false); InstallSignalHandler(SIGCHLD, /* skipWhenSigIgn */ false); InstallSignalHandler(SIGWINCH, /* skipWhenSigIgn */ false); @@ -320,10 +381,53 @@ int32_t InitializeSignalHandlingCore() return 1; } +int32_t SystemNative_RegisterForPosixSignal(PosixSignal signal, PosixSignalHandler signalHandler) +{ + assert(signalHandler != NULL); + assert(g_posixSignalHandler == NULL || g_posixSignalHandler == signalHandler); + + if (signal >= 0 || -signal > PosixSignalCount) + { + errno = EINVAL; + return 0; + } + + g_posixSignalHandler = signalHandler; + g_signalHasRegistrations[-signal] = true; + + return 1; +} + +void SystemNative_UnregisterForPosixSignal(PosixSignal signal) +{ + assert(signal < 0 && -signal <= PosixSignalCount); + + g_signalHasRegistrations[-signal] = false; +} + +void SystemNative_HandlePosixSignal(PosixSignal signal) +{ + if (signal == PosixSignalSIGQUIT || + signal == PosixSignalSIGINT || + signal == PosixSignalSIGHUP || + signal == PosixSignalSIGTERM) + { +#ifdef HAS_CONSOLE_SIGNALS + UninitializeTerminal(); +#endif + // Restore the original signal handler and invoke it. + int signalCode; + TryConvertPosixSignalToSignalCode(signal, &signalCode); + sigaction(signalCode, OrigActionFor(signalCode), NULL); + kill(getpid(), signalCode); + } +} + #ifndef HAS_CONSOLE_SIGNALS int32_t SystemNative_InitializeTerminalAndSignalHandling() { + errno = ENOSYS; return 0; } diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.h b/src/libraries/Native/Unix/System.Native/pal_signal.h index 6b0882c0413e1..cbf1ba68b6ec8 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.h +++ b/src/libraries/Native/Unix/System.Native/pal_signal.h @@ -13,28 +13,6 @@ */ int32_t InitializeSignalHandlingCore(void); -/** - * Hooks up the specified callback for notifications when SIGINT or SIGQUIT is received. - * - * Not thread safe. Caller must provide its owns synchronization to ensure RegisterForCtrl - * is not called concurrently with itself or with UnregisterForCtrl. - * - * Should only be called when a callback is not currently registered. - */ -PALEXPORT void SystemNative_RegisterForCtrl(CtrlCallback callback); - -/** - * Unregisters the previously registered ctrlCCallback. - * - * Not thread safe. Caller must provide its owns synchronization to ensure UnregisterForCtrl - * is not called concurrently with itself or with RegisterForCtrl. - * - * Should only be called when a callback is currently registered. The pointer - * previously registered must remain valid until all ctrl handling activity - * has quiesced. - */ -PALEXPORT void SystemNative_UnregisterForCtrl(void); - typedef void (*SigChldCallback)(int reapAll); /** @@ -60,6 +38,23 @@ typedef void (*TerminalInvalidationCallback)(void); */ PALEXPORT void SystemNative_SetTerminalInvalidationHandler(TerminalInvalidationCallback callback); +typedef enum +{ + PosixSignalSIGHUP = -1, + PosixSignalSIGINT = -2, + PosixSignalSIGQUIT = -3, + PosixSignalSIGTERM = -4, + PosixSignalSIGCHLD = -5, + + PosixSignalCount = 5 +} PosixSignal; + +typedef int32_t (*PosixSignalHandler)(PosixSignal signal); + +PALEXPORT int32_t SystemNative_RegisterForPosixSignal(PosixSignal signal, PosixSignalHandler signalHandler); +PALEXPORT void SystemNative_UnregisterForPosixSignal(PosixSignal signal); +PALEXPORT void SystemNative_HandlePosixSignal(PosixSignal signal); + #ifndef HAS_CONSOLE_SIGNALS /** diff --git a/src/libraries/System.Console/src/System.Console.csproj b/src/libraries/System.Console/src/System.Console.csproj index b4b5eb0bcb247..87a0fc8b164bc 100644 --- a/src/libraries/System.Console/src/System.Console.csproj +++ b/src/libraries/System.Console/src/System.Console.csproj @@ -196,10 +196,6 @@ Link="Common\Interop\Unix\Interop.GetEUid.cs" /> - - - - - diff --git a/src/libraries/System.Console/src/System/Console.cs b/src/libraries/System.Console/src/System/Console.cs index 6bbcb9f0dea41..097e4ac9b80a5 100644 --- a/src/libraries/System.Console/src/System/Console.cs +++ b/src/libraries/System.Console/src/System/Console.cs @@ -960,7 +960,7 @@ public static void Write(string? value) Out.Write(value); } - internal static bool HandleBreakEvent(ConsoleSpecialKey controlKey) + internal static bool HandleBreakEvent(ConsoleSpecialKey controlKey, bool cancel = false) { ConsoleCancelEventHandler? handler = s_cancelCallbacks; if (handler == null) @@ -969,6 +969,7 @@ internal static bool HandleBreakEvent(ConsoleSpecialKey controlKey) } var args = new ConsoleCancelEventArgs(controlKey); + args.Cancel = cancel; handler(null, args); return args.Cancel; } diff --git a/src/libraries/System.Console/src/System/ConsolePal.Unix.cs b/src/libraries/System.Console/src/System/ConsolePal.Unix.cs index d581ae795ce5b..49d5b108392d6 100644 --- a/src/libraries/System.Console/src/System/ConsolePal.Unix.cs +++ b/src/libraries/System.Console/src/System/ConsolePal.Unix.cs @@ -1445,49 +1445,33 @@ public override void Flush() internal sealed class ControlCHandlerRegistrar { private bool _handlerRegistered; + private IDisposable? _sigIntRegistration; + private IDisposable? _sigQuitRegistration; internal unsafe void Register() { Debug.Assert(s_initialized); // by CancelKeyPress add. Debug.Assert(!_handlerRegistered); - Interop.Sys.RegisterForCtrl(&OnBreakEvent); + _sigIntRegistration = PosixSignalRegistration.Create(PosixSignal.SIGINT, HandlePosixSignal); + _sigQuitRegistration = PosixSignalRegistration.Create(PosixSignal.SIGQUIT, HandlePosixSignal); _handlerRegistered = true; } internal void Unregister() { Debug.Assert(_handlerRegistered); - _handlerRegistered = false; - Interop.Sys.UnregisterForCtrl(); + _sigIntRegistration?.Dispose(); + _sigQuitRegistration?.Dispose(); } - [UnmanagedCallersOnly] - private static void OnBreakEvent(Interop.Sys.CtrlCode ctrlCode) + private static void HandlePosixSignal(PosixSignalContext ctx) { - // 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. - // We use a new thread rather than queueing to the ThreadPool in order to prioritize handling - // in case the ThreadPool is saturated. - Thread handlerThread = new Thread(HandleBreakEvent) - { - IsBackground = true, - Name = ".NET Console Break" - }; - handlerThread.Start(ctrlCode); - } + Debug.Assert(ctx.Signal == PosixSignal.SIGINT || ctx.Signal == PosixSignal.SIGQUIT); - private static void HandleBreakEvent(object? state) - { - Debug.Assert(state != null); - var ctrlCode = (Interop.Sys.CtrlCode)state; - ConsoleSpecialKey controlKey = (ctrlCode == Interop.Sys.CtrlCode.Break ? ConsoleSpecialKey.ControlBreak : ConsoleSpecialKey.ControlC); - bool cancel = Console.HandleBreakEvent(controlKey); - if (!cancel) - { - Interop.Sys.RestoreAndHandleCtrl(ctrlCode); - } + ConsoleSpecialKey controlKey = ctx.Signal == PosixSignal.SIGINT ? ConsoleSpecialKey.ControlC : ConsoleSpecialKey.ControlBreak; + bool cancel = Console.HandleBreakEvent(controlKey, ctx.Cancel); + ctx.Cancel = cancel; } } diff --git a/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs b/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs index 02e739ad73173..b6cd1e702cc27 100644 --- a/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs +++ b/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs @@ -796,6 +796,26 @@ public sealed partial class OptionalAttribute : System.Attribute { public OptionalAttribute() { } } + public enum PosixSignal + { + SIGHUP = -1, + SIGINT = -2, + SIGQUIT = -3, + SIGTERM = -4, + SIGCHLD = -5 + } + public sealed class PosixSignalContext + { + public PosixSignal Signal { get { throw null; } } + public bool Cancel { get { throw null; } set { throw null; } } + public PosixSignalContext(PosixSignal signal) { } + } + public sealed class PosixSignalRegistration : IDisposable + { + private PosixSignalRegistration() { throw null; } + public static PosixSignalRegistration Create(PosixSignal signal, Action handler) { throw null; } + public void Dispose() { throw null; } + } [System.AttributeUsageAttribute(System.AttributeTargets.Method, Inherited=false)] public sealed partial class PreserveSigAttribute : System.Attribute { diff --git a/src/libraries/System.Runtime.InteropServices/src/System.Runtime.InteropServices.csproj b/src/libraries/System.Runtime.InteropServices/src/System.Runtime.InteropServices.csproj index 0565ac47a77c1..a03881f0a30b8 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System.Runtime.InteropServices.csproj +++ b/src/libraries/System.Runtime.InteropServices/src/System.Runtime.InteropServices.csproj @@ -2,7 +2,7 @@ true enable - $(NetCoreAppCurrent) + $(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Android;$(NetCoreAppCurrent)-iOS;$(NetCoreAppCurrent)-tvOS;$(NetCoreAppCurrent)-Browser @@ -33,6 +33,8 @@ + + @@ -48,7 +50,23 @@ + + + + + + + + + + + + + \ No newline at end of file diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignal.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignal.cs new file mode 100644 index 0000000000000..7a5a74488299b --- /dev/null +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignal.cs @@ -0,0 +1,14 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Runtime.InteropServices +{ + public enum PosixSignal + { + SIGHUP = -1, + SIGINT = -2, + SIGQUIT = -3, + SIGTERM = -4, + SIGCHLD = -5 + } +} diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalContext.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalContext.cs new file mode 100644 index 0000000000000..d6cb5a1bf6843 --- /dev/null +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalContext.cs @@ -0,0 +1,24 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Runtime.InteropServices +{ + public sealed class PosixSignalContext + { + public PosixSignal Signal + { + get; + } + + public bool Cancel + { + get; + set; // TODO: should this throw if Canceling doesn't do anything? + } + + public PosixSignalContext(PosixSignal signal) + { + Signal = signal; + } + } +} diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs new file mode 100644 index 0000000000000..16759c8828d33 --- /dev/null +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs @@ -0,0 +1,144 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.ComponentModel; +using System.Threading; + +namespace System.Runtime.InteropServices +{ + public sealed class PosixSignalRegistration : IDisposable + { + private readonly Action _handler; + private readonly PosixSignal _signal; + private bool _registered; + private readonly object _gate = new object(); + + private PosixSignalRegistration(PosixSignal signal, Action handler) + { + _signal = signal; + _handler = handler; + } + + public static PosixSignalRegistration Create(PosixSignal signal, Action handler) + { + if (handler == null) + { + throw new ArgumentNullException(nameof(handler)); + } + if (signal > PosixSignal.SIGHUP || signal < PosixSignal.SIGCHLD) + { + throw new IndexOutOfRangeException(); + } + PosixSignalRegistration registration = new PosixSignalRegistration(signal, handler); + registration.RegisterFor(signal); + return registration; + } + + private unsafe void RegisterFor(PosixSignal signal) + { + if (!s_initialized) + { + if (!Interop.Sys.InitializeTerminalAndSignalHandling()) + { + throw new Win32Exception(); + } + s_initialized = true; + } + lock (s_registrations) + { + if (!s_registrations.TryGetValue(signal, out List? signalRegistrations)) + { + if (!Interop.Sys.RegisterForPosixSignal(signal, &OnPosixSignal)) + { + throw new Win32Exception(); + } + signalRegistrations = new List(); + s_registrations.Add(signal, signalRegistrations); + } + signalRegistrations.Add(this); + } + _registered = true; + } + + private void Handle(PosixSignalContext context) + { + lock (_gate) + { + if (_registered) + { + _handler(context); + } + } + } + + [UnmanagedCallersOnly] + private static int OnPosixSignal(PosixSignal signal) + { + lock (s_registrations) + { + if (s_registrations.TryGetValue(signal, out List? signalRegistrations)) + { + if (signalRegistrations.Count != 0) + { + PosixSignalRegistration[] registrations = signalRegistrations.ToArray(); + // 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. + // We use a new thread rather than queueing to the ThreadPool in order to prioritize handling + // in case the ThreadPool is saturated. + Thread handlerThread = new Thread(HandleSignal) + { + IsBackground = true, + Name = ".NET Signal Handler" + }; + handlerThread.Start((signal, registrations)); + return 1; + } + } + } + return 0; + } + + private static void HandleSignal(object? state) + { + var (signal, registrations) = ((PosixSignal, PosixSignalRegistration[]))state!; + + PosixSignalContext ctx = new(signal); + foreach (var registration in registrations) + { + registration.Handle(ctx); + } + + if (!ctx.Cancel) + { + Interop.Sys.HandlePosixSignal(signal); + } + } + + public void Dispose() + { + if (_registered) + { + lock (s_registrations) + { + List signalRegistrations = s_registrations[_signal]; + signalRegistrations.Remove(this); + if (signalRegistrations.Count == 0) + { + Interop.Sys.UnregisterForPosixSignal(_signal); + } + } + + lock (_gate) + { + _registered = false; + } + } + } + + private static volatile bool s_initialized; + private static Dictionary> s_registrations = new(); + } +} diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.cs new file mode 100644 index 0000000000000..7c88a92cae944 --- /dev/null +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.cs @@ -0,0 +1,18 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; + +namespace System.Runtime.InteropServices +{ + public sealed class PosixSignalRegistration : IDisposable + { + private PosixSignalRegistration() { } + + public static PosixSignalRegistration Create(PosixSignal signal, Action handler) + => throw new PlatformNotSupportedException(); + + public void Dispose() + => throw new PlatformNotSupportedException(); + } +} From ae4d67e1c9ba7adf6464f6039fecbb2fd0c7fe13 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 14 Jun 2021 12:16:44 +0200 Subject: [PATCH 02/65] PosixSignalRegistration: implement finalizer --- .../PosixSignalRegistration.Unix.cs | 71 +++++++++++++------ 1 file changed, 50 insertions(+), 21 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs index 16759c8828d33..1a305ab7c8986 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs @@ -48,16 +48,16 @@ private unsafe void RegisterFor(PosixSignal signal) } lock (s_registrations) { - if (!s_registrations.TryGetValue(signal, out List? signalRegistrations)) + if (!s_registrations.TryGetValue(signal, out List>? signalRegistrations)) { if (!Interop.Sys.RegisterForPosixSignal(signal, &OnPosixSignal)) { throw new Win32Exception(); } - signalRegistrations = new List(); + signalRegistrations = new List>(); s_registrations.Add(signal, signalRegistrations); } - signalRegistrations.Add(this); + signalRegistrations.Add(new WeakReference(this)); } _registered = true; } @@ -78,23 +78,35 @@ private static int OnPosixSignal(PosixSignal signal) { lock (s_registrations) { - if (s_registrations.TryGetValue(signal, out List? signalRegistrations)) + if (s_registrations.TryGetValue(signal, out List>? signalRegistrations)) { if (signalRegistrations.Count != 0) { - PosixSignalRegistration[] registrations = signalRegistrations.ToArray(); - // 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. - // We use a new thread rather than queueing to the ThreadPool in order to prioritize handling - // in case the ThreadPool is saturated. - Thread handlerThread = new Thread(HandleSignal) + var registrations = new PosixSignalRegistration?[signalRegistrations.Count]; + bool hasRegistrations = false; + for (int i = 0; i < signalRegistrations.Count; i++) { - IsBackground = true, - Name = ".NET Signal Handler" - }; - handlerThread.Start((signal, registrations)); - return 1; + if (signalRegistrations[i].TryGetTarget(out PosixSignalRegistration? registration)) + { + registrations[i] = registration; + hasRegistrations = true; + } + } + if (hasRegistrations) + { + // 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. + // We use a new thread rather than queueing to the ThreadPool in order to prioritize handling + // in case the ThreadPool is saturated. + Thread handlerThread = new Thread(HandleSignal) + { + IsBackground = true, + Name = ".NET Signal Handler" + }; + handlerThread.Start((signal, registrations)); + return 1; + } } } } @@ -103,12 +115,12 @@ private static int OnPosixSignal(PosixSignal signal) private static void HandleSignal(object? state) { - var (signal, registrations) = ((PosixSignal, PosixSignalRegistration[]))state!; + var (signal, registrations) = ((PosixSignal, PosixSignalRegistration?[]))state!; PosixSignalContext ctx = new(signal); foreach (var registration in registrations) { - registration.Handle(ctx); + registration?.Handle(ctx); } if (!ctx.Cancel) @@ -117,14 +129,31 @@ private static void HandleSignal(object? state) } } + ~PosixSignalRegistration() + => Dispose(false); + public void Dispose() + { + Dispose(true); + GC.SuppressFinalize(this); + } + + private void Dispose(bool disposing) { if (_registered) { lock (s_registrations) { - List signalRegistrations = s_registrations[_signal]; - signalRegistrations.Remove(this); + List> signalRegistrations = s_registrations[_signal]; + for (int i = 0; i < signalRegistrations.Count; i++) + { + if (signalRegistrations[i].TryGetTarget(out PosixSignalRegistration? registration) && + object.ReferenceEquals(this, registration)) + { + signalRegistrations.RemoveAt(i); + break; + } + } if (signalRegistrations.Count == 0) { Interop.Sys.UnregisterForPosixSignal(_signal); @@ -139,6 +168,6 @@ public void Dispose() } private static volatile bool s_initialized; - private static Dictionary> s_registrations = new(); + private static Dictionary>> s_registrations = new(); } } From 72a24758ce860a6b4e2a9dd43f65a35c9126c816 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 14 Jun 2021 17:38:14 +0200 Subject: [PATCH 03/65] Support using positive PosixSignal values as raw signal numbers --- .../Unix/System.Native/Interop.PosixSignal.cs | 20 +- .../Native/Unix/System.Native/entrypoints.c | 5 + .../Native/Unix/System.Native/pal_signal.c | 210 +++++++----------- .../Native/Unix/System.Native/pal_signal.h | 14 +- .../InteropServices/PosixSignalContext.cs | 6 +- .../PosixSignalRegistration.Unix.cs | 56 +++-- 6 files changed, 145 insertions(+), 166 deletions(-) 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 dfeb44469c9d7..4c42728b653a2 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 @@ -7,16 +7,24 @@ internal static partial class Interop { internal static partial class Sys { - [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_RegisterForPosixSignal")] + [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_SetPosixSignalHandler")] [SuppressGCTransition] - internal static extern unsafe bool RegisterForPosixSignal(PosixSignal signal, delegate* unmanaged handler); + internal static extern unsafe void SetPosixSignalHandler(delegate* unmanaged handler); - [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_UnregisterForPosixSignal")] + [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_EnablePosixSignalHandling")] [SuppressGCTransition] - internal static extern void UnregisterForPosixSignal(PosixSignal signal); + internal static extern void EnablePosixSignalHandling(int signal); - [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_HandlePosixSignal")] + [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_DisablePosixSignalHandling")] [SuppressGCTransition] - internal static extern void HandlePosixSignal(PosixSignal signal); + internal static extern void DisablePosixSignalHandling(int signal); + + [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_DefaultSignalHandler")] + [SuppressGCTransition] + internal static extern void DefaultSignalHandler(int signal); + + [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetPlatformSignalNumber")] + [SuppressGCTransition] + internal static extern int GetPlatformSignalNumber(PosixSignal signal); } } diff --git a/src/libraries/Native/Unix/System.Native/entrypoints.c b/src/libraries/Native/Unix/System.Native/entrypoints.c index e2bcdd1ba297f..70c1d61e61e45 100644 --- a/src/libraries/Native/Unix/System.Native/entrypoints.c +++ b/src/libraries/Native/Unix/System.Native/entrypoints.c @@ -251,6 +251,11 @@ static const Entry s_sysNative[] = DllImportEntry(SystemNative_RegisterForPosixSignal) DllImportEntry(SystemNative_UnregisterForPosixSignal) DllImportEntry(SystemNative_HandlePosixSignal) + DllImportEntry(SystemNative_EnablePosixSignalHandling) + DllImportEntry(SystemNative_DisablePosixSignalHandling) + DllImportEntry(SystemNative_DefaultSignalHandler) + DllImportEntry(SystemNative_SetPosixSignalHandler) + DllImportEntry(SystemNative_GetPlatformSignalNumber) }; EXTERN_C const void* SystemResolveDllImport(const char* name); diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index 4ad2274579d3e..1e186cc6eab08 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -19,14 +19,9 @@ static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER; // Saved signal handlers -static struct sigaction g_origSigIntHandler; -static struct sigaction g_origSigQuitHandler; -static struct sigaction g_origSigIntHandler; -static struct sigaction g_origSigTermHandler; -static struct sigaction g_origSigContHandler; -static struct sigaction g_origSigChldHandler; -static struct sigaction g_origSigWinchHandler; -static struct sigaction g_origSigHupHandler; +static struct sigaction g_origSigHandler[NSIG]; +static bool g_origSigHandlerIsSet[NSIG]; + // Callback invoked for SIGCHLD/SIGCONT/SIGWINCH static volatile TerminalInvalidationCallback g_terminalInvalidationCallback = NULL; @@ -34,92 +29,52 @@ static volatile TerminalInvalidationCallback g_terminalInvalidationCallback = NU static volatile SigChldCallback g_sigChldCallback = NULL; // Callback invoked for PosixSignal handling. static PosixSignalHandler g_posixSignalHandler = NULL; +// Tracks whether there are PosixSignal handlers registered. +static volatile bool g_hasPosixSignalRegistrations[NSIG]; static int g_signalPipe[2] = {-1, -1}; // Pipe used between signal handler and worker -static volatile bool g_signalHasRegistrations[PosixSignalCount]; - -static bool TryConvertSignalCodeToPosixSignal(int signalCode, PosixSignal* posixSignal) +int SystemNative_GetPlatformSignalNumber(PosixSignal signal) { - assert(posixSignal != NULL); - - switch (signalCode) - { - case SIGHUP: - *posixSignal = PosixSignalSIGHUP; - return true; - - case SIGINT: - *posixSignal = PosixSignalSIGINT; - return true; - - case SIGQUIT: - *posixSignal = PosixSignalSIGQUIT; - return true; - - case SIGTERM: - *posixSignal = PosixSignalSIGTERM; - return true; - - case SIGCHLD: - *posixSignal = PosixSignalSIGCHLD; - return true; - - default: - *posixSignal = signalCode; - return false; - } -} - -static bool TryConvertPosixSignalToSignalCode(PosixSignal signal, int* signalCode) -{ - assert(signalCode != NULL); - switch (signal) { case PosixSignalSIGHUP: - *signalCode = SIGHUP; - return true; + return SIGHUP; case PosixSignalSIGINT: - *signalCode = SIGINT; - return true; + return SIGINT; case PosixSignalSIGQUIT: - *signalCode = SIGQUIT; - return true; + return SIGQUIT; case PosixSignalSIGTERM: - *signalCode = SIGTERM; - return true; + return SIGTERM; case PosixSignalSIGCHLD: - *signalCode = SIGCHLD; - return true; + return SIGCHLD; + } - case PosixSignalCount: - break; + if (signal > 0 && + signal <= NSIG && // Ensure we stay within the static arrays. + signal <= SIGRTMAX) // Runtime check for highest value. + { + return signal; } - *signalCode = signal; - return false; + return 0; +} + +void SystemNative_SetPosixSignalHandler(PosixSignalHandler signalHandler) +{ + assert(signalHandler); + assert(g_posixSignalHandler == NULL || g_posixSignalHandler == signalHandler); + + g_posixSignalHandler = signalHandler; } static struct sigaction* OrigActionFor(int sig) { - switch (sig) - { - case SIGINT: return &g_origSigIntHandler; - case SIGTERM: return &g_origSigTermHandler; - case SIGQUIT: return &g_origSigQuitHandler; - case SIGCONT: return &g_origSigContHandler; - case SIGCHLD: return &g_origSigChldHandler; - case SIGWINCH: return &g_origSigWinchHandler; - case SIGHUP: return &g_origSigHupHandler; - } - - assert(false); - return NULL; + return &g_origSigHandler[sig - 1]; } static void SignalHandler(int sig, siginfo_t* siginfo, void* context) @@ -149,6 +104,22 @@ static void SignalHandler(int sig, siginfo_t* siginfo, void* context) } } +void SystemNative_DefaultSignalHandler(int signalCode) +{ + if (signalCode == SIGQUIT || + signalCode == SIGINT || + signalCode == SIGHUP || + signalCode == SIGTERM) + { +#ifdef HAS_CONSOLE_SIGNALS + UninitializeTerminal(); +#endif + // Restore the original signal handler and invoke it. + sigaction(signalCode, OrigActionFor(signalCode), NULL); + kill(getpid(), signalCode); + } +} + // Entrypoint for the thread that handles signals where our handling // isn't signal-safe. Those signal handlers write the signal to a pipe, // which this loop reads and processes. @@ -228,19 +199,16 @@ static void* SignalHandlerLoop(void* arg) #endif } - PosixSignal signal; - if (TryConvertSignalCodeToPosixSignal(signalCode, &signal)) + bool usePosixSignalHandler = g_hasPosixSignalRegistrations[signalCode - 1]; + if (usePosixSignalHandler) { - bool hasManagedHandler = g_signalHasRegistrations[-signal]; - if (hasManagedHandler) - { - assert(g_posixSignalHandler != NULL); - hasManagedHandler = g_posixSignalHandler(signal) != 0; - } - if (!hasManagedHandler) - { - SystemNative_HandlePosixSignal(signal); - } + assert(g_posixSignalHandler != NULL); + usePosixSignalHandler = g_posixSignalHandler(signalCode) != 0; + } + + if (!usePosixSignalHandler) + { + SystemNative_DefaultSignalHandler(signalCode); } } } @@ -274,13 +242,26 @@ void SystemNative_RegisterForSigChld(SigChldCallback callback) pthread_mutex_unlock(&lock); } -static void InstallSignalHandler(int sig, bool skipWhenSigIgn) +static void InstallSignalHandler(int sig) { int rv; (void)rv; // only used for assert struct sigaction* orig = OrigActionFor(sig); + bool* isSet = &g_origSigHandlerIsSet[sig - 1]; - if (skipWhenSigIgn) + if (*isSet) + { + return; + } + *isSet = true; + + // We don't handle ignored signals that terminate the process. If we'd + // setup a handler, our child processes would reset to the default on exec + // causing them to terminate on these signals. + if (sig == SIGTERM || + sig == SIGINT || + sig == SIGQUIT || + sig == SIGHUP) { rv = sigaction(sig, NULL, orig); assert(rv == 0); @@ -365,62 +346,33 @@ int32_t InitializeSignalHandlingCore() return 0; } - // Finally, register our signal handlers - // We don't handle ignored SIGINT/SIGQUIT/SIGHUP/SIGTERM signals. If we'd - // setup a handler, our child processes would reset to the default on exec - // causing them to terminate on these signals. - InstallSignalHandler(SIGINT , /* skipWhenSigIgn */ true); - InstallSignalHandler(SIGQUIT, /* skipWhenSigIgn */ true); - InstallSignalHandler(SIGHUP, /* skipWhenSigIgn */ true); - InstallSignalHandler(SIGTERM, /* skipWhenSigIgn */ true); - - InstallSignalHandler(SIGCONT, /* skipWhenSigIgn */ false); - InstallSignalHandler(SIGCHLD, /* skipWhenSigIgn */ false); - InstallSignalHandler(SIGWINCH, /* skipWhenSigIgn */ false); + // Signals that are handled direcly from SignalHandlerLoop. + InstallSignalHandler(SIGCONT); + InstallSignalHandler(SIGCHLD); + InstallSignalHandler(SIGWINCH); return 1; } -int32_t SystemNative_RegisterForPosixSignal(PosixSignal signal, PosixSignalHandler signalHandler) +void SystemNative_EnablePosixSignalHandling(int signalCode) { - assert(signalHandler != NULL); - assert(g_posixSignalHandler == NULL || g_posixSignalHandler == signalHandler); + assert(g_posixSignalHandler != NULL); + assert(signalCode > 0 && signalCode <= NSIG); - if (signal >= 0 || -signal > PosixSignalCount) + pthread_mutex_lock(&lock); { - errno = EINVAL; - return 0; + InstallSignalHandler(signalCode); } + pthread_mutex_unlock(&lock); - g_posixSignalHandler = signalHandler; - g_signalHasRegistrations[-signal] = true; - - return 1; + g_hasPosixSignalRegistrations[signalCode - 1] = true; } -void SystemNative_UnregisterForPosixSignal(PosixSignal signal) +void SystemNative_DisablePosixSignalHandling(int signalCode) { - assert(signal < 0 && -signal <= PosixSignalCount); - - g_signalHasRegistrations[-signal] = false; -} + assert(signalCode > 0 && signalCode <= NSIG); -void SystemNative_HandlePosixSignal(PosixSignal signal) -{ - if (signal == PosixSignalSIGQUIT || - signal == PosixSignalSIGINT || - signal == PosixSignalSIGHUP || - signal == PosixSignalSIGTERM) - { -#ifdef HAS_CONSOLE_SIGNALS - UninitializeTerminal(); -#endif - // Restore the original signal handler and invoke it. - int signalCode; - TryConvertPosixSignalToSignalCode(signal, &signalCode); - sigaction(signalCode, OrigActionFor(signalCode), NULL); - kill(getpid(), signalCode); - } + g_hasPosixSignalRegistrations[signalCode - 1] = false; } #ifndef HAS_CONSOLE_SIGNALS diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.h b/src/libraries/Native/Unix/System.Native/pal_signal.h index cbf1ba68b6ec8..836c594a85271 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.h +++ b/src/libraries/Native/Unix/System.Native/pal_signal.h @@ -44,16 +44,16 @@ typedef enum PosixSignalSIGINT = -2, PosixSignalSIGQUIT = -3, PosixSignalSIGTERM = -4, - PosixSignalSIGCHLD = -5, - - PosixSignalCount = 5 + PosixSignalSIGCHLD = -5 } PosixSignal; -typedef int32_t (*PosixSignalHandler)(PosixSignal signal); +typedef int32_t (*PosixSignalHandler)(int32_t signal); -PALEXPORT int32_t SystemNative_RegisterForPosixSignal(PosixSignal signal, PosixSignalHandler signalHandler); -PALEXPORT void SystemNative_UnregisterForPosixSignal(PosixSignal signal); -PALEXPORT void SystemNative_HandlePosixSignal(PosixSignal signal); +PALEXPORT void SystemNative_SetPosixSignalHandler(PosixSignalHandler signalHandler); +PALEXPORT int SystemNative_GetPlatformSignalNumber(PosixSignal signal); +PALEXPORT void SystemNative_EnablePosixSignalHandling(int signalCode); +PALEXPORT void SystemNative_DisablePosixSignalHandling(int signalCode); +PALEXPORT void SystemNative_DefaultSignalHandler(int signalCode); #ifndef HAS_CONSOLE_SIGNALS diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalContext.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalContext.cs index d6cb5a1bf6843..080fa49b4b227 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalContext.cs +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalContext.cs @@ -8,17 +8,21 @@ public sealed class PosixSignalContext public PosixSignal Signal { get; + internal set; } public bool Cancel { get; - set; // TODO: should this throw if Canceling doesn't do anything? + set; } public PosixSignalContext(PosixSignal signal) { Signal = signal; } + + internal PosixSignalContext() + { } } } diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs index 1a305ab7c8986..1fc3836453238 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs @@ -12,12 +12,14 @@ public sealed class PosixSignalRegistration : IDisposable { 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, Action handler) + private PosixSignalRegistration(PosixSignal signal, int signo, Action handler) { _signal = signal; + _signo = signo; _handler = handler; } @@ -27,16 +29,17 @@ public static PosixSignalRegistration Create(PosixSignal signal, Action PosixSignal.SIGHUP || signal < PosixSignal.SIGCHLD) + int signo = Interop.Sys.GetPlatformSignalNumber(signal); + if (signo == 0) { - throw new IndexOutOfRangeException(); + throw new ArgumentOutOfRangeException(nameof(signal)); } - PosixSignalRegistration registration = new PosixSignalRegistration(signal, handler); - registration.RegisterFor(signal); + PosixSignalRegistration registration = new PosixSignalRegistration(signal, signo, handler); + registration.Register(); return registration; } - private unsafe void RegisterFor(PosixSignal signal) + private unsafe void Register() { if (!s_initialized) { @@ -44,18 +47,19 @@ private unsafe void RegisterFor(PosixSignal signal) { throw new Win32Exception(); } + Interop.Sys.SetPosixSignalHandler(&OnPosixSignal); s_initialized = true; } lock (s_registrations) { - if (!s_registrations.TryGetValue(signal, out List>? signalRegistrations)) + if (!s_registrations.TryGetValue(_signo, out List>? signalRegistrations)) { - if (!Interop.Sys.RegisterForPosixSignal(signal, &OnPosixSignal)) - { - throw new Win32Exception(); - } signalRegistrations = new List>(); - s_registrations.Add(signal, signalRegistrations); + s_registrations.Add(_signo, signalRegistrations); + } + if (signalRegistrations.Count == 0) + { + Interop.Sys.EnablePosixSignalHandling(_signo); } signalRegistrations.Add(new WeakReference(this)); } @@ -74,11 +78,11 @@ private void Handle(PosixSignalContext context) } [UnmanagedCallersOnly] - private static int OnPosixSignal(PosixSignal signal) + private static int OnPosixSignal(int signo) { lock (s_registrations) { - if (s_registrations.TryGetValue(signal, out List>? signalRegistrations)) + if (s_registrations.TryGetValue(signo, out List>? signalRegistrations)) { if (signalRegistrations.Count != 0) { @@ -104,7 +108,7 @@ private static int OnPosixSignal(PosixSignal signal) IsBackground = true, Name = ".NET Signal Handler" }; - handlerThread.Start((signal, registrations)); + handlerThread.Start((signo, registrations)); return 1; } } @@ -115,17 +119,23 @@ private static int OnPosixSignal(PosixSignal signal) private static void HandleSignal(object? state) { - var (signal, registrations) = ((PosixSignal, PosixSignalRegistration?[]))state!; + var (signo, registrations) = ((int, PosixSignalRegistration?[]))state!; - PosixSignalContext ctx = new(signal); - foreach (var registration in registrations) + PosixSignalContext ctx = new(); + foreach (PosixSignalRegistration? registration in registrations) { - registration?.Handle(ctx); + if (registration != null) + { + // Different values for PosixSignal map to the same signo. + // Match the PosixSignal value used when registering. + ctx.Signal = registration._signal; + registration.Handle(ctx); + } } if (!ctx.Cancel) { - Interop.Sys.HandlePosixSignal(signal); + Interop.Sys.DefaultSignalHandler(signo); } } @@ -144,7 +154,7 @@ private void Dispose(bool disposing) { lock (s_registrations) { - List> signalRegistrations = s_registrations[_signal]; + List> signalRegistrations = s_registrations[_signo]; for (int i = 0; i < signalRegistrations.Count; i++) { if (signalRegistrations[i].TryGetTarget(out PosixSignalRegistration? registration) && @@ -156,7 +166,7 @@ private void Dispose(bool disposing) } if (signalRegistrations.Count == 0) { - Interop.Sys.UnregisterForPosixSignal(_signal); + Interop.Sys.DisablePosixSignalHandling(_signo); } } @@ -168,6 +178,6 @@ private void Dispose(bool disposing) } private static volatile bool s_initialized; - private static Dictionary>> s_registrations = new(); + private static readonly Dictionary>> s_registrations = new(); } } From 17045d8bce27a5e329dcf70dc4c04f34098e8e03 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 14 Jun 2021 21:04:52 +0200 Subject: [PATCH 04/65] Reduce nr of TargetFrameworks --- .../src/System.Runtime.InteropServices.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Runtime.InteropServices/src/System.Runtime.InteropServices.csproj b/src/libraries/System.Runtime.InteropServices/src/System.Runtime.InteropServices.csproj index a03881f0a30b8..0f4e84a5bd258 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System.Runtime.InteropServices.csproj +++ b/src/libraries/System.Runtime.InteropServices/src/System.Runtime.InteropServices.csproj @@ -2,7 +2,7 @@ true enable - $(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Android;$(NetCoreAppCurrent)-iOS;$(NetCoreAppCurrent)-tvOS;$(NetCoreAppCurrent)-Browser + $(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser From 61a0fba8b02aab5717371e313cb797d33630b5ae Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 14 Jun 2021 21:36:01 +0200 Subject: [PATCH 05/65] #ifdef SIGRTMAX --- src/libraries/Native/Unix/System.Native/pal_signal.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index 1e186cc6eab08..96beaa78636b2 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -54,9 +54,12 @@ int SystemNative_GetPlatformSignalNumber(PosixSignal signal) return SIGCHLD; } - if (signal > 0 && - signal <= NSIG && // Ensure we stay within the static arrays. - signal <= SIGRTMAX) // Runtime check for highest value. + if ( signal > 0 + && signal <= NSIG // Ensure we stay within the static arrays. +#ifdef SIGRTMAX + && signal <= SIGRTMAX // Runtime check for highest value. +#endif + ) { return signal; } From b5ae07abfffb2d8fb1c3411deb78dc68e2595422 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 14 Jun 2021 21:36:06 +0200 Subject: [PATCH 06/65] Don't throw Win32Exception (for now) --- .../src/System.Runtime.InteropServices.csproj | 4 ---- .../Runtime/InteropServices/PosixSignalRegistration.Unix.cs | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/src/System.Runtime.InteropServices.csproj b/src/libraries/System.Runtime.InteropServices/src/System.Runtime.InteropServices.csproj index 0f4e84a5bd258..4846d5a55e276 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System.Runtime.InteropServices.csproj +++ b/src/libraries/System.Runtime.InteropServices/src/System.Runtime.InteropServices.csproj @@ -65,8 +65,4 @@ - - - - \ No newline at end of file diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs index 1fc3836453238..fd0da4d94be53 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs @@ -45,7 +45,7 @@ private unsafe void Register() { if (!Interop.Sys.InitializeTerminalAndSignalHandling()) { - throw new Win32Exception(); + throw new Exception(); // TODO: can this throw Win32Exception? } Interop.Sys.SetPosixSignalHandler(&OnPosixSignal); s_initialized = true; From 52e1f1751f18da6211a786b446e0b52615cdbca1 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 16 Jun 2021 10:47:02 +0200 Subject: [PATCH 07/65] Rename PosixSignalRegistration.cs to PosixSignalRegistration.Unsupported.cs --- .../src/System.Runtime.InteropServices.csproj | 2 +- ...alRegistration.cs => PosixSignalRegistration.Unsupported.cs} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/{PosixSignalRegistration.cs => PosixSignalRegistration.Unsupported.cs} (100%) diff --git a/src/libraries/System.Runtime.InteropServices/src/System.Runtime.InteropServices.csproj b/src/libraries/System.Runtime.InteropServices/src/System.Runtime.InteropServices.csproj index 4846d5a55e276..3f618438e16c4 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System.Runtime.InteropServices.csproj +++ b/src/libraries/System.Runtime.InteropServices/src/System.Runtime.InteropServices.csproj @@ -60,7 +60,7 @@ Link="Common\Interop\Unix\Interop.Libraries.cs" /> - + diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unsupported.cs similarity index 100% rename from src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.cs rename to src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unsupported.cs From 26f22b35f72c9412c06fdb51a39514b750f5a5ac Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 16 Jun 2021 11:53:41 +0200 Subject: [PATCH 08/65] Spawn new Thread only for specific signals --- .../Unix/System.Native/Interop.PosixSignal.cs | 2 +- .../Native/Unix/System.Native/pal_signal.c | 42 +++++++++++++++++-- .../Native/Unix/System.Native/pal_signal.h | 4 +- .../PosixSignalRegistration.Unix.cs | 34 ++++++++++----- 4 files changed, 66 insertions(+), 16 deletions(-) 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 4c42728b653a2..7ab0407d27745 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 @@ -9,7 +9,7 @@ internal static partial class Sys { [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_SetPosixSignalHandler")] [SuppressGCTransition] - internal static extern unsafe void SetPosixSignalHandler(delegate* unmanaged handler); + internal static extern unsafe void SetPosixSignalHandler(delegate* unmanaged handler); [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_EnablePosixSignalHandling")] [SuppressGCTransition] diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index 96beaa78636b2..f4660b4099cdc 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -22,7 +22,6 @@ static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER; static struct sigaction g_origSigHandler[NSIG]; static bool g_origSigHandlerIsSet[NSIG]; - // Callback invoked for SIGCHLD/SIGCONT/SIGWINCH static volatile TerminalInvalidationCallback g_terminalInvalidationCallback = NULL; // Callback invoked for SIGCHLD @@ -34,7 +33,39 @@ static volatile bool g_hasPosixSignalRegistrations[NSIG]; static int g_signalPipe[2] = {-1, -1}; // Pipe used between signal handler and worker -int SystemNative_GetPlatformSignalNumber(PosixSignal signal) +static bool TryConvertSignalCodeToPosixSignal(int signalCode, PosixSignal* posixSignal) +{ + assert(posixSignal != NULL); + + switch (signalCode) + { + case SIGHUP: + *posixSignal = PosixSignalSIGHUP; + return true; + + case SIGINT: + *posixSignal = PosixSignalSIGINT; + return true; + + case SIGQUIT: + *posixSignal = PosixSignalSIGQUIT; + return true; + + case SIGTERM: + *posixSignal = PosixSignalSIGTERM; + return true; + + case SIGCHLD: + *posixSignal = PosixSignalSIGCHLD; + return true; + + default: + *posixSignal = signalCode; + return false; + } +} + +int32_t SystemNative_GetPlatformSignalNumber(PosixSignal signal) { switch (signal) { @@ -206,7 +237,12 @@ static void* SignalHandlerLoop(void* arg) if (usePosixSignalHandler) { assert(g_posixSignalHandler != NULL); - usePosixSignalHandler = g_posixSignalHandler(signalCode) != 0; + PosixSignal signal; + if (!TryConvertSignalCodeToPosixSignal(signalCode, &signal)) + { + signal = (PosixSignal)0; + } + usePosixSignalHandler = g_posixSignalHandler(signalCode, signal) != 0; } if (!usePosixSignalHandler) diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.h b/src/libraries/Native/Unix/System.Native/pal_signal.h index 836c594a85271..51f6c295afb70 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.h +++ b/src/libraries/Native/Unix/System.Native/pal_signal.h @@ -47,10 +47,10 @@ typedef enum PosixSignalSIGCHLD = -5 } PosixSignal; -typedef int32_t (*PosixSignalHandler)(int32_t signal); +typedef int32_t (*PosixSignalHandler)(int32_t signalCode, PosixSignal signal); PALEXPORT void SystemNative_SetPosixSignalHandler(PosixSignalHandler signalHandler); -PALEXPORT int SystemNative_GetPlatformSignalNumber(PosixSignal signal); +PALEXPORT int32_t SystemNative_GetPlatformSignalNumber(PosixSignal signal); PALEXPORT void SystemNative_EnablePosixSignalHandling(int signalCode); PALEXPORT void SystemNative_DisablePosixSignalHandling(int signalCode); PALEXPORT void SystemNative_DefaultSignalHandler(int signalCode); diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs index fd0da4d94be53..37a294fd73923 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs @@ -78,7 +78,7 @@ private void Handle(PosixSignalContext context) } [UnmanagedCallersOnly] - private static int OnPosixSignal(int signo) + private static int OnPosixSignal(int signo, PosixSignal signal) { lock (s_registrations) { @@ -101,14 +101,25 @@ private static int OnPosixSignal(int signo) // 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. - // We use a new thread rather than queueing to the ThreadPool in order to prioritize handling + + // For terminate/interrupt signals we use a dedicated Thread // in case the ThreadPool is saturated. - Thread handlerThread = new Thread(HandleSignal) + bool useDedicatedThread = signal == PosixSignal.SIGINT || + signal == PosixSignal.SIGQUIT || + signal == PosixSignal.SIGTERM; + if (useDedicatedThread) + { + Thread handlerThread = new Thread(HandleSignal) + { + IsBackground = true, + Name = ".NET Signal Handler" + }; + handlerThread.Start((signo, registrations)); + } + else { - IsBackground = true, - Name = ".NET Signal Handler" - }; - handlerThread.Start((signo, registrations)); + ThreadPool.UnsafeQueueUserWorkItem(HandleSignal, (signo, registrations)); + } return 1; } } @@ -119,10 +130,13 @@ private static int OnPosixSignal(int signo) private static void HandleSignal(object? state) { - var (signo, registrations) = ((int, PosixSignalRegistration?[]))state!; + HandleSignal(((int, PosixSignalRegistration?[]))state!); + } + private static void HandleSignal((int signo, PosixSignalRegistration?[] registrations) state) + { PosixSignalContext ctx = new(); - foreach (PosixSignalRegistration? registration in registrations) + foreach (PosixSignalRegistration? registration in state.registrations) { if (registration != null) { @@ -135,7 +149,7 @@ private static void HandleSignal(object? state) if (!ctx.Cancel) { - Interop.Sys.DefaultSignalHandler(signo); + Interop.Sys.DefaultSignalHandler(state.signo); } } From 0bd03f95c92ac63bc8d57a19a1c0157fe993ad5d Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 17 Jun 2021 09:19:50 +0200 Subject: [PATCH 09/65] Add PosixSignal.SIGCONT, SIGWINCH, SIGTTIN, SIGTTOU, SIGTSTP --- .../Native/Unix/System.Native/pal_console.c | 19 +- .../Native/Unix/System.Native/pal_signal.c | 206 +++++++++++++----- .../Native/Unix/System.Native/pal_signal.h | 20 +- .../ref/System.Runtime.InteropServices.cs | 7 +- .../Runtime/InteropServices/PosixSignal.cs | 7 +- 5 files changed, 182 insertions(+), 77 deletions(-) diff --git a/src/libraries/Native/Unix/System.Native/pal_console.c b/src/libraries/Native/Unix/System.Native/pal_console.c index c18e3f82620f6..54a54e3c0e3a6 100644 --- a/src/libraries/Native/Unix/System.Native/pal_console.c +++ b/src/libraries/Native/Unix/System.Native/pal_console.c @@ -97,23 +97,11 @@ static bool g_hasTty = false; // cache we are not a tty static volatile bool g_receivedSigTtou = false; -static void ttou_handler(int signo) +static void ttou_handler() { - (void)signo; g_receivedSigTtou = true; } -static void InstallTTOUHandler(void (*handler)(int), int flags) -{ - struct sigaction action; - memset(&action, 0, sizeof(action)); - action.sa_handler = handler; - action.sa_flags = flags; - int rvSigaction = sigaction(SIGTTOU, &action, NULL); - assert(rvSigaction == 0); - (void)rvSigaction; -} - static bool TcSetAttr(struct termios* termios, bool blockIfBackground) { if (g_terminalUninitialized) @@ -131,7 +119,7 @@ static bool TcSetAttr(struct termios* termios, bool blockIfBackground) // stdout. We set SA_RESETHAND to avoid that handler's write loops infinitly // on EINTR when the process is running in background and the terminal // configured with TOSTOP. - InstallTTOUHandler(ttou_handler, (int)SA_RESETHAND); + InstallTTOUHandlerForConsole(ttou_handler); g_receivedSigTtou = false; } @@ -147,8 +135,7 @@ static bool TcSetAttr(struct termios* termios, bool blockIfBackground) rv = true; } - // Restore default SIGTTOU handler. - InstallTTOUHandler(SIG_DFL, 0); + UninstallTTOUHandlerForConsole(); } // On success, update the cached value. diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index f4660b4099cdc..5d74f4bb07682 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -20,12 +20,15 @@ static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER; // Saved signal handlers static struct sigaction g_origSigHandler[NSIG]; -static bool g_origSigHandlerIsSet[NSIG]; +static bool g_handlerIsInstalled[NSIG]; // Callback invoked for SIGCHLD/SIGCONT/SIGWINCH static volatile TerminalInvalidationCallback g_terminalInvalidationCallback = NULL; // Callback invoked for SIGCHLD static volatile SigChldCallback g_sigChldCallback = NULL; +// Callback invoked for for SIGTTOU while terminal settings are changed. +static volatile ConsoleSigTtouHandler g_consoleTtouHandler; + // Callback invoked for PosixSignal handling. static PosixSignalHandler g_posixSignalHandler = NULL; // Tracks whether there are PosixSignal handlers registered. @@ -59,6 +62,26 @@ static bool TryConvertSignalCodeToPosixSignal(int signalCode, PosixSignal* posix *posixSignal = PosixSignalSIGCHLD; return true; + case SIGWINCH: + *posixSignal = PosixSignalSIGWINCH; + return true; + + case SIGCONT: + *posixSignal = PosixSignalSIGCONT; + return true; + + case SIGTTIN: + *posixSignal = PosixSignalSIGTTIN; + return true; + + case SIGTTOU: + *posixSignal = PosixSignalSIGTTOU; + return true; + + case SIGTSTP: + *posixSignal = PosixSignalSIGTSTP; + return true; + default: *posixSignal = signalCode; return false; @@ -83,6 +106,21 @@ int32_t SystemNative_GetPlatformSignalNumber(PosixSignal signal) case PosixSignalSIGCHLD: return SIGCHLD; + + case PosixSignalSIGWINCH: + return SIGWINCH; + + case PosixSignalSIGCONT: + return SIGCONT; + + case PosixSignalSIGTTIN: + return SIGTTIN; + + case PosixSignalSIGTTOU: + return SIGTTOU; + + case PosixSignalSIGTSTP: + return SIGTSTP; } if ( signal > 0 @@ -111,6 +149,13 @@ static struct sigaction* OrigActionFor(int sig) return &g_origSigHandler[sig - 1]; } +static void RestoreSignalHandler(int sig) +{ + bool* isInstalled = &g_handlerIsInstalled[sig - 1]; + *isInstalled = false; + sigaction(sig, OrigActionFor(sig), NULL); +} + static void SignalHandler(int sig, siginfo_t* siginfo, void* context) { // Signal handler for signals where we want our background thread to do the real processing. @@ -124,6 +169,15 @@ static void SignalHandler(int sig, siginfo_t* siginfo, void* context) abort(); // fatal error } + if (sig == SIGCONT) + { + ConsoleSigTtouHandler consoleTtouHandler = g_consoleTtouHandler; + if (consoleTtouHandler != NULL) + { + consoleTtouHandler(); + } + } + // Delegate to any saved handler we may have // We assume the original SIGCHLD handler will not reap our children. if (sig == SIGCONT || sig == SIGCHLD || sig == SIGWINCH) @@ -140,16 +194,25 @@ static void SignalHandler(int sig, siginfo_t* siginfo, void* context) void SystemNative_DefaultSignalHandler(int signalCode) { - if (signalCode == SIGQUIT || - signalCode == SIGINT || - signalCode == SIGHUP || - signalCode == SIGTERM) + // Perform the default runtime action for the signal. + + if (signalCode == SIGCHLD || + signalCode == SIGCONT || + signalCode == SIGWINCH) + { + // The default disposition does nothing. + // If there is a non-default disposition, it was called from SignalHandler. + } + // Terminate the process using the original handler. + else if (signalCode == SIGQUIT || + signalCode == SIGTERM || + signalCode == SIGINT) { #ifdef HAS_CONSOLE_SIGNALS UninitializeTerminal(); #endif // Restore the original signal handler and invoke it. - sigaction(signalCode, OrigActionFor(signalCode), NULL); + RestoreSignalHandler(signalCode); kill(getpid(), signalCode); } } @@ -262,52 +325,29 @@ static void CloseSignalHandlingPipe() g_signalPipe[1] = -1; } -void SystemNative_SetTerminalInvalidationHandler(TerminalInvalidationCallback callback) -{ - assert(callback != NULL); - assert(g_terminalInvalidationCallback == NULL); - g_terminalInvalidationCallback = callback; -} - -void SystemNative_RegisterForSigChld(SigChldCallback callback) -{ - assert(callback != NULL); - assert(g_sigChldCallback == NULL); - - pthread_mutex_lock(&lock); - { - g_sigChldCallback = callback; - } - pthread_mutex_unlock(&lock); -} - static void InstallSignalHandler(int sig) { int rv; (void)rv; // only used for assert struct sigaction* orig = OrigActionFor(sig); - bool* isSet = &g_origSigHandlerIsSet[sig - 1]; + bool* isInstalled = &g_handlerIsInstalled[sig - 1]; - if (*isSet) + if (*isInstalled) { + // Already installed. return; } - *isSet = true; - - // We don't handle ignored signals that terminate the process. If we'd - // setup a handler, our child processes would reset to the default on exec - // causing them to terminate on these signals. - if (sig == SIGTERM || - sig == SIGINT || - sig == SIGQUIT || - sig == SIGHUP) + + // We respect ignored signals. + // Setting up a handler for them causes child processes to reset to the + // default handler on exec, which means they will terminate on some signals + // which were set to ignore. + rv = sigaction(sig, NULL, orig); + assert(rv == 0); + if ((void*)orig->sa_sigaction == (void*)SIG_IGN) { - rv = sigaction(sig, NULL, orig); - assert(rv == 0); - if ((void*)orig->sa_sigaction == (void*)SIG_IGN) - { - return; - } + *isInstalled = true; + return; } struct sigaction newAction; @@ -318,6 +358,37 @@ static void InstallSignalHandler(int sig) rv = sigaction(sig, &newAction, orig); assert(rv == 0); + *isInstalled = true; +} + +void SystemNative_SetTerminalInvalidationHandler(TerminalInvalidationCallback callback) +{ + assert(callback != NULL); + assert(g_terminalInvalidationCallback == NULL); + + pthread_mutex_lock(&lock); + { + g_terminalInvalidationCallback = callback; + + InstallSignalHandler(SIGCONT); + InstallSignalHandler(SIGCHLD); + InstallSignalHandler(SIGWINCH); + } + pthread_mutex_unlock(&lock); +} + +void SystemNative_RegisterForSigChld(SigChldCallback callback) +{ + assert(callback != NULL); + assert(g_sigChldCallback == NULL); + + pthread_mutex_lock(&lock); + { + g_sigChldCallback = callback; + + InstallSignalHandler(SIGCHLD); + } + pthread_mutex_unlock(&lock); } static bool CreateSignalHandlerThread(int* readFdPtr) @@ -385,11 +456,6 @@ int32_t InitializeSignalHandlingCore() return 0; } - // Signals that are handled direcly from SignalHandlerLoop. - InstallSignalHandler(SIGCONT); - InstallSignalHandler(SIGCHLD); - InstallSignalHandler(SIGWINCH); - return 1; } @@ -401,17 +467,57 @@ void SystemNative_EnablePosixSignalHandling(int signalCode) pthread_mutex_lock(&lock); { InstallSignalHandler(signalCode); + + g_hasPosixSignalRegistrations[signalCode - 1] = true; } pthread_mutex_unlock(&lock); - - g_hasPosixSignalRegistrations[signalCode - 1] = true; } void SystemNative_DisablePosixSignalHandling(int signalCode) { assert(signalCode > 0 && signalCode <= NSIG); - g_hasPosixSignalRegistrations[signalCode - 1] = false; + pthread_mutex_lock(&lock); + { + g_hasPosixSignalRegistrations[signalCode - 1] = false; + + if (!(g_consoleTtouHandler && signalCode == SIGTTOU) && + !(g_sigChldCallback && signalCode == SIGCHLD) && + !(g_terminalInvalidationCallback && (signalCode == SIGCONT || + signalCode == SIGCHLD || + signalCode == SIGWINCH))) + { + RestoreSignalHandler(signalCode); + } + } + pthread_mutex_unlock(&lock); +} + +void InstallTTOUHandlerForConsole(ConsoleSigTtouHandler handler) +{ + pthread_mutex_lock(&lock); + { + assert(g_consoleTtouHandler == NULL); + g_consoleTtouHandler = handler; + + InstallSignalHandler(SIGTTOU); + } + pthread_mutex_unlock(&lock); +} + +void UninstallTTOUHandlerForConsole(void) +{ + pthread_mutex_lock(&lock); + { + g_consoleTtouHandler = NULL; + + // Don't restore SIGTTOU while there are PosixSignal registrations. + if (!g_hasPosixSignalRegistrations[SIGTTOU - 1]) + { + RestoreSignalHandler(SIGTTOU); + } + } + pthread_mutex_unlock(&lock); } #ifndef HAS_CONSOLE_SIGNALS diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.h b/src/libraries/Native/Unix/System.Native/pal_signal.h index 51f6c295afb70..572f6b0326d49 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.h +++ b/src/libraries/Native/Unix/System.Native/pal_signal.h @@ -22,14 +22,6 @@ typedef void (*SigChldCallback)(int reapAll); */ PALEXPORT void SystemNative_RegisterForSigChld(SigChldCallback callback); -/** - * Remove our handler and reissue the signal to be picked up by the previously registered handler. - * - * In the most common case, this will be the default handler, causing the process to be torn down. - * It could also be a custom handler registered by other code before us. - */ -PALEXPORT void SystemNative_RestoreAndHandleCtrl(CtrlCode ctrlCode); - typedef void (*TerminalInvalidationCallback)(void); /** @@ -44,7 +36,12 @@ typedef enum PosixSignalSIGINT = -2, PosixSignalSIGQUIT = -3, PosixSignalSIGTERM = -4, - PosixSignalSIGCHLD = -5 + PosixSignalSIGCHLD = -5, + PosixSignalSIGWINCH = -6, + PosixSignalSIGCONT = -7, + PosixSignalSIGTTIN = -8, + PosixSignalSIGTTOU = -9, + PosixSignalSIGTSTP = -10 } PosixSignal; typedef int32_t (*PosixSignalHandler)(int32_t signalCode, PosixSignal signal); @@ -55,6 +52,11 @@ PALEXPORT void SystemNative_EnablePosixSignalHandling(int signalCode); PALEXPORT void SystemNative_DisablePosixSignalHandling(int signalCode); PALEXPORT void SystemNative_DefaultSignalHandler(int signalCode); +typedef void (*ConsoleSigTtouHandler)(void); + +void InstallTTOUHandlerForConsole(ConsoleSigTtouHandler handler); +void UninstallTTOUHandlerForConsole(void); + #ifndef HAS_CONSOLE_SIGNALS /** diff --git a/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs b/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs index b6cd1e702cc27..adf7158df07d7 100644 --- a/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs +++ b/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs @@ -802,7 +802,12 @@ public enum PosixSignal SIGINT = -2, SIGQUIT = -3, SIGTERM = -4, - SIGCHLD = -5 + SIGCHLD = -5, + SIGCONT = -6, + SIGWINCH = -7, + SIGTTIN = -8, + SIGTTOU = -9, + SIGTSTP = -10 } public sealed class PosixSignalContext { diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignal.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignal.cs index 7a5a74488299b..f1cb32a362b8d 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignal.cs +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignal.cs @@ -9,6 +9,11 @@ public enum PosixSignal SIGINT = -2, SIGQUIT = -3, SIGTERM = -4, - SIGCHLD = -5 + SIGCHLD = -5, + SIGCONT = -6, + SIGWINCH = -7, + SIGTTIN = -8, + SIGTTOU = -9, + SIGTSTP = -10 } } From e8d01c84cc14acff375a62b02b01ca919d59c48a Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 17 Jun 2021 09:35:09 +0200 Subject: [PATCH 10/65] SystemNative_DefaultSignalHandler: remove SuppressGCTransition attribute --- .../Common/src/Interop/Unix/System.Native/Interop.PosixSignal.cs | 1 - 1 file changed, 1 deletion(-) 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 7ab0407d27745..085f0b337fff0 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 @@ -20,7 +20,6 @@ internal static partial class Sys internal static extern void DisablePosixSignalHandling(int signal); [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_DefaultSignalHandler")] - [SuppressGCTransition] internal static extern void DefaultSignalHandler(int signal); [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetPlatformSignalNumber")] From 6cc2d8920c649658971c2e5961075790de3a9cdd Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 17 Jun 2021 09:37:48 +0200 Subject: [PATCH 11/65] Update comment in SystemNative_InitializeTerminalAndSignalHandling --- src/libraries/Native/Unix/System.Native/pal_console.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Native/Unix/System.Native/pal_console.c b/src/libraries/Native/Unix/System.Native/pal_console.c index 54a54e3c0e3a6..9a69cc3881ffb 100644 --- a/src/libraries/Native/Unix/System.Native/pal_console.c +++ b/src/libraries/Native/Unix/System.Native/pal_console.c @@ -460,7 +460,7 @@ int32_t SystemNative_InitializeTerminalAndSignalHandling() { static int32_t initialized = 0; - // Both the Process and Console class call this method for initialization. + // The Process, Console and PosixSignalRegistration classes call this method for initialization. if (pthread_mutex_lock(&g_lock) == 0) { if (initialized == 0) From 5892f0d5fa6a8034fab8ef3a7e37397ca607fe84 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 17 Jun 2021 13:38:51 +0200 Subject: [PATCH 12/65] Run original handler except for cancelable signals. --- .../Native/Unix/System.Native/pal_signal.c | 55 +++++++++---------- 1 file changed, 27 insertions(+), 28 deletions(-) diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index 5d74f4bb07682..80c5893439973 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -158,17 +158,6 @@ static void RestoreSignalHandler(int sig) static void SignalHandler(int sig, siginfo_t* siginfo, void* context) { - // Signal handler for signals where we want our background thread to do the real processing. - // It simply writes the signal code to a pipe that's read by the thread. - uint8_t signalCodeByte = (uint8_t)sig; - ssize_t writtenBytes; - while ((writtenBytes = write(g_signalPipe[1], &signalCodeByte, 1)) < 0 && errno == EINTR); - - if (writtenBytes != 1) - { - abort(); // fatal error - } - if (sig == SIGCONT) { ConsoleSigTtouHandler consoleTtouHandler = g_consoleTtouHandler; @@ -178,17 +167,34 @@ static void SignalHandler(int sig, siginfo_t* siginfo, void* context) } } - // Delegate to any saved handler we may have - // We assume the original SIGCHLD handler will not reap our children. - if (sig == SIGCONT || sig == SIGCHLD || sig == SIGWINCH) + // Signals that can be canceled through the PosixSignal API are handled later. + // For other signals, we immediately invoke the original handler. + if (sig != SIGQUIT && + sig != SIGTERM && + sig != SIGINT) { struct sigaction* origHandler = OrigActionFor(sig); - if (origHandler->sa_sigaction != NULL && - (void*)origHandler->sa_sigaction != (void*)SIG_DFL && - (void*)origHandler->sa_sigaction != (void*)SIG_IGN) + if (origHandler->sa_flags & SA_SIGINFO) { + assert(origHandler->sa_sigaction); origHandler->sa_sigaction(sig, siginfo, context); } + else if (origHandler->sa_handler != SIG_IGN && + origHandler->sa_handler != SIG_DFL) + { + origHandler->sa_handler(sig); + } + } + + // Perform further processing on background thread. + // Write the signal code to a pipe that's read by the thread. + uint8_t signalCodeByte = (uint8_t)sig; + ssize_t writtenBytes; + while ((writtenBytes = write(g_signalPipe[1], &signalCodeByte, 1)) < 0 && errno == EINTR); + + if (writtenBytes != 1) + { + abort(); // fatal error } } @@ -196,18 +202,11 @@ void SystemNative_DefaultSignalHandler(int signalCode) { // Perform the default runtime action for the signal. - if (signalCode == SIGCHLD || - signalCode == SIGCONT || - signalCode == SIGWINCH) - { - // The default disposition does nothing. - // If there is a non-default disposition, it was called from SignalHandler. - } - // Terminate the process using the original handler. - else if (signalCode == SIGQUIT || - signalCode == SIGTERM || - signalCode == SIGINT) + if (signalCode == SIGQUIT || + signalCode == SIGTERM || + signalCode == SIGINT) { + // Terminate the process using the original handler. #ifdef HAS_CONSOLE_SIGNALS UninitializeTerminal(); #endif From f33767716454f7b7597d367a420f71ea6d1575bf Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 17 Jun 2021 13:53:43 +0200 Subject: [PATCH 13/65] Cleanup ControlCHandlerRegistrar --- .../System.Console/src/System/ConsolePal.Unix.cs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Console/src/System/ConsolePal.Unix.cs b/src/libraries/System.Console/src/System/ConsolePal.Unix.cs index 49d5b108392d6..c438413c801cd 100644 --- a/src/libraries/System.Console/src/System/ConsolePal.Unix.cs +++ b/src/libraries/System.Console/src/System/ConsolePal.Unix.cs @@ -1444,23 +1444,21 @@ public override void Flush() internal sealed class ControlCHandlerRegistrar { - private bool _handlerRegistered; - private IDisposable? _sigIntRegistration; - private IDisposable? _sigQuitRegistration; + private PosixSignalRegistration? _sigIntRegistration; + private PosixSignalRegistration? _sigQuitRegistration; internal unsafe void Register() { - Debug.Assert(s_initialized); // by CancelKeyPress add. + Debug.Assert(_sigIntRegistration is null); - Debug.Assert(!_handlerRegistered); _sigIntRegistration = PosixSignalRegistration.Create(PosixSignal.SIGINT, HandlePosixSignal); _sigQuitRegistration = PosixSignalRegistration.Create(PosixSignal.SIGQUIT, HandlePosixSignal); - _handlerRegistered = true; } internal void Unregister() { - Debug.Assert(_handlerRegistered); + Debug.Assert(_sigIntRegistration is not null); + _sigIntRegistration?.Dispose(); _sigQuitRegistration?.Dispose(); } From c3dce04a1b5b5e010fb8bec9bc136b87bc099ace Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 17 Jun 2021 14:13:51 +0200 Subject: [PATCH 14/65] Handle TryGetTarget returning false due to finalized PosixSignalRegistrations --- .../PosixSignalRegistration.Unix.cs | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs index 37a294fd73923..3edd2a24ffa67 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs @@ -95,6 +95,12 @@ private static int OnPosixSignal(int signo, PosixSignal signal) registrations[i] = registration; hasRegistrations = true; } + else + { + // WeakReference no longer holds an object. PosixSignalRegistration got finalized. + signalRegistrations.RemoveAt(i); + i--; + } } if (hasRegistrations) { @@ -171,11 +177,19 @@ private void Dispose(bool disposing) List> signalRegistrations = s_registrations[_signo]; for (int i = 0; i < signalRegistrations.Count; i++) { - if (signalRegistrations[i].TryGetTarget(out PosixSignalRegistration? registration) && - object.ReferenceEquals(this, registration)) + if (signalRegistrations[i].TryGetTarget(out PosixSignalRegistration? registration)) + { + if (object.ReferenceEquals(this, registration)) + { + signalRegistrations.RemoveAt(i); + break; + } + } + else { + // WeakReference no longer holds an object. PosixSignalRegistration got finalized. signalRegistrations.RemoveAt(i); - break; + i--; } } if (signalRegistrations.Count == 0) @@ -184,6 +198,7 @@ private void Dispose(bool disposing) } } + // Synchronize with _handler invocations. lock (_gate) { _registered = false; From db75bbac14b22d9ff9e3050603a1f22edbe0af80 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 17 Jun 2021 14:58:23 +0200 Subject: [PATCH 15/65] Use Thread.UnsafeStart --- .../Runtime/InteropServices/PosixSignalRegistration.Unix.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs index 3edd2a24ffa67..3b6cd9eff3caf 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs @@ -120,7 +120,7 @@ private static int OnPosixSignal(int signo, PosixSignal signal) IsBackground = true, Name = ".NET Signal Handler" }; - handlerThread.Start((signo, registrations)); + handlerThread.UnsafeStart((signo, registrations)); } else { From 96ded5927ba21e2deb65b3a55afc01d8eb1756e1 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 17 Jun 2021 16:30:48 +0200 Subject: [PATCH 16/65] Remove SuppressGCTransition from methods that take a lock --- .../src/Interop/Unix/System.Native/Interop.PosixSignal.cs | 2 -- .../System.Native/Interop.SetTerminalInvalidationHandler.cs | 1 - 2 files changed, 3 deletions(-) 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 085f0b337fff0..5a0e903fbdd6c 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 @@ -12,11 +12,9 @@ internal static partial class Sys internal static extern unsafe void SetPosixSignalHandler(delegate* unmanaged handler); [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_EnablePosixSignalHandling")] - [SuppressGCTransition] internal static extern void EnablePosixSignalHandling(int signal); [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_DisablePosixSignalHandling")] - [SuppressGCTransition] internal static extern void DisablePosixSignalHandling(int signal); [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_DefaultSignalHandler")] diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.SetTerminalInvalidationHandler.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.SetTerminalInvalidationHandler.cs index a12194e173755..3082cb96b7183 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.SetTerminalInvalidationHandler.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.SetTerminalInvalidationHandler.cs @@ -8,7 +8,6 @@ internal static partial class Interop internal static partial class Sys { [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_SetTerminalInvalidationHandler")] - [SuppressGCTransition] internal static extern unsafe void SetTerminalInvalidationHandler(delegate* unmanaged handler); } } From 55f1abe6bba4357d41485bc074d89b21701c2cef Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Fri, 18 Jun 2021 10:01:12 +0200 Subject: [PATCH 17/65] Support canceling terminal configuration on SIGCHLD/SIGCONT --- .../Interop.RegisterForSigChld.cs | 2 +- ...ayedSigChildConsoleConfigurationHandler.cs | 14 +++++++ .../Native/Unix/System.Native/entrypoints.c | 1 + .../Native/Unix/System.Native/pal_signal.c | 42 ++++++++++++++----- .../Native/Unix/System.Native/pal_signal.h | 4 +- .../src/System.Diagnostics.Process.csproj | 2 + ...Unix.ConfigureTerminalForChildProcesses.cs | 5 ++- .../src/System/Diagnostics/Process.Unix.cs | 41 +++++++++++++++--- .../Diagnostics/ProcessWaitState.Unix.cs | 10 ++--- 9 files changed, 97 insertions(+), 24 deletions(-) create mode 100644 src/libraries/Common/src/Interop/Unix/System.Native/Interop.SetDelayedSigChildConsoleConfigurationHandler.cs diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.RegisterForSigChld.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.RegisterForSigChld.cs index 3082c661cbaec..e11fcfaf0e65c 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.RegisterForSigChld.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.RegisterForSigChld.cs @@ -8,6 +8,6 @@ internal static partial class Interop internal static partial class Sys { [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_RegisterForSigChld")] - internal static extern unsafe void RegisterForSigChld(delegate* unmanaged handler); + internal static extern unsafe void RegisterForSigChld(delegate* unmanaged handler); } } diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.SetDelayedSigChildConsoleConfigurationHandler.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.SetDelayedSigChildConsoleConfigurationHandler.cs new file mode 100644 index 0000000000000..bd09aa12abc00 --- /dev/null +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.SetDelayedSigChildConsoleConfigurationHandler.cs @@ -0,0 +1,14 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.InteropServices; + +internal static partial class Interop +{ + internal static partial class Sys + { + [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_SetDelayedSigChildConsoleConfigurationHandler")] + [SuppressGCTransition] + internal static extern unsafe void SetDelayedSigChildConsoleConfigurationHandler(delegate* unmanaged callback); + } +} diff --git a/src/libraries/Native/Unix/System.Native/entrypoints.c b/src/libraries/Native/Unix/System.Native/entrypoints.c index 70c1d61e61e45..3c85b8bda01fa 100644 --- a/src/libraries/Native/Unix/System.Native/entrypoints.c +++ b/src/libraries/Native/Unix/System.Native/entrypoints.c @@ -217,6 +217,7 @@ static const Entry s_sysNative[] = DllImportEntry(SystemNative_GetProcessArchitecture) DllImportEntry(SystemNative_SearchPath) DllImportEntry(SystemNative_RegisterForSigChld) + DllImportEntry(SystemNative_SetDelayedSigChildConsoleConfigurationHandler) DllImportEntry(SystemNative_SetTerminalInvalidationHandler) DllImportEntry(SystemNative_InitializeTerminalAndSignalHandling) DllImportEntry(SystemNative_SNPrintF) diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index 80c5893439973..a2e8816701f24 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -26,6 +26,8 @@ static bool g_handlerIsInstalled[NSIG]; static volatile TerminalInvalidationCallback g_terminalInvalidationCallback = NULL; // Callback invoked for SIGCHLD static volatile SigChldCallback g_sigChldCallback = NULL; +static volatile bool g_sigChldConsoleConfigurationDelayed; +static void (*g_sigChldConsoleConfigurationCallback)(void); // Callback invoked for for SIGTTOU while terminal settings are changed. static volatile ConsoleSigTtouHandler g_consoleTtouHandler; @@ -200,7 +202,8 @@ static void SignalHandler(int sig, siginfo_t* siginfo, void* context) void SystemNative_DefaultSignalHandler(int signalCode) { - // Perform the default runtime action for the signal. + // Performs action the runtime performs on signal that is cancelable + // using the PosixSignal API. if (signalCode == SIGQUIT || signalCode == SIGTERM || @@ -214,6 +217,22 @@ void SystemNative_DefaultSignalHandler(int signalCode) RestoreSignalHandler(signalCode); kill(getpid(), signalCode); } + else if (signalCode == SIGCHLD) + { + if (g_sigChldConsoleConfigurationDelayed) + { + g_sigChldConsoleConfigurationDelayed = false; + + assert(g_sigChldConsoleConfigurationCallback); + g_sigChldConsoleConfigurationCallback(); + } + } + else if (signalCode == SIGCONT) + { +#ifdef HAS_CONSOLE_SIGNALS + ReinitializeTerminal(); +#endif + } } // Entrypoint for the thread that handles signals where our handling @@ -255,6 +274,7 @@ static void* SignalHandlerLoop(void* arg) } } + bool usePosixSignalHandler = g_hasPosixSignalRegistrations[signalCode - 1]; if (signalCode == SIGCHLD) { // When the original disposition is SIG_IGN, children that terminated did not become zombies. @@ -284,18 +304,13 @@ static void* SignalHandlerLoop(void* arg) if (callback != NULL) { - callback(reapAll ? 1 : 0); + if (callback(reapAll ? 1 : 0, usePosixSignalHandler ? 0 : 1 /* configureConsole */)) + { + g_sigChldConsoleConfigurationDelayed = true; + } } } - else if (signalCode == SIGCONT) - { - // TODO: should this be cancelable? -#ifdef HAS_CONSOLE_SIGNALS - ReinitializeTerminal(); -#endif - } - bool usePosixSignalHandler = g_hasPosixSignalRegistrations[signalCode - 1]; if (usePosixSignalHandler) { assert(g_posixSignalHandler != NULL); @@ -390,6 +405,13 @@ void SystemNative_RegisterForSigChld(SigChldCallback callback) pthread_mutex_unlock(&lock); } +void SystemNative_SetDelayedSigChildConsoleConfigurationHandler(void (*callback)(void)) +{ + assert(g_sigChldConsoleConfigurationCallback == NULL); + + g_sigChldConsoleConfigurationCallback = callback; +} + static bool CreateSignalHandlerThread(int* readFdPtr) { pthread_attr_t attr; diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.h b/src/libraries/Native/Unix/System.Native/pal_signal.h index 572f6b0326d49..ae7a7532e6a88 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.h +++ b/src/libraries/Native/Unix/System.Native/pal_signal.h @@ -13,7 +13,7 @@ */ int32_t InitializeSignalHandlingCore(void); -typedef void (*SigChldCallback)(int reapAll); +typedef int32_t (*SigChldCallback)(int32_t reapAll, int32_t configureConsole); /** * Hooks up the specified callback for notifications when SIGCHLD is received. @@ -24,6 +24,8 @@ PALEXPORT void SystemNative_RegisterForSigChld(SigChldCallback callback); typedef void (*TerminalInvalidationCallback)(void); +PALEXPORT void SystemNative_SetDelayedSigChildConsoleConfigurationHandler(void (*callback)(void)); + /** * Hooks up the specified callback for notifications when SIGCHLD, SIGCONT, SIGWINCH are received. * diff --git a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj index cdd65c22c4431..d24be05a40371 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -257,6 +257,8 @@ Link="Common\Interop\Unix\Interop.ReadLink.cs" /> + 0) { Debug.Assert(s_processStartLock.IsReadLockHeld); + Debug.Assert(configureConsole); // At least one child is using the terminal. Interop.Sys.ConfigureTerminalForChildProcess(childUsesTerminal: true); @@ -25,7 +26,7 @@ static partial void ConfigureTerminalForChildProcessesInner(int increment) { Debug.Assert(s_processStartLock.IsWriteLockHeld); - if (childrenUsingTerminalRemaining == 0) + if (childrenUsingTerminalRemaining == 0 && configureConsole) { // No more children are using the terminal. Interop.Sys.ConfigureTerminalForChildProcess(childUsesTerminal: false); diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index 1b52e4d141ebb..9034bb27d9c63 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -1023,6 +1023,7 @@ private static unsafe void EnsureInitialized() // Register our callback. Interop.Sys.RegisterForSigChld(&OnSigChild); + Interop.Sys.SetDelayedSigChildConsoleConfigurationHandler(&DelayedSigChildConsoleConfiguration); s_initialized = true; } @@ -1030,13 +1031,43 @@ private static unsafe void EnsureInitialized() } [UnmanagedCallersOnly] - private static void OnSigChild(int reapAll) + private static int OnSigChild(int reapAll, int configureConsole) { + // configureConsole is non zero when there are PosixSignalRegistrations that + // may Cancel the terminal configuration that happens when there are no more + // children using the terminal. + // When the registrations don't cancel the terminal configuration, + // DelayedSigChildConsoleConfiguration will be called. + // Lock to avoid races with Process.Start s_processStartLock.EnterWriteLock(); try { - ProcessWaitState.CheckChildren(reapAll != 0); + int childrenUsingTerminalPre = s_childrenUsingTerminalCount; + ProcessWaitState.CheckChildren(reapAll != 0, configureConsole != 0); + int childrenUsingTerminalPost = s_childrenUsingTerminalCount; + + // return whether console configuration was skipped. + return childrenUsingTerminalPre > 0 && childrenUsingTerminalPost == 0 && configureConsole == 0 ? 1 : 0; + } + finally + { + s_processStartLock.ExitWriteLock(); + } + } + + [UnmanagedCallersOnly] + private static void DelayedSigChildConsoleConfiguration() + { + // Lock to avoid races with Process.Start + s_processStartLock.EnterWriteLock(); + try + { + if (s_childrenUsingTerminalCount == 0) + { + // No more children are using the terminal. + Interop.Sys.ConfigureTerminalForChildProcess(childUsesTerminal: false); + } } finally { @@ -1048,11 +1079,11 @@ private static void OnSigChild(int reapAll) /// This method is called when the number of child processes that are using the terminal changes. /// It updates the terminal configuration if necessary. /// - internal static void ConfigureTerminalForChildProcesses(int increment) + internal static void ConfigureTerminalForChildProcesses(int increment, bool configureConsole = true) { - ConfigureTerminalForChildProcessesInner(increment); + ConfigureTerminalForChildProcessesInner(increment, configureConsole); } - static partial void ConfigureTerminalForChildProcessesInner(int increment); + static partial void ConfigureTerminalForChildProcessesInner(int increment, bool configureConsole); } } 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 03e93a725c4b7..482a627918fe4 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 @@ -552,7 +552,7 @@ private Task WaitForExitAsync(CancellationToken cancellationToken = default) }, cancellationToken); } - private bool TryReapChild() + private bool TryReapChild(bool configureConsole) { lock (_gate) { @@ -572,7 +572,7 @@ private bool TryReapChild() if (_usesTerminal) { // Update terminal settings before calling SetExited. - Process.ConfigureTerminalForChildProcesses(-1); + Process.ConfigureTerminalForChildProcesses(-1, configureConsole); } SetExited(); @@ -593,7 +593,7 @@ private bool TryReapChild() } } - internal static void CheckChildren(bool reapAll) + internal static void CheckChildren(bool reapAll, bool configureConsole) { // This is called on SIGCHLD from a native thread. // A lock in Process ensures no new processes are spawned while we are checking. @@ -612,7 +612,7 @@ internal static void CheckChildren(bool reapAll) if (s_childProcessWaitStates.TryGetValue(pid, out ProcessWaitState? pws)) { // Known Process. - if (pws.TryReapChild()) + if (pws.TryReapChild(configureConsole)) { pws.ReleaseRef(); } @@ -645,7 +645,7 @@ internal static void CheckChildren(bool reapAll) foreach (KeyValuePair kv in s_childProcessWaitStates) { ProcessWaitState pws = kv.Value; - if (pws.TryReapChild()) + if (pws.TryReapChild(configureConsole)) { if (firstToRemove == null) { From 7a2594ca8a7ccd95d679e44f612196c2c04b04fc Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Fri, 18 Jun 2021 10:21:16 +0200 Subject: [PATCH 18/65] Throw for errno using Interop.CheckIO --- .../src/Resources/Strings.resx | 36 +++++++++++++++++++ .../src/System.Runtime.InteropServices.csproj | 4 +++ .../PosixSignalRegistration.Unix.cs | 4 ++- 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Runtime.InteropServices/src/Resources/Strings.resx b/src/libraries/System.Runtime.InteropServices/src/Resources/Strings.resx index b65b6b5d5b1a2..7834b952cc9a9 100644 --- a/src/libraries/System.Runtime.InteropServices/src/Resources/Strings.resx +++ b/src/libraries/System.Runtime.InteropServices/src/Resources/Strings.resx @@ -75,4 +75,40 @@ Event invocation for COM objects requires event to be attributed with DispIdAttribute. + + The file '{0}' already exists. + + + Unable to find the specified file. + + + Could not find file '{0}'. + + + Could not find a part of the path. + + + Could not find a part of the path '{0}'. + + + The specified file name or path is too long, or a component of the specified path is too long. + + + The process cannot access the file '{0}' because it is being used by another process. + + + The process cannot access the file because it is being used by another process. + + + The path '{0}' is too long, or a component of the specified path is too long. + + + Access to the path '{0}' is denied. + + + Access to the path is denied. + + + Specified file length was too large for the file system. + \ No newline at end of file diff --git a/src/libraries/System.Runtime.InteropServices/src/System.Runtime.InteropServices.csproj b/src/libraries/System.Runtime.InteropServices/src/System.Runtime.InteropServices.csproj index 3f618438e16c4..78d6c70d9a08f 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System.Runtime.InteropServices.csproj +++ b/src/libraries/System.Runtime.InteropServices/src/System.Runtime.InteropServices.csproj @@ -58,6 +58,10 @@ Link="Common\Interop\Unix\Interop.InitializeTerminalAndSignalHandling.cs" /> + + diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs index 3b6cd9eff3caf..80f7af83462b4 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs @@ -45,7 +45,9 @@ private unsafe void Register() { if (!Interop.Sys.InitializeTerminalAndSignalHandling()) { - throw new Exception(); // TODO: can this throw Win32Exception? + // We can't use Win32Exception because that causes a cycle with + // Microsoft.Win32.Primitives. + Interop.CheckIo(-1); } Interop.Sys.SetPosixSignalHandler(&OnPosixSignal); s_initialized = true; From 9c27d6ea46c69e24e0cef1b223005934c62c886a Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Fri, 18 Jun 2021 10:25:41 +0200 Subject: [PATCH 19/65] Rename DefaultSignalHandler to HandleNonCanceledPosixSignal --- .../src/Interop/Unix/System.Native/Interop.PosixSignal.cs | 4 ++-- src/libraries/Native/Unix/System.Native/entrypoints.c | 2 +- src/libraries/Native/Unix/System.Native/pal_signal.c | 4 ++-- src/libraries/Native/Unix/System.Native/pal_signal.h | 2 +- .../Runtime/InteropServices/PosixSignalRegistration.Unix.cs | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) 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 5a0e903fbdd6c..4d7d27bde23ac 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 @@ -17,8 +17,8 @@ internal static partial class Sys [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_DisablePosixSignalHandling")] internal static extern void DisablePosixSignalHandling(int signal); - [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_DefaultSignalHandler")] - internal static extern void DefaultSignalHandler(int signal); + [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_HandleNonCanceledPosixSignal")] + internal static extern void HandleNonCanceledPosixSignal(int signal); [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetPlatformSignalNumber")] [SuppressGCTransition] diff --git a/src/libraries/Native/Unix/System.Native/entrypoints.c b/src/libraries/Native/Unix/System.Native/entrypoints.c index 3c85b8bda01fa..4455d9896adee 100644 --- a/src/libraries/Native/Unix/System.Native/entrypoints.c +++ b/src/libraries/Native/Unix/System.Native/entrypoints.c @@ -254,7 +254,7 @@ static const Entry s_sysNative[] = DllImportEntry(SystemNative_HandlePosixSignal) DllImportEntry(SystemNative_EnablePosixSignalHandling) DllImportEntry(SystemNative_DisablePosixSignalHandling) - DllImportEntry(SystemNative_DefaultSignalHandler) + DllImportEntry(SystemNative_HandleNonCanceledPosixSignal) DllImportEntry(SystemNative_SetPosixSignalHandler) DllImportEntry(SystemNative_GetPlatformSignalNumber) }; diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index a2e8816701f24..c9874e051b86a 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -200,7 +200,7 @@ static void SignalHandler(int sig, siginfo_t* siginfo, void* context) } } -void SystemNative_DefaultSignalHandler(int signalCode) +void SystemNative_HandleNonCanceledPosixSignal(int signalCode) { // Performs action the runtime performs on signal that is cancelable // using the PosixSignal API. @@ -324,7 +324,7 @@ static void* SignalHandlerLoop(void* arg) if (!usePosixSignalHandler) { - SystemNative_DefaultSignalHandler(signalCode); + 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 ae7a7532e6a88..8860fb9bc1744 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.h +++ b/src/libraries/Native/Unix/System.Native/pal_signal.h @@ -52,7 +52,7 @@ PALEXPORT void SystemNative_SetPosixSignalHandler(PosixSignalHandler signalHandl PALEXPORT int32_t SystemNative_GetPlatformSignalNumber(PosixSignal signal); PALEXPORT void SystemNative_EnablePosixSignalHandling(int signalCode); PALEXPORT void SystemNative_DisablePosixSignalHandling(int signalCode); -PALEXPORT void SystemNative_DefaultSignalHandler(int signalCode); +PALEXPORT void SystemNative_HandleNonCanceledPosixSignal(int signalCode); typedef void (*ConsoleSigTtouHandler)(void); diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs index 80f7af83462b4..b026ce9f33d95 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs @@ -157,7 +157,7 @@ private static void HandleSignal((int signo, PosixSignalRegistration?[] registra if (!ctx.Cancel) { - Interop.Sys.DefaultSignalHandler(state.signo); + Interop.Sys.HandleNonCanceledPosixSignal(state.signo); } } From b5ed74dcb59f5da701d4f5170c528a194370186b Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 21 Jun 2021 08:33:51 +0200 Subject: [PATCH 20/65] Register Console SIGTTOU with SA_RESETHAND --- .../Native/Unix/System.Native/pal_signal.c | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index c9874e051b86a..13b87fc144e66 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -339,7 +339,7 @@ static void CloseSignalHandlingPipe() g_signalPipe[1] = -1; } -static void InstallSignalHandler(int sig) +static void InstallSignalHandler(int sig, int flags) { int rv; (void)rv; // only used for assert @@ -366,7 +366,7 @@ static void InstallSignalHandler(int sig) struct sigaction newAction; memset(&newAction, 0, sizeof(struct sigaction)); - newAction.sa_flags = SA_RESTART | SA_SIGINFO; + newAction.sa_flags = flags | SA_SIGINFO; sigemptyset(&newAction.sa_mask); newAction.sa_sigaction = &SignalHandler; @@ -384,9 +384,9 @@ void SystemNative_SetTerminalInvalidationHandler(TerminalInvalidationCallback ca { g_terminalInvalidationCallback = callback; - InstallSignalHandler(SIGCONT); - InstallSignalHandler(SIGCHLD); - InstallSignalHandler(SIGWINCH); + InstallSignalHandler(SIGCONT, SA_RESTART); + InstallSignalHandler(SIGCHLD, SA_RESTART); + InstallSignalHandler(SIGWINCH, SA_RESTART); } pthread_mutex_unlock(&lock); } @@ -400,7 +400,7 @@ void SystemNative_RegisterForSigChld(SigChldCallback callback) { g_sigChldCallback = callback; - InstallSignalHandler(SIGCHLD); + InstallSignalHandler(SIGCHLD, SA_RESTART); } pthread_mutex_unlock(&lock); } @@ -487,7 +487,7 @@ void SystemNative_EnablePosixSignalHandling(int signalCode) pthread_mutex_lock(&lock); { - InstallSignalHandler(signalCode); + InstallSignalHandler(signalCode, SA_RESTART); g_hasPosixSignalRegistrations[signalCode - 1] = true; } @@ -521,7 +521,15 @@ void InstallTTOUHandlerForConsole(ConsoleSigTtouHandler handler) assert(g_consoleTtouHandler == NULL); g_consoleTtouHandler = handler; - InstallSignalHandler(SIGTTOU); + // When the process is running in background, changing terminal settings + // will stop it (default SIGTTOU action). + // We change SIGTTOU's disposition to get EINTR instead. + // This thread may be used to run a signal handler, which may write to + // stdout. We set SA_RESETHAND to avoid that handler's write loops infinitly + // on EINTR when the process is running in background and the terminal + // configured with TOSTOP. + RestoreSignalHandler(SIGTTOU); + InstallSignalHandler(SIGTTOU, (int)SA_RESETHAND); } pthread_mutex_unlock(&lock); } @@ -532,10 +540,10 @@ void UninstallTTOUHandlerForConsole(void) { g_consoleTtouHandler = NULL; - // Don't restore SIGTTOU while there are PosixSignal registrations. - if (!g_hasPosixSignalRegistrations[SIGTTOU - 1]) + RestoreSignalHandler(SIGTTOU); + if (g_hasPosixSignalRegistrations[SIGTTOU - 1]) { - RestoreSignalHandler(SIGTTOU); + InstallSignalHandler(SIGTTOU, SA_RESTART); } } pthread_mutex_unlock(&lock); From 76b870423dd7ff52f8d92dfcc814be893d577c54 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 21 Jun 2021 09:13:19 +0200 Subject: [PATCH 21/65] Allocate storage for tracking signals dynamically --- .../Native/Unix/System.Native/pal_signal.c | 43 ++++++++++++++----- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index 13b87fc144e66..dc10a2822ab96 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -19,8 +19,8 @@ static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER; // Saved signal handlers -static struct sigaction g_origSigHandler[NSIG]; -static bool g_handlerIsInstalled[NSIG]; +static struct sigaction* g_origSigHandler; +static bool* g_handlerIsInstalled; // Callback invoked for SIGCHLD/SIGCONT/SIGWINCH static volatile TerminalInvalidationCallback g_terminalInvalidationCallback = NULL; @@ -34,10 +34,15 @@ static volatile ConsoleSigTtouHandler g_consoleTtouHandler; // Callback invoked for PosixSignal handling. static PosixSignalHandler g_posixSignalHandler = NULL; // Tracks whether there are PosixSignal handlers registered. -static volatile bool g_hasPosixSignalRegistrations[NSIG]; +static volatile bool* g_hasPosixSignalRegistrations; static int g_signalPipe[2] = {-1, -1}; // Pipe used between signal handler and worker +static int GetSignalMax() // Returns the highest usable signal number. +{ + return NSIG; +} + static bool TryConvertSignalCodeToPosixSignal(int signalCode, PosixSignal* posixSignal) { assert(posixSignal != NULL); @@ -125,12 +130,7 @@ int32_t SystemNative_GetPlatformSignalNumber(PosixSignal signal) return SIGTSTP; } - if ( signal > 0 - && signal <= NSIG // Ensure we stay within the static arrays. -#ifdef SIGRTMAX - && signal <= SIGRTMAX // Runtime check for highest value. -#endif - ) + if (signal > 0 && signal <= GetSignalMax()) { return signal; } @@ -445,6 +445,27 @@ static bool CreateSignalHandlerThread(int* readFdPtr) int32_t InitializeSignalHandlingCore() { + size_t signalMax = (size_t)GetSignalMax(); + g_origSigHandler = (struct sigaction*)calloc(sizeof(struct sigaction), signalMax); + g_handlerIsInstalled = (bool*)calloc(sizeof(bool), signalMax); + g_hasPosixSignalRegistrations = (bool*)calloc(sizeof(bool), signalMax); + if (g_origSigHandler == NULL || + g_handlerIsInstalled == NULL || + g_hasPosixSignalRegistrations == NULL) + { + free(g_origSigHandler); + free(g_handlerIsInstalled); +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wcast-qual" + free((void*)g_hasPosixSignalRegistrations); +#pragma clang diagnostic pop + g_origSigHandler = NULL; + g_handlerIsInstalled = NULL; + g_hasPosixSignalRegistrations = NULL; + errno = ENOMEM; + return 0; + } + // Create a pipe we'll use to communicate with our worker // thread. We can't do anything interesting in the signal handler, // so we instead send a message to another thread that'll do @@ -483,7 +504,7 @@ int32_t InitializeSignalHandlingCore() void SystemNative_EnablePosixSignalHandling(int signalCode) { assert(g_posixSignalHandler != NULL); - assert(signalCode > 0 && signalCode <= NSIG); + assert(signalCode > 0 && signalCode <= GetSignalMax()); pthread_mutex_lock(&lock); { @@ -496,7 +517,7 @@ void SystemNative_EnablePosixSignalHandling(int signalCode) void SystemNative_DisablePosixSignalHandling(int signalCode) { - assert(signalCode > 0 && signalCode <= NSIG); + assert(signalCode > 0 && signalCode <= GetSignalMax()); pthread_mutex_lock(&lock); { From 5f5deb3389a59e395b7ef06ca37c4d7c2e1ef416 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 21 Jun 2021 09:23:20 +0200 Subject: [PATCH 22/65] Propagate sigaction errors when enabling PosixSignal --- .../Unix/System.Native/Interop.PosixSignal.cs | 2 +- .../Native/Unix/System.Native/pal_signal.c | 27 +++++++++++++------ .../Native/Unix/System.Native/pal_signal.h | 2 +- .../PosixSignalRegistration.Unix.cs | 7 ++++- 4 files changed, 27 insertions(+), 11 deletions(-) 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 4d7d27bde23ac..683c29329cb6d 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 @@ -12,7 +12,7 @@ internal static partial class Sys internal static extern unsafe void SetPosixSignalHandler(delegate* unmanaged handler); [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_EnablePosixSignalHandling")] - internal static extern void EnablePosixSignalHandling(int signal); + internal static extern bool EnablePosixSignalHandling(int signal); [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_DisablePosixSignalHandling")] internal static extern void DisablePosixSignalHandling(int signal); diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index dc10a2822ab96..446a9441b3664 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -339,7 +339,7 @@ static void CloseSignalHandlingPipe() g_signalPipe[1] = -1; } -static void InstallSignalHandler(int sig, int flags) +static bool InstallSignalHandler(int sig, int flags) { int rv; (void)rv; // only used for assert @@ -349,7 +349,7 @@ static void InstallSignalHandler(int sig, int flags) if (*isInstalled) { // Already installed. - return; + return true; } // We respect ignored signals. @@ -357,11 +357,14 @@ static void InstallSignalHandler(int sig, int flags) // default handler on exec, which means they will terminate on some signals // which were set to ignore. rv = sigaction(sig, NULL, orig); - assert(rv == 0); + if (rv == 0) + { + return false; + } if ((void*)orig->sa_sigaction == (void*)SIG_IGN) { *isInstalled = true; - return; + return true; } struct sigaction newAction; @@ -371,8 +374,12 @@ static void InstallSignalHandler(int sig, int flags) newAction.sa_sigaction = &SignalHandler; rv = sigaction(sig, &newAction, orig); - assert(rv == 0); + if (rv == 0) + { + return false; + } *isInstalled = true; + return false; } void SystemNative_SetTerminalInvalidationHandler(TerminalInvalidationCallback callback) @@ -501,18 +508,22 @@ int32_t InitializeSignalHandlingCore() return 1; } -void SystemNative_EnablePosixSignalHandling(int signalCode) +int32_t SystemNative_EnablePosixSignalHandling(int signalCode) { assert(g_posixSignalHandler != NULL); assert(signalCode > 0 && signalCode <= GetSignalMax()); + bool installed; + pthread_mutex_lock(&lock); { - InstallSignalHandler(signalCode, SA_RESTART); + installed = InstallSignalHandler(signalCode, SA_RESTART); - g_hasPosixSignalRegistrations[signalCode - 1] = true; + g_hasPosixSignalRegistrations[signalCode - 1] = installed; } pthread_mutex_unlock(&lock); + + return installed ? 1 : 0; } void SystemNative_DisablePosixSignalHandling(int signalCode) diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.h b/src/libraries/Native/Unix/System.Native/pal_signal.h index 8860fb9bc1744..95d05fd0f73bb 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.h +++ b/src/libraries/Native/Unix/System.Native/pal_signal.h @@ -50,7 +50,7 @@ typedef int32_t (*PosixSignalHandler)(int32_t signalCode, PosixSignal signal); PALEXPORT void SystemNative_SetPosixSignalHandler(PosixSignalHandler signalHandler); PALEXPORT int32_t SystemNative_GetPlatformSignalNumber(PosixSignal signal); -PALEXPORT void SystemNative_EnablePosixSignalHandling(int signalCode); +PALEXPORT int32_t SystemNative_EnablePosixSignalHandling(int signalCode); PALEXPORT void SystemNative_DisablePosixSignalHandling(int signalCode); PALEXPORT void SystemNative_HandleNonCanceledPosixSignal(int signalCode); diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs index b026ce9f33d95..8bf1ddf7b6d46 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs @@ -61,7 +61,12 @@ private unsafe void Register() } if (signalRegistrations.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); + } } signalRegistrations.Add(new WeakReference(this)); } From 03c03b8ed119bf89a7de82afdb194e5af961f33d Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 21 Jun 2021 12:10:10 +0200 Subject: [PATCH 23/65] Add some tests --- .../Native/Unix/System.Native/pal_signal.c | 6 +- ...ystem.Runtime.InteropServices.Tests.csproj | 1 + .../InteropServices/PosixSignalTests.cs | 117 ++++++++++++++++++ 3 files changed, 121 insertions(+), 3 deletions(-) create mode 100644 src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalTests.cs diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index 446a9441b3664..479c586f0d8df 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -357,7 +357,7 @@ static bool InstallSignalHandler(int sig, int flags) // default handler on exec, which means they will terminate on some signals // which were set to ignore. rv = sigaction(sig, NULL, orig); - if (rv == 0) + if (rv != 0) { return false; } @@ -374,12 +374,12 @@ static bool InstallSignalHandler(int sig, int flags) newAction.sa_sigaction = &SignalHandler; rv = sigaction(sig, &newAction, orig); - if (rv == 0) + if (rv != 0) { return false; } *isInstalled = true; - return false; + return true; } void SystemNative_SetTerminalInvalidationHandler(TerminalInvalidationCallback callback) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.Tests.csproj b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.Tests.csproj index 9ed8c6ff8a776..b5cb4b6662aae 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.Tests.csproj +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.Tests.csproj @@ -150,6 +150,7 @@ + diff --git a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalTests.cs new file mode 100644 index 0000000000000..8452e9794a18c --- /dev/null +++ b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalTests.cs @@ -0,0 +1,117 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Xunit; +using System.Threading; +using System.Runtime.InteropServices; + +namespace System.Tests +{ + public class PosixSignalTests + { + private static TimeSpan Timeout => TimeSpan.FromSeconds(30); + + [Fact] + public void HandlerNullThrows() + { + Assert.Throws(() => PosixSignalRegistration.Create(PosixSignal.SIGCONT, null)); + } + + [Theory] + [InlineData(0)] + [InlineData(-1000)] + [InlineData(1000)] + public void InvalidSignalValueThrows(int value) + { + Assert.Throws(() => PosixSignalRegistration.Create((PosixSignal)value, ctx => { })); + } + + [Theory] + [MemberData(nameof(PosixSignalValues))] + public void CanRegisterForKnownValues(PosixSignal signal) + { + using var _ = PosixSignalRegistration.Create(signal, ctx => { }); + } + + [Theory] + [MemberData(nameof(PosixSignalValues))] + public void SignalHandlerCalledForKnownSignals(PosixSignal signal) + { + using SemaphoreSlim semaphore = new(0); + using var _ = PosixSignalRegistration.Create(signal, ctx => + { + Assert.Equal(signal, ctx.Signal); + + // Ensure signal doesn't cause the process to terminate. + ctx.Cancel = true; + + semaphore.Release(); + }); + kill(signal); + bool entered = semaphore.Wait(Timeout); + Assert.True(entered); + } + + [Theory] + [MemberData(nameof(PosixSignalAsRawValues))] + public void SignalHandlerCalledForRawSignals(PosixSignal signal) + { + using SemaphoreSlim semaphore = new(0); + using var _ = PosixSignalRegistration.Create(signal, ctx => + { + Assert.Equal(signal, ctx.Signal); + + // Ensure signal doesn't cause the process to terminate. + ctx.Cancel = true; + + semaphore.Release(); + }); + kill(signal); + bool entered = semaphore.Wait(Timeout); + Assert.True(entered); + } + + public static TheoryData PosixSignalValues + { + get + { + var data = new TheoryData(); + foreach (var value in Enum.GetValues(typeof(PosixSignal))) + { + data.Add((PosixSignal)value); + } + return data; + } + } + + public static TheoryData PosixSignalAsRawValues + { + get + { + var data = new TheoryData(); + foreach (var value in Enum.GetValues(typeof(PosixSignal))) + { + int signo = GetPlatformSignalNumber((PosixSignal)value); + Assert.True(signo > 0, "Expected raw signal number to be greater than 0."); + data.Add((PosixSignal)signo); + } + return data; + } + } + + [DllImport("libc", SetLastError = true)] + private static extern int kill(int pid, int sig); + + private static void kill(PosixSignal sig) + { + int signo = GetPlatformSignalNumber(sig); + Assert.NotEqual(0, signo); + int rv = kill(Environment.ProcessId, signo); + Assert.Equal(0, rv); + } + + [DllImport("libSystem.Native", EntryPoint = "SystemNative_GetPlatformSignalNumber")] + [SuppressGCTransition] + private static extern int GetPlatformSignalNumber(PosixSignal signal); + } +} From 6f0c69b834650d807a197f1dd7debcbf1369f542 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 21 Jun 2021 13:08:24 +0200 Subject: [PATCH 24/65] Fix entrypoints --- src/libraries/Native/Unix/System.Native/entrypoints.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/libraries/Native/Unix/System.Native/entrypoints.c b/src/libraries/Native/Unix/System.Native/entrypoints.c index 4455d9896adee..b1b5a92e5f35b 100644 --- a/src/libraries/Native/Unix/System.Native/entrypoints.c +++ b/src/libraries/Native/Unix/System.Native/entrypoints.c @@ -249,9 +249,6 @@ static const Entry s_sysNative[] = DllImportEntry(SystemNative_PWrite) DllImportEntry(SystemNative_PReadV) DllImportEntry(SystemNative_PWriteV) - DllImportEntry(SystemNative_RegisterForPosixSignal) - DllImportEntry(SystemNative_UnregisterForPosixSignal) - DllImportEntry(SystemNative_HandlePosixSignal) DllImportEntry(SystemNative_EnablePosixSignalHandling) DllImportEntry(SystemNative_DisablePosixSignalHandling) DllImportEntry(SystemNative_HandleNonCanceledPosixSignal) From d873120c4113d6e48c63830663cb8af665740bf2 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 21 Jun 2021 13:56:08 +0200 Subject: [PATCH 25/65] Fix compilation --- ....Unix.ConfigureTerminalForChildProcesses.cs | 18 ++++++++++++++++++ .../src/System/Diagnostics/Process.Unix.cs | 17 +++-------------- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.ConfigureTerminalForChildProcesses.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.ConfigureTerminalForChildProcesses.cs index b1ef8db228b52..96f9e1103ece8 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.ConfigureTerminalForChildProcesses.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.ConfigureTerminalForChildProcesses.cs @@ -33,5 +33,23 @@ static partial void ConfigureTerminalForChildProcessesInner(int increment, bool } } } + + static partial void DelayedSigChildConsoleConfigurationInner() + { + // Lock to avoid races with Process.Start + s_processStartLock.EnterWriteLock(); + try + { + if (s_childrenUsingTerminalCount == 0) + { + // No more children are using the terminal. + Interop.Sys.ConfigureTerminalForChildProcess(childUsesTerminal: false); + } + } + finally + { + s_processStartLock.ExitWriteLock(); + } + } } } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index 9034bb27d9c63..a7e329661d9eb 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -1059,22 +1059,11 @@ private static int OnSigChild(int reapAll, int configureConsole) [UnmanagedCallersOnly] private static void DelayedSigChildConsoleConfiguration() { - // Lock to avoid races with Process.Start - s_processStartLock.EnterWriteLock(); - try - { - if (s_childrenUsingTerminalCount == 0) - { - // No more children are using the terminal. - Interop.Sys.ConfigureTerminalForChildProcess(childUsesTerminal: false); - } - } - finally - { - s_processStartLock.ExitWriteLock(); - } + DelayedSigChildConsoleConfigurationInner(); } + static partial void DelayedSigChildConsoleConfigurationInner(); + /// /// This method is called when the number of child processes that are using the terminal changes. /// It updates the terminal configuration if necessary. From bc1590c37e1e2635c3e294aa4283e5f9d9d75ca6 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 21 Jun 2021 14:42:21 +0200 Subject: [PATCH 26/65] Add some more tests --- .../Unix/System.Native/Interop.PosixSignal.cs | 2 +- .../InteropServices/PosixSignalTests.cs | 70 +++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) 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 683c29329cb6d..65e3ca35f4844 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 @@ -11,7 +11,7 @@ internal static partial class Sys [SuppressGCTransition] internal static extern unsafe void SetPosixSignalHandler(delegate* unmanaged handler); - [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_EnablePosixSignalHandling")] + [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_EnablePosixSignalHandling", SetLastError = true)] internal static extern bool EnablePosixSignalHandling(int signal); [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_DisablePosixSignalHandling")] diff --git a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalTests.cs index 8452e9794a18c..ff65ca320702f 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalTests.cs @@ -2,8 +2,10 @@ // The .NET Foundation licenses this file to you under the MIT license. using Xunit; +using System.IO; using System.Threading; using System.Runtime.InteropServices; +using System.Runtime.CompilerServices; namespace System.Tests { @@ -26,6 +28,14 @@ public void InvalidSignalValueThrows(int value) Assert.Throws(() => PosixSignalRegistration.Create((PosixSignal)value, ctx => { })); } + [Theory] + [InlineData((PosixSignal)9)] // SIGKILL + [InlineData((PosixSignal)19)] // SIGSTOP + public void UninstallableSignalsThrow(PosixSignal signal) + { + Assert.Throws(() => PosixSignalRegistration.Create(signal, ctx => { })); + } + [Theory] [MemberData(nameof(PosixSignalValues))] public void CanRegisterForKnownValues(PosixSignal signal) @@ -71,6 +81,66 @@ public void SignalHandlerCalledForRawSignals(PosixSignal signal) Assert.True(entered); } + [Fact] + public void SignalHandlerWorksForSecondRegistration() + { + PosixSignal signal = PosixSignal.SIGCONT; + + for (int i = 0; i < 2; i++) + { + using SemaphoreSlim semaphore = new(0); + using var _ = PosixSignalRegistration.Create(signal, ctx => + { + Assert.Equal(signal, ctx.Signal); + + // Ensure signal doesn't cause the process to terminate. + ctx.Cancel = true; + + semaphore.Release(); + }); + kill(signal); + bool entered = semaphore.Wait(Timeout); + Assert.True(entered); + } + } + + [Fact] + public void SignalHandlerCalledNotCalledWhenDisposed() + { + PosixSignal signal = PosixSignal.SIGCONT; + + using var registration = PosixSignalRegistration.Create(signal, ctx => + { + Assert.False(true, "Signal handler was called."); + }); + registration.Dispose(); + + kill(signal); + Thread.Sleep(100); + } + + [Fact] + public void SignalHandlerCalledNotCalledWhenFinalized() + { + PosixSignal signal = PosixSignal.SIGCONT; + + CreateDanglingRegistration(); + GC.Collect(); + GC.WaitForPendingFinalizers(); + + kill(signal); + Thread.Sleep(100); + + [MethodImpl(MethodImplOptions.NoInlining)] + void CreateDanglingRegistration() + { + PosixSignalRegistration.Create(signal, ctx => + { + Assert.False(true, "Signal handler was called."); + }); + } + } + public static TheoryData PosixSignalValues { get From e8827ceb09b431c8ec14fc14c49e2ec351a38dfa Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 21 Jun 2021 15:09:05 +0200 Subject: [PATCH 27/65] Move tests to PosixSignalRegistrationTests.Unix file --- .../tests/System.Runtime.InteropServices.Tests.csproj | 8 ++++++-- ...ignalTests.cs => PosixSignalRegistrationTests.Unix.cs} | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) rename src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/{PosixSignalTests.cs => PosixSignalRegistrationTests.Unix.cs} (98%) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.Tests.csproj b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.Tests.csproj index b5cb4b6662aae..e86fb807464df 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.Tests.csproj +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.Tests.csproj @@ -1,7 +1,7 @@ true - $(NetCoreAppCurrent);$(NetCoreAppCurrent)-windows + $(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser true true @@ -150,7 +150,6 @@ - @@ -184,4 +183,9 @@ + + + + diff --git a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs similarity index 98% rename from src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalTests.cs rename to src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs index ff65ca320702f..9419c89493043 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs @@ -180,7 +180,7 @@ private static void kill(PosixSignal sig) Assert.Equal(0, rv); } - [DllImport("libSystem.Native", EntryPoint = "SystemNative_GetPlatformSignalNumber")] + [DllImport(Interop.Libraries.SystemNative, EntryPoint = "SystemNative_GetPlatformSignalNumber")] [SuppressGCTransition] private static extern int GetPlatformSignalNumber(PosixSignal signal); } From 511f60420f2a1419e8b0db5f485e416116f762b2 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 21 Jun 2021 16:01:10 +0200 Subject: [PATCH 28/65] Add SignalCanCancelTermination test --- ...ystem.Runtime.InteropServices.Tests.csproj | 2 +- .../PosixSignalRegistrationTests.Unix.cs | 35 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.Tests.csproj b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.Tests.csproj index e86fb807464df..a30422e68cbc4 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.Tests.csproj +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.Tests.csproj @@ -1,7 +1,7 @@ true - $(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser + $(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser true true diff --git a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs index 9419c89493043..db51359ff4bff 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs @@ -4,8 +4,10 @@ using Xunit; using System.IO; using System.Threading; +using System.Threading.Tasks; using System.Runtime.InteropServices; using System.Runtime.CompilerServices; +using Microsoft.DotNet.RemoteExecutor; namespace System.Tests { @@ -141,6 +143,39 @@ void CreateDanglingRegistration() } } + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(PosixSignal.SIGINT, true, 0)] + [InlineData(PosixSignal.SIGINT, false, 130)] + [InlineData(PosixSignal.SIGTERM, true, 0)] + [InlineData(PosixSignal.SIGTERM, false, 143)] + [InlineData(PosixSignal.SIGQUIT, true, 0)] + [InlineData(PosixSignal.SIGQUIT, false, 999)] // TODO: 131 + public void SignalCanCancelTermination(PosixSignal signal, bool cancel, int expectedExitCode) + { + RemoteExecutor.Invoke((signalStr, cancelStr) => + { + PosixSignal signalArg = Enum.Parse(signalStr); + bool cancelArg = bool.Parse(cancelStr); + + using SemaphoreSlim semaphore = new(0); + using var _ = PosixSignalRegistration.Create(signalArg, ctx => + { + ctx.Cancel = cancelArg; + + semaphore.Release(); + }); + + kill(signalArg); + + bool entered = semaphore.Wait(Timeout); + Assert.True(entered); + + // Give the default signal handler a chance to run. + Thread.Sleep(cancelArg ? TimeSpan.FromSeconds(1) : Timeout); + }, signal.ToString(), cancel.ToString(), + new RemoteInvokeOptions() { ExpectedExitCode = expectedExitCode }).Dispose(); + } + public static TheoryData PosixSignalValues { get From 1d15e42b98e9cc7d4557860d94c60e903b691b16 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 21 Jun 2021 17:14:16 +0200 Subject: [PATCH 29/65] ConfigureTerminalForChildProcesses: split Unix and iOS --- .../src/System.Diagnostics.Process.csproj | 5 +++- ...onfigureTerminalForChildProcesses.Unix.cs} | 13 +++++++-- ....ConfigureTerminalForChildProcesses.iOS.cs | 21 +++++++++++++++ .../src/System/Diagnostics/Process.Unix.cs | 27 ++++--------------- 4 files changed, 41 insertions(+), 25 deletions(-) rename src/libraries/System.Diagnostics.Process/src/System/Diagnostics/{Process.Unix.ConfigureTerminalForChildProcesses.cs => Process.ConfigureTerminalForChildProcesses.Unix.cs} (75%) create mode 100644 src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.ConfigureTerminalForChildProcesses.iOS.cs diff --git a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj index d24be05a40371..424a7a87b31c6 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -273,7 +273,10 @@ - + + + + diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.ConfigureTerminalForChildProcesses.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.ConfigureTerminalForChildProcesses.Unix.cs similarity index 75% rename from src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.ConfigureTerminalForChildProcesses.cs rename to src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.ConfigureTerminalForChildProcesses.Unix.cs index 96f9e1103ece8..2f3a5f7c8bb08 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.ConfigureTerminalForChildProcesses.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.ConfigureTerminalForChildProcesses.Unix.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Threading; +using System.Runtime.InteropServices; namespace System.Diagnostics { @@ -9,7 +10,7 @@ public partial class Process { private static int s_childrenUsingTerminalCount; - static partial void ConfigureTerminalForChildProcessesInner(int increment, bool configureConsole) + internal static void ConfigureTerminalForChildProcesses(int increment, bool configureConsole = true) { Debug.Assert(increment != 0); @@ -34,7 +35,13 @@ static partial void ConfigureTerminalForChildProcessesInner(int increment, bool } } - static partial void DelayedSigChildConsoleConfigurationInner() + static unsafe partial void SetDelayedSigChildConsoleConfigurationHandler() + { + Interop.Sys.SetDelayedSigChildConsoleConfigurationHandler(&DelayedSigChildConsoleConfiguration); + } + + [UnmanagedCallersOnly] + private static void DelayedSigChildConsoleConfiguration() { // Lock to avoid races with Process.Start s_processStartLock.EnterWriteLock(); @@ -51,5 +58,7 @@ static partial void DelayedSigChildConsoleConfigurationInner() s_processStartLock.ExitWriteLock(); } } + + private static bool AreChildrenUsingTerminal => s_childrenUsingTerminalCount > 0; } } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.ConfigureTerminalForChildProcesses.iOS.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.ConfigureTerminalForChildProcesses.iOS.cs new file mode 100644 index 0000000000000..d89116504da7d --- /dev/null +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.ConfigureTerminalForChildProcesses.iOS.cs @@ -0,0 +1,21 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Threading; + +namespace System.Diagnostics +{ + public partial class Process + { + /// + /// This method is called when the number of child processes that are using the terminal changes. + /// It updates the terminal configuration if necessary. + /// + internal static void ConfigureTerminalForChildProcesses(int increment, bool configureConsole = true) + { + + } + + private static bool AreChildrenUsingTerminal => false; + } +} diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index a7e329661d9eb..63d98d75b1744 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -1023,7 +1023,7 @@ private static unsafe void EnsureInitialized() // Register our callback. Interop.Sys.RegisterForSigChld(&OnSigChild); - Interop.Sys.SetDelayedSigChildConsoleConfigurationHandler(&DelayedSigChildConsoleConfiguration); + SetDelayedSigChildConsoleConfigurationHandler(); s_initialized = true; } @@ -1043,12 +1043,12 @@ private static int OnSigChild(int reapAll, int configureConsole) s_processStartLock.EnterWriteLock(); try { - int childrenUsingTerminalPre = s_childrenUsingTerminalCount; + bool childrenUsingTerminalPre = AreChildrenUsingTerminal; ProcessWaitState.CheckChildren(reapAll != 0, configureConsole != 0); - int childrenUsingTerminalPost = s_childrenUsingTerminalCount; + bool childrenUsingTerminalPost = AreChildrenUsingTerminal; // return whether console configuration was skipped. - return childrenUsingTerminalPre > 0 && childrenUsingTerminalPost == 0 && configureConsole == 0 ? 1 : 0; + return childrenUsingTerminalPre && !childrenUsingTerminalPost && configureConsole == 0 ? 1 : 0; } finally { @@ -1056,23 +1056,6 @@ private static int OnSigChild(int reapAll, int configureConsole) } } - [UnmanagedCallersOnly] - private static void DelayedSigChildConsoleConfiguration() - { - DelayedSigChildConsoleConfigurationInner(); - } - - static partial void DelayedSigChildConsoleConfigurationInner(); - - /// - /// This method is called when the number of child processes that are using the terminal changes. - /// It updates the terminal configuration if necessary. - /// - internal static void ConfigureTerminalForChildProcesses(int increment, bool configureConsole = true) - { - ConfigureTerminalForChildProcessesInner(increment, configureConsole); - } - - static partial void ConfigureTerminalForChildProcessesInner(int increment, bool configureConsole); + static unsafe partial void SetDelayedSigChildConsoleConfigurationHandler(); } } From a7606124405ccb4f74cb00e4be4feb4ca520aa6a Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 21 Jun 2021 17:36:32 +0200 Subject: [PATCH 30/65] Fix iOS compilation failure --- .../Process.ConfigureTerminalForChildProcesses.iOS.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.ConfigureTerminalForChildProcesses.iOS.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.ConfigureTerminalForChildProcesses.iOS.cs index d89116504da7d..f7220aa465fe3 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.ConfigureTerminalForChildProcesses.iOS.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.ConfigureTerminalForChildProcesses.iOS.cs @@ -12,9 +12,7 @@ public partial class Process /// It updates the terminal configuration if necessary. /// internal static void ConfigureTerminalForChildProcesses(int increment, bool configureConsole = true) - { - - } + { } private static bool AreChildrenUsingTerminal => false; } From dddc5bcd47f9fe43025339e03fe54186b83acf9d Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 21 Jun 2021 20:06:30 +0200 Subject: [PATCH 31/65] Skip signals that cause xunit to exit with non zero --- .../PosixSignalRegistrationTests.Unix.cs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs index db51359ff4bff..bd2047225b577 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs @@ -49,6 +49,12 @@ public void CanRegisterForKnownValues(PosixSignal signal) [MemberData(nameof(PosixSignalValues))] public void SignalHandlerCalledForKnownSignals(PosixSignal signal) { + if (TriggersConsoleCancelKeyPress(signal)) + { + // skip: xunit test runner returns non zero exit code. + return; + } + using SemaphoreSlim semaphore = new(0); using var _ = PosixSignalRegistration.Create(signal, ctx => { @@ -68,6 +74,12 @@ public void SignalHandlerCalledForKnownSignals(PosixSignal signal) [MemberData(nameof(PosixSignalAsRawValues))] public void SignalHandlerCalledForRawSignals(PosixSignal signal) { + if (TriggersConsoleCancelKeyPress(signal)) + { + // skip: xunit test runner returns non zero exit code. + return; + } + using SemaphoreSlim semaphore = new(0); using var _ = PosixSignalRegistration.Create(signal, ctx => { @@ -218,5 +230,13 @@ private static void kill(PosixSignal sig) [DllImport(Interop.Libraries.SystemNative, EntryPoint = "SystemNative_GetPlatformSignalNumber")] [SuppressGCTransition] private static extern int GetPlatformSignalNumber(PosixSignal signal); + + private static bool TriggersConsoleCancelKeyPress(PosixSignal signal) + { + return (signal == PosixSignal.SIGINT || + signal == PosixSignal.SIGQUIT || + signal == (PosixSignal)2 || // SIGINT raw + signal == (PosixSignal)3); // SIGQUIT raw + } } } From d4c8e2443a775dac67d1530b40d80213758631a3 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 21 Jun 2021 21:06:47 +0200 Subject: [PATCH 32/65] Try fix Android compilation --- src/libraries/Native/Unix/System.Native/pal_signal.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index 479c586f0d8df..de43f3d572a6d 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -176,7 +176,10 @@ static void SignalHandler(int sig, siginfo_t* siginfo, void* context) sig != SIGINT) { struct sigaction* origHandler = OrigActionFor(sig); +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wsign-conversion" // sa_flags is unsigned on Android. if (origHandler->sa_flags & SA_SIGINFO) +#pragma clang diagnostic pop { assert(origHandler->sa_sigaction); origHandler->sa_sigaction(sig, siginfo, context); @@ -369,7 +372,10 @@ static bool InstallSignalHandler(int sig, int flags) struct sigaction newAction; memset(&newAction, 0, sizeof(struct sigaction)); +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wsign-conversion" // sa_flags is unsigned on Android. newAction.sa_flags = flags | SA_SIGINFO; +#pragma clang diagnostic pop sigemptyset(&newAction.sa_mask); newAction.sa_sigaction = &SignalHandler; From 12150aba9a61e91268d4bba4a07e0f18c4862d95 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 22 Jun 2021 09:22:49 +0200 Subject: [PATCH 33/65] Remove SIGSTOP from UninstallableSignalsThrow because it doesn't throw on OSX --- .../Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs index bd2047225b577..f9460dc587a4c 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs @@ -32,7 +32,6 @@ public void InvalidSignalValueThrows(int value) [Theory] [InlineData((PosixSignal)9)] // SIGKILL - [InlineData((PosixSignal)19)] // SIGSTOP public void UninstallableSignalsThrow(PosixSignal signal) { Assert.Throws(() => PosixSignalRegistration.Create(signal, ctx => { })); From 469b6fec88590d93932d27ce297bfaf74b77efa1 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 22 Jun 2021 14:51:10 +0200 Subject: [PATCH 34/65] Cleanup --- .../Native/Unix/System.Native/pal_signal.c | 28 +++++++++++++----- .../Native/Unix/System.Native/pal_signal.h | 29 +++++++++++++++++++ ...ConfigureTerminalForChildProcesses.Unix.cs | 2 +- ....ConfigureTerminalForChildProcesses.iOS.cs | 10 ++++--- .../src/System/Diagnostics/Process.Unix.cs | 2 -- .../PosixSignalRegistrationTests.Unix.cs | 4 +-- 6 files changed, 58 insertions(+), 17 deletions(-) diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index de43f3d572a6d..822232f955a3e 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -345,7 +345,6 @@ static void CloseSignalHandlingPipe() static bool InstallSignalHandler(int sig, int flags) { int rv; - (void)rv; // only used for assert struct sigaction* orig = OrigActionFor(sig); bool* isInstalled = &g_handlerIsInstalled[sig - 1]; @@ -392,14 +391,19 @@ void SystemNative_SetTerminalInvalidationHandler(TerminalInvalidationCallback ca { assert(callback != NULL); assert(g_terminalInvalidationCallback == NULL); + bool installed; + (void)installed; // only used for assert pthread_mutex_lock(&lock); { g_terminalInvalidationCallback = callback; - InstallSignalHandler(SIGCONT, SA_RESTART); - InstallSignalHandler(SIGCHLD, SA_RESTART); - InstallSignalHandler(SIGWINCH, SA_RESTART); + installed = InstallSignalHandler(SIGCONT, SA_RESTART); + assert(installed); + installed = InstallSignalHandler(SIGCHLD, SA_RESTART); + assert(installed); + installed = InstallSignalHandler(SIGWINCH, SA_RESTART); + assert(installed); } pthread_mutex_unlock(&lock); } @@ -408,12 +412,15 @@ void SystemNative_RegisterForSigChld(SigChldCallback callback) { assert(callback != NULL); assert(g_sigChldCallback == NULL); + bool installed; + (void)installed; // only used for assert pthread_mutex_lock(&lock); { g_sigChldCallback = callback; - InstallSignalHandler(SIGCHLD, SA_RESTART); + installed = InstallSignalHandler(SIGCHLD, SA_RESTART); + assert(installed); } pthread_mutex_unlock(&lock); } @@ -520,7 +527,6 @@ int32_t SystemNative_EnablePosixSignalHandling(int signalCode) assert(signalCode > 0 && signalCode <= GetSignalMax()); bool installed; - pthread_mutex_lock(&lock); { installed = InstallSignalHandler(signalCode, SA_RESTART); @@ -554,6 +560,8 @@ void SystemNative_DisablePosixSignalHandling(int signalCode) void InstallTTOUHandlerForConsole(ConsoleSigTtouHandler handler) { + bool installed; + pthread_mutex_lock(&lock); { assert(g_consoleTtouHandler == NULL); @@ -567,13 +575,16 @@ void InstallTTOUHandlerForConsole(ConsoleSigTtouHandler handler) // on EINTR when the process is running in background and the terminal // configured with TOSTOP. RestoreSignalHandler(SIGTTOU); - InstallSignalHandler(SIGTTOU, (int)SA_RESETHAND); + installed = InstallSignalHandler(SIGTTOU, (int)SA_RESETHAND); + assert(installed); } pthread_mutex_unlock(&lock); } void UninstallTTOUHandlerForConsole(void) { + bool installed; + (void)installed; // only used for assert pthread_mutex_lock(&lock); { g_consoleTtouHandler = NULL; @@ -581,7 +592,8 @@ void UninstallTTOUHandlerForConsole(void) RestoreSignalHandler(SIGTTOU); if (g_hasPosixSignalRegistrations[SIGTTOU - 1]) { - InstallSignalHandler(SIGTTOU, SA_RESTART); + installed = InstallSignalHandler(SIGTTOU, SA_RESTART); + assert(installed); } } pthread_mutex_unlock(&lock); diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.h b/src/libraries/Native/Unix/System.Native/pal_signal.h index 95d05fd0f73bb..c40c5ff0f1ec9 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.h +++ b/src/libraries/Native/Unix/System.Native/pal_signal.h @@ -48,15 +48,44 @@ typedef enum typedef int32_t (*PosixSignalHandler)(int32_t signalCode, PosixSignal signal); +/** + * Hooks up the specified callback for handling PosixSignalRegistrations. + * + * Should only be called when a callback is not currently registered. + */ PALEXPORT void SystemNative_SetPosixSignalHandler(PosixSignalHandler signalHandler); + +/** + * Converts a PosixSignal value to the platform signal number. + * When the signal is out of range, the function returns zero. + */ PALEXPORT int32_t SystemNative_GetPlatformSignalNumber(PosixSignal signal); + +/** + * Enables calling the PosixSignalHandler for the specified signal. + */ PALEXPORT int32_t SystemNative_EnablePosixSignalHandling(int signalCode); + +/** + * Disables calling the PosixSignalHandler for the specified signal. + */ PALEXPORT void SystemNative_DisablePosixSignalHandling(int signalCode); + +/** + * Performs the default runtime action for a non-canceled PosixSignal. + */ PALEXPORT void SystemNative_HandleNonCanceledPosixSignal(int signalCode); typedef void (*ConsoleSigTtouHandler)(void); +/** + * Hooks up callback to be called from the signal handler directly on SIGTTOU. + */ void InstallTTOUHandlerForConsole(ConsoleSigTtouHandler handler); + +/** + * Uninstalls the SIGTTOU handler. + */ void UninstallTTOUHandlerForConsole(void); #ifndef HAS_CONSOLE_SIGNALS diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.ConfigureTerminalForChildProcesses.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.ConfigureTerminalForChildProcesses.Unix.cs index 2f3a5f7c8bb08..b51957fb881bf 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.ConfigureTerminalForChildProcesses.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.ConfigureTerminalForChildProcesses.Unix.cs @@ -35,7 +35,7 @@ internal static void ConfigureTerminalForChildProcesses(int increment, bool conf } } - static unsafe partial void SetDelayedSigChildConsoleConfigurationHandler() + private static unsafe void SetDelayedSigChildConsoleConfigurationHandler() { Interop.Sys.SetDelayedSigChildConsoleConfigurationHandler(&DelayedSigChildConsoleConfiguration); } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.ConfigureTerminalForChildProcesses.iOS.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.ConfigureTerminalForChildProcesses.iOS.cs index f7220aa465fe3..3d9f85f7a4301 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.ConfigureTerminalForChildProcesses.iOS.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.ConfigureTerminalForChildProcesses.iOS.cs @@ -7,13 +7,15 @@ namespace System.Diagnostics { public partial class Process { - /// - /// This method is called when the number of child processes that are using the terminal changes. - /// It updates the terminal configuration if necessary. - /// + /// These methods are used on other Unix systems to track how many children use the terminal, + /// and update the terminal configuration when necessary. + internal static void ConfigureTerminalForChildProcesses(int increment, bool configureConsole = true) { } + private static unsafe void SetDelayedSigChildConsoleConfigurationHandler() + { } + private static bool AreChildrenUsingTerminal => false; } } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index 63d98d75b1744..cce32e3760109 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -1055,7 +1055,5 @@ private static int OnSigChild(int reapAll, int configureConsole) s_processStartLock.ExitWriteLock(); } } - - static unsafe partial void SetDelayedSigChildConsoleConfigurationHandler(); } } diff --git a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs index f9460dc587a4c..f4b6fb69402b4 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs @@ -118,7 +118,7 @@ public void SignalHandlerWorksForSecondRegistration() } [Fact] - public void SignalHandlerCalledNotCalledWhenDisposed() + public void SignalHandlerNotCalledWhenDisposed() { PosixSignal signal = PosixSignal.SIGCONT; @@ -133,7 +133,7 @@ public void SignalHandlerCalledNotCalledWhenDisposed() } [Fact] - public void SignalHandlerCalledNotCalledWhenFinalized() + public void SignalHandlerNotCalledWhenFinalized() { PosixSignal signal = PosixSignal.SIGCONT; From 38a4d76c2993ed948d778120eebccbc1adaaa017 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 22 Jun 2021 16:03:09 +0200 Subject: [PATCH 35/65] Handle case where all handlers got disposed by the time we want to call them. --- .../Unix/System.Native/Interop.PosixSignal.cs | 2 +- .../Native/Unix/System.Native/pal_signal.c | 25 ++++- .../Native/Unix/System.Native/pal_signal.h | 2 +- .../PosixSignalRegistration.Unix.cs | 105 +++++++++++------- 4 files changed, 91 insertions(+), 43 deletions(-) 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 65e3ca35f4844..b42b5138ec622 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 void HandleNonCanceledPosixSignal(int signal); + internal static extern bool HandleNonCanceledPosixSignal(int signal, int handlersDisposed); [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 822232f955a3e..87e0f596e743e 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -203,7 +203,7 @@ static void SignalHandler(int sig, siginfo_t* siginfo, void* context) } } -void SystemNative_HandleNonCanceledPosixSignal(int signalCode) +int32_t SystemNative_HandleNonCanceledPosixSignal(int32_t signalCode, int32_t handlersDisposed) { // Performs action the runtime performs on signal that is cancelable // using the PosixSignal API. @@ -236,6 +236,27 @@ void SystemNative_HandleNonCanceledPosixSignal(int signalCode) ReinitializeTerminal(); #endif } + else if (handlersDisposed) + { + // All PosixSignalRegistrations got Disposed by the time + // we wanted to call their handlers. + bool resendSignal; + pthread_mutex_lock(&lock); + { + if (g_hasPosixSignalRegistrations[signalCode - 1]) + { + // New handlers got registered. + return 0; + } + // Disposition changed back to the default. + resendSignal = !g_handlerIsInstalled[signalCode - 1]; + } + if (resendSignal) + { + kill(getpid(), signalCode); + } + } + return 1; } // Entrypoint for the thread that handles signals where our handling @@ -327,7 +348,7 @@ static void* SignalHandlerLoop(void* arg) if (!usePosixSignalHandler) { - SystemNative_HandleNonCanceledPosixSignal(signalCode); + SystemNative_HandleNonCanceledPosixSignal(signalCode, 0); } } } diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.h b/src/libraries/Native/Unix/System.Native/pal_signal.h index c40c5ff0f1ec9..7bbd279a66d4a 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.h +++ b/src/libraries/Native/Unix/System.Native/pal_signal.h @@ -74,7 +74,7 @@ PALEXPORT void SystemNative_DisablePosixSignalHandling(int signalCode); /** * Performs the default runtime action for a non-canceled PosixSignal. */ -PALEXPORT void SystemNative_HandleNonCanceledPosixSignal(int signalCode); +PALEXPORT int32_t SystemNative_HandleNonCanceledPosixSignal(int32_t signalCode, int32_t handlersDisposed); typedef void (*ConsoleSigTtouHandler)(void); diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs index 8bf1ddf7b6d46..e7bb2b08ef455 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs @@ -73,19 +73,53 @@ private unsafe void Register() _registered = true; } - private void Handle(PosixSignalContext context) + private bool CallHandler(PosixSignalContext context) { lock (_gate) { if (_registered) { _handler(context); + return true; } + return false; } } [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; + if (useDedicatedThread) + { + Thread handlerThread = new Thread(HandleSignal) + { + IsBackground = true, + Name = ".NET Signal Handler" + }; + handlerThread.UnsafeStart((signo, registrations)); + } + else + { + ThreadPool.UnsafeQueueUserWorkItem(HandleSignal, (signo, registrations)); + } + return 1; + } + return 0; + } + + private static PosixSignalRegistration?[]? GetRegistrations(int signo) { lock (s_registrations) { @@ -111,34 +145,16 @@ private static int OnPosixSignal(int signo, PosixSignal signal) } if (hasRegistrations) { - // 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; - if (useDedicatedThread) - { - Thread handlerThread = new Thread(HandleSignal) - { - IsBackground = true, - Name = ".NET Signal Handler" - }; - handlerThread.UnsafeStart((signo, registrations)); - } - else - { - ThreadPool.UnsafeQueueUserWorkItem(HandleSignal, (signo, registrations)); - } - return 1; + return registrations; + } + else + { + Interop.Sys.DisablePosixSignalHandling(signo); } } } + return null; } - return 0; } private static void HandleSignal(object? state) @@ -146,24 +162,35 @@ private static void HandleSignal(object? state) HandleSignal(((int, PosixSignalRegistration?[]))state!); } - private static void HandleSignal((int signo, PosixSignalRegistration?[] registrations) state) + private static void HandleSignal((int signo, PosixSignalRegistration?[]? registrations) state) { - PosixSignalContext ctx = new(); - foreach (PosixSignalRegistration? registration in state.registrations) + do { - if (registration != null) + bool handlersCalled = false; + if (state.registrations != null) + { + PosixSignalContext ctx = new(); + 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 (Interop.Sys.HandleNonCanceledPosixSignal(state.signo, handlersCalled ? 0 : 1)) { - // Different values for PosixSignal map to the same signo. - // Match the PosixSignal value used when registering. - ctx.Signal = registration._signal; - registration.Handle(ctx); + break; } - } - - if (!ctx.Cancel) - { - Interop.Sys.HandleNonCanceledPosixSignal(state.signo); - } + // HandleNonCanceledPosixSignal returns false when handlers got registered. + state.registrations = GetRegistrations(state.signo); + } while (true); } ~PosixSignalRegistration() From 8841d41343c4d9c3859c416d1316c9b49ed967c3 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 22 Jun 2021 20:21:37 +0200 Subject: [PATCH 36/65] GenerateReferenceAssemblySource --- .../ref/System.Runtime.InteropServices.cs | 35 ++++++++++--------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs b/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs index adf7158df07d7..8be17302389c1 100644 --- a/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs +++ b/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs @@ -798,28 +798,29 @@ public OptionalAttribute() { } } public enum PosixSignal { - SIGHUP = -1, - SIGINT = -2, - SIGQUIT = -3, - SIGTERM = -4, - SIGCHLD = -5, - SIGCONT = -6, - SIGWINCH = -7, - SIGTTIN = -8, + SIGTSTP = -10, SIGTTOU = -9, - SIGTSTP = -10 + SIGTTIN = -8, + SIGWINCH = -7, + SIGCONT = -6, + SIGCHLD = -5, + SIGTERM = -4, + SIGQUIT = -3, + SIGINT = -2, + SIGHUP = -1, } - public sealed class PosixSignalContext + public sealed partial class PosixSignalContext { - public PosixSignal Signal { get { throw null; } } - public bool Cancel { get { throw null; } set { throw null; } } - public PosixSignalContext(PosixSignal signal) { } + public PosixSignalContext(System.Runtime.InteropServices.PosixSignal signal) { } + public bool Cancel { get { throw null; } set { } } + public System.Runtime.InteropServices.PosixSignal Signal { get { throw null; } } } - public sealed class PosixSignalRegistration : IDisposable + public sealed partial class PosixSignalRegistration : System.IDisposable { - private PosixSignalRegistration() { throw null; } - public static PosixSignalRegistration Create(PosixSignal signal, Action handler) { throw null; } - public void Dispose() { throw null; } + internal PosixSignalRegistration() { } + public static System.Runtime.InteropServices.PosixSignalRegistration Create(System.Runtime.InteropServices.PosixSignal signal, System.Action handler) { throw null; } + public void Dispose() { } + ~PosixSignalRegistration() { } } [System.AttributeUsageAttribute(System.AttributeTargets.Method, Inherited=false)] public sealed partial class PreserveSigAttribute : System.Attribute From c2ef9ebac88771139fdaf79dcb124206ce95d387 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 22 Jun 2021 20:26:42 +0200 Subject: [PATCH 37/65] Replace PosixSignalRegistration.Unsupported with .Windows/.Browser file --- .../src/System.Runtime.InteropServices.csproj | 7 +++++-- ...d.cs => PosixSignalRegistration.Browser.cs} | 0 .../PosixSignalRegistration.Windows.cs | 18 ++++++++++++++++++ 3 files changed, 23 insertions(+), 2 deletions(-) rename src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/{PosixSignalRegistration.Unsupported.cs => PosixSignalRegistration.Browser.cs} (100%) create mode 100644 src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs diff --git a/src/libraries/System.Runtime.InteropServices/src/System.Runtime.InteropServices.csproj b/src/libraries/System.Runtime.InteropServices/src/System.Runtime.InteropServices.csproj index 78d6c70d9a08f..c6ba2fbe496fd 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System.Runtime.InteropServices.csproj +++ b/src/libraries/System.Runtime.InteropServices/src/System.Runtime.InteropServices.csproj @@ -63,8 +63,11 @@ - - + + + + + diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unsupported.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Browser.cs similarity index 100% rename from src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unsupported.cs rename to src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Browser.cs diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs new file mode 100644 index 0000000000000..7c88a92cae944 --- /dev/null +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs @@ -0,0 +1,18 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; + +namespace System.Runtime.InteropServices +{ + public sealed class PosixSignalRegistration : IDisposable + { + private PosixSignalRegistration() { } + + public static PosixSignalRegistration Create(PosixSignal signal, Action handler) + => throw new PlatformNotSupportedException(); + + public void Dispose() + => throw new PlatformNotSupportedException(); + } +} From 575dd7dce3d5caf2d53f24a3967dd181a5785c32 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 23 Jun 2021 09:30:43 +0200 Subject: [PATCH 38/65] Remove assert(g_hasTty) from UninitializeTerminal --- src/libraries/Native/Unix/System.Native/pal_console.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libraries/Native/Unix/System.Native/pal_console.c b/src/libraries/Native/Unix/System.Native/pal_console.c index 9a69cc3881ffb..1fd55aa3d0430 100644 --- a/src/libraries/Native/Unix/System.Native/pal_console.c +++ b/src/libraries/Native/Unix/System.Native/pal_console.c @@ -192,8 +192,6 @@ static bool ConfigureTerminal(bool signalForBreak, bool forChild, uint8_t minCha void UninitializeTerminal() { - assert(g_hasTty); - // This method is called on SIGQUIT/SIGINT from the signal dispatching thread // and on atexit. From 3756ac2c8dc3755aeb88de0a180e48e8ac837f51 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 23 Jun 2021 12:06:32 +0200 Subject: [PATCH 39/65] Initialize signal handling when ifndef HAS_CONSOLE_SIGNALS --- .../Native/Unix/System.Native/pal_signal.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index 87e0f596e743e..50ec0d2ee7502 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -624,8 +624,19 @@ void UninstallTTOUHandlerForConsole(void) int32_t SystemNative_InitializeTerminalAndSignalHandling() { - errno = ENOSYS; - return 0; + static int32_t initialized = 0; + + // The Process, Console and PosixSignalRegistration classes call this method for initialization. + if (pthread_mutex_lock(&lock) == 0) + { + if (initialized == 0) + { + initialized = InitializeSignalHandlingCore(); + } + pthread_mutex_unlock(&lock); + } + + return initialized; } #endif From 778350f15d93de5a426b22c0c5a2400f0b3a73d6 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 23 Jun 2021 14:50:07 +0200 Subject: [PATCH 40/65] Fix broken Cancel --- .../PosixSignalRegistration.Unix.cs | 6 +- .../PosixSignalRegistrationTests.Unix.cs | 78 ++++++++----------- 2 files changed, 38 insertions(+), 46 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs index e7bb2b08ef455..3e14e54c307eb 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs @@ -183,10 +183,14 @@ private static void HandleSignal((int signo, PosixSignalRegistration?[]? registr } } } + if (ctx.Cancel) + { + return; + } } if (Interop.Sys.HandleNonCanceledPosixSignal(state.signo, handlersCalled ? 0 : 1)) { - break; + return; } // HandleNonCanceledPosixSignal returns false when handlers got registered. state.registrations = GetRegistrations(state.signo); diff --git a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs index f4b6fb69402b4..cc8fc89481667 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs @@ -46,52 +46,46 @@ public void CanRegisterForKnownValues(PosixSignal signal) [Theory] [MemberData(nameof(PosixSignalValues))] - public void SignalHandlerCalledForKnownSignals(PosixSignal signal) + public void SignalHandlerCalledForKnownSignals(PosixSignal s) { - if (TriggersConsoleCancelKeyPress(signal)) - { - // skip: xunit test runner returns non zero exit code. - return; - } - - using SemaphoreSlim semaphore = new(0); - using var _ = PosixSignalRegistration.Create(signal, ctx => - { - Assert.Equal(signal, ctx.Signal); + RemoteExecutor.Invoke((signalStr) => { + PosixSignal signal = Enum.Parse(signalStr); + using SemaphoreSlim semaphore = new(0); + using var _ = PosixSignalRegistration.Create(signal, ctx => + { + Assert.Equal(signal, ctx.Signal); - // Ensure signal doesn't cause the process to terminate. - ctx.Cancel = true; + // Ensure signal doesn't cause the process to terminate. + ctx.Cancel = true; - semaphore.Release(); - }); - kill(signal); - bool entered = semaphore.Wait(Timeout); - Assert.True(entered); + semaphore.Release(); + }); + kill(signal); + bool entered = semaphore.Wait(Timeout); + Assert.True(entered); + }, s.ToString()).Dispose(); } [Theory] [MemberData(nameof(PosixSignalAsRawValues))] - public void SignalHandlerCalledForRawSignals(PosixSignal signal) + public void SignalHandlerCalledForRawSignals(PosixSignal s) { - if (TriggersConsoleCancelKeyPress(signal)) - { - // skip: xunit test runner returns non zero exit code. - return; - } - - using SemaphoreSlim semaphore = new(0); - using var _ = PosixSignalRegistration.Create(signal, ctx => - { - Assert.Equal(signal, ctx.Signal); + RemoteExecutor.Invoke((signalStr) => { + PosixSignal signal = Enum.Parse(signalStr); + using SemaphoreSlim semaphore = new(0); + using var _ = PosixSignalRegistration.Create(signal, ctx => + { + Assert.Equal(signal, ctx.Signal); - // Ensure signal doesn't cause the process to terminate. - ctx.Cancel = true; + // Ensure signal doesn't cause the process to terminate. + ctx.Cancel = true; - semaphore.Release(); - }); - kill(signal); - bool entered = semaphore.Wait(Timeout); - Assert.True(entered); + semaphore.Release(); + }); + kill(signal); + bool entered = semaphore.Wait(Timeout); + Assert.True(entered); + }, s.ToString()).Dispose(); } [Fact] @@ -160,7 +154,7 @@ void CreateDanglingRegistration() [InlineData(PosixSignal.SIGTERM, true, 0)] [InlineData(PosixSignal.SIGTERM, false, 143)] [InlineData(PosixSignal.SIGQUIT, true, 0)] - [InlineData(PosixSignal.SIGQUIT, false, 999)] // TODO: 131 + [InlineData(PosixSignal.SIGQUIT, false, 131)] public void SignalCanCancelTermination(PosixSignal signal, bool cancel, int expectedExitCode) { RemoteExecutor.Invoke((signalStr, cancelStr) => @@ -183,6 +177,8 @@ public void SignalCanCancelTermination(PosixSignal signal, bool cancel, int expe // Give the default signal handler a chance to run. Thread.Sleep(cancelArg ? TimeSpan.FromSeconds(1) : Timeout); + + return 0; }, signal.ToString(), cancel.ToString(), new RemoteInvokeOptions() { ExpectedExitCode = expectedExitCode }).Dispose(); } @@ -229,13 +225,5 @@ private static void kill(PosixSignal sig) [DllImport(Interop.Libraries.SystemNative, EntryPoint = "SystemNative_GetPlatformSignalNumber")] [SuppressGCTransition] private static extern int GetPlatformSignalNumber(PosixSignal signal); - - private static bool TriggersConsoleCancelKeyPress(PosixSignal signal) - { - return (signal == PosixSignal.SIGINT || - signal == PosixSignal.SIGQUIT || - signal == (PosixSignal)2 || // SIGINT raw - signal == (PosixSignal)3); // SIGQUIT raw - } } } From ed39929a846a552746c47107d34278c0182fe926 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 23 Jun 2021 15:51:29 +0200 Subject: [PATCH 41/65] Use SIGRTMAX as highest usable signal number. --- src/libraries/Native/Unix/System.Native/pal_signal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index 50ec0d2ee7502..336a0f38748df 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -40,7 +40,7 @@ static int g_signalPipe[2] = {-1, -1}; // Pipe used between signal handler and w static int GetSignalMax() // Returns the highest usable signal number. { - return NSIG; + return SIGRTMAX; } static bool TryConvertSignalCodeToPosixSignal(int signalCode, PosixSignal* posixSignal) From 3c028350ba032135f10288ba80af2cd251d1941c Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 23 Jun 2021 16:05:20 +0200 Subject: [PATCH 42/65] SIGRTMAX is not defined on all platforms, fall back to NSIG --- src/libraries/Native/Unix/System.Native/pal_signal.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index 336a0f38748df..3029c3fd48655 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -40,7 +40,11 @@ static int g_signalPipe[2] = {-1, -1}; // Pipe used between signal handler and w static int GetSignalMax() // Returns the highest usable signal number. { +#ifdef SIGRTMAX return SIGRTMAX; +#else + return NSIG; +#endif } static bool TryConvertSignalCodeToPosixSignal(int signalCode, PosixSignal* posixSignal) From 8ccfd3e073462ae6d4733be7856ae31cde84d3d0 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 23 Jun 2021 21:49:41 +0200 Subject: [PATCH 43/65] Resend signal only when restored to SIG_DFL --- src/libraries/Native/Unix/System.Native/pal_signal.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index 3029c3fd48655..fc03f88865726 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -252,8 +252,9 @@ int32_t SystemNative_HandleNonCanceledPosixSignal(int32_t signalCode, int32_t ha // New handlers got registered. return 0; } - // Disposition changed back to the default. - resendSignal = !g_handlerIsInstalled[signalCode - 1]; + // Disposition changed back to SIG_DFL. + resendSignal = !g_handlerIsInstalled[signalCode - 1] && + OrigActionFor(signalCode)->sa_handler == SIG_DFL; } if (resendSignal) { From 481f40efad514a4b360268edfe28b3126c251f48 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 23 Jun 2021 21:51:57 +0200 Subject: [PATCH 44/65] SignalCanCancelTermination test: increase timeout --- .../InteropServices/PosixSignalRegistrationTests.Unix.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs index cc8fc89481667..2d50163cc85a0 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs @@ -176,7 +176,7 @@ public void SignalCanCancelTermination(PosixSignal signal, bool cancel, int expe Assert.True(entered); // Give the default signal handler a chance to run. - Thread.Sleep(cancelArg ? TimeSpan.FromSeconds(1) : Timeout); + Thread.Sleep(cancelArg ? TimeSpan.FromSeconds(30) : Timeout); return 0; }, signal.ToString(), cancel.ToString(), From 669e09faf7079811bd7882bb9fe57132de35a47d Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 24 Jun 2021 00:12:43 +0200 Subject: [PATCH 45/65] Increase timeout to 10min --- .../InteropServices/PosixSignalRegistrationTests.Unix.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs index 2d50163cc85a0..7766788a017e5 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs @@ -176,11 +176,11 @@ public void SignalCanCancelTermination(PosixSignal signal, bool cancel, int expe Assert.True(entered); // Give the default signal handler a chance to run. - Thread.Sleep(cancelArg ? TimeSpan.FromSeconds(30) : Timeout); + Thread.Sleep(cancelArg ? TimeSpan.FromSeconds(1) : TimeSpan.FromMinutes(10)); return 0; }, signal.ToString(), cancel.ToString(), - new RemoteInvokeOptions() { ExpectedExitCode = expectedExitCode }).Dispose(); + new RemoteInvokeOptions() { ExpectedExitCode = expectedExitCode, TimeOut = 10 * 60 * 1000 }).Dispose(); } public static TheoryData PosixSignalValues From 2cb8cda1498e62bf09286b51d0582fabd5235642 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 24 Jun 2021 08:16:50 +0200 Subject: [PATCH 46/65] Update expected SIGQUIT exit code for mono --- .../PosixSignalRegistrationTests.Unix.cs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs index 7766788a017e5..d5721262ff9c8 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs @@ -157,10 +157,18 @@ void CreateDanglingRegistration() [InlineData(PosixSignal.SIGQUIT, false, 131)] public void SignalCanCancelTermination(PosixSignal signal, bool cancel, int expectedExitCode) { - RemoteExecutor.Invoke((signalStr, cancelStr) => + // Mono doesn't restore and call SIG_DFL on SIGQUIT. + bool isMono = Type.GetType("Mono.Runtime") != null; + if (isMono && signal == PosixSignal.SIGQUIT && cancel == false) + { + expectedExitCode = 0; + } + + RemoteExecutor.Invoke((signalStr, cancelStr, expectedStr) => { PosixSignal signalArg = Enum.Parse(signalStr); bool cancelArg = bool.Parse(cancelStr); + int expected = int.Parse(expectedStr); using SemaphoreSlim semaphore = new(0); using var _ = PosixSignalRegistration.Create(signalArg, ctx => @@ -176,10 +184,10 @@ public void SignalCanCancelTermination(PosixSignal signal, bool cancel, int expe Assert.True(entered); // Give the default signal handler a chance to run. - Thread.Sleep(cancelArg ? TimeSpan.FromSeconds(1) : TimeSpan.FromMinutes(10)); + Thread.Sleep(expected == 0 ? TimeSpan.FromSeconds(5) : TimeSpan.FromMinutes(10)); return 0; - }, signal.ToString(), cancel.ToString(), + }, signal.ToString(), cancel.ToString(), expectedExitCode.ToString(), new RemoteInvokeOptions() { ExpectedExitCode = expectedExitCode, TimeOut = 10 * 60 * 1000 }).Dispose(); } From 459f3e607f349d54bcf07e48b1b0df547dd50971 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 24 Jun 2021 09:38:52 +0200 Subject: [PATCH 47/65] Change isMono check --- .../InteropServices/PosixSignalRegistrationTests.Unix.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs index d5721262ff9c8..66854767bb02d 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs @@ -158,7 +158,7 @@ void CreateDanglingRegistration() public void SignalCanCancelTermination(PosixSignal signal, bool cancel, int expectedExitCode) { // Mono doesn't restore and call SIG_DFL on SIGQUIT. - bool isMono = Type.GetType("Mono.Runtime") != null; + bool isMono = typeof(object).Assembly.GetType("Mono.RuntimeStructs") != null; if (isMono && signal == PosixSignal.SIGQUIT && cancel == false) { expectedExitCode = 0; @@ -184,7 +184,7 @@ public void SignalCanCancelTermination(PosixSignal signal, bool cancel, int expe Assert.True(entered); // Give the default signal handler a chance to run. - Thread.Sleep(expected == 0 ? TimeSpan.FromSeconds(5) : TimeSpan.FromMinutes(10)); + Thread.Sleep(expected == 0 ? TimeSpan.FromSeconds(2) : TimeSpan.FromMinutes(10)); return 0; }, signal.ToString(), cancel.ToString(), expectedExitCode.ToString(), From 834350f508a946d8d673e32b74091c68d2b0c4a5 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 29 Jun 2021 08:14:14 +0200 Subject: [PATCH 48/65] Use PlatformDetection.IsMonoRuntime --- .../InteropServices/PosixSignalRegistrationTests.Unix.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs index 66854767bb02d..87d9853cb10e4 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs @@ -158,8 +158,7 @@ void CreateDanglingRegistration() public void SignalCanCancelTermination(PosixSignal signal, bool cancel, int expectedExitCode) { // Mono doesn't restore and call SIG_DFL on SIGQUIT. - bool isMono = typeof(object).Assembly.GetType("Mono.RuntimeStructs") != null; - if (isMono && signal == PosixSignal.SIGQUIT && cancel == false) + if (PlatformDetection.IsMonoRuntime && signal == PosixSignal.SIGQUIT && cancel == false) { expectedExitCode = 0; } From c8cb44907981aea07f10cbaa8c2d258dfacece48 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 29 Jun 2021 08:50:10 +0200 Subject: [PATCH 49/65] Add IsSignalThatTerminates method --- .../Native/Unix/System.Native/pal_signal.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index fc03f88865726..395aff2f5e39b 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -47,6 +47,14 @@ static int GetSignalMax() // Returns the highest usable signal number. #endif } +// For these signals, the default handler is expected to terminate the application. +static bool IsSignalThatTerminates(int signalCode) +{ + return signalCode == SIGINT || + signalCode == SIGQUIT || + signalCode == SIGTERM; +} + static bool TryConvertSignalCodeToPosixSignal(int signalCode, PosixSignal* posixSignal) { assert(posixSignal != NULL); @@ -175,9 +183,7 @@ static void SignalHandler(int sig, siginfo_t* siginfo, void* context) // Signals that can be canceled through the PosixSignal API are handled later. // For other signals, we immediately invoke the original handler. - if (sig != SIGQUIT && - sig != SIGTERM && - sig != SIGINT) + if (!IsSignalThatTerminates(sig)) { struct sigaction* origHandler = OrigActionFor(sig); #pragma clang diagnostic push @@ -211,10 +217,7 @@ int32_t SystemNative_HandleNonCanceledPosixSignal(int32_t signalCode, int32_t ha { // Performs action the runtime performs on signal that is cancelable // using the PosixSignal API. - - if (signalCode == SIGQUIT || - signalCode == SIGTERM || - signalCode == SIGINT) + if (IsSignalThatTerminates(signalCode)) { // Terminate the process using the original handler. #ifdef HAS_CONSOLE_SIGNALS From cba68f3215a798328e3e3abe2ee18da84dfd3494 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 29 Jun 2021 10:47:57 +0200 Subject: [PATCH 50/65] Add some documentation xml --- .../InteropServices/PosixSignalContext.cs | 3 ++ .../PosixSignalRegistration.Unix.cs | 34 +++++++++++++++---- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalContext.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalContext.cs index 080fa49b4b227..21e283dd54061 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalContext.cs +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalContext.cs @@ -11,6 +11,9 @@ public PosixSignal Signal internal set; } + /// + /// Cancels default handling of the signal. + /// public bool Cancel { get; diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs index 3e14e54c307eb..7575107fd2c6a 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs @@ -8,6 +8,9 @@ namespace System.Runtime.InteropServices { + /// + /// Handles a . + /// public sealed class PosixSignalRegistration : IDisposable { private readonly Action _handler; @@ -23,6 +26,22 @@ private PosixSignalRegistration(PosixSignal signal, int signo, Action + /// Registers a that is invoked when the occurs. + /// + /// The signal to register for. + /// The handler that gets invoked. + /// A instance that can be disposed to unregister. + /// is . + /// is out the range of expected values for the platform. + /// An error occurred while setting up the signal handling or while installing the handler for the specified signal. + /// + /// Raw values can be provided for by casting them to . + /// + /// Default handling of the signal can be canceled through . + /// For SIGTERM, SIGINT, and SIGQUIT process termination can be canceled. + /// For SIGCHLD, and SIGCONT terminal configuration can be canceled. + /// public static PosixSignalRegistration Create(PosixSignal signal, Action handler) { if (handler == null) @@ -39,6 +58,15 @@ public static PosixSignalRegistration Create(PosixSignal signal, Action + /// Unregister the handler. + /// + public void Dispose() + { + Dispose(true); + GC.SuppressFinalize(this); + } + private unsafe void Register() { if (!s_initialized) @@ -200,12 +228,6 @@ private static void HandleSignal((int signo, PosixSignalRegistration?[]? registr ~PosixSignalRegistration() => Dispose(false); - public void Dispose() - { - Dispose(true); - GC.SuppressFinalize(this); - } - private void Dispose(bool disposing) { if (_registered) From 19d30d67978557700d7a4845ad09d49ade8cc333 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 29 Jun 2021 14:32:06 +0200 Subject: [PATCH 51/65] Console.ManualTests: test terminal echo during and after child process used terminal. --- .../tests/ManualTests/ManualTests.cs | 28 +++++++++++++++++++ .../System.Console.Manual.Tests.csproj | 3 ++ 2 files changed, 31 insertions(+) diff --git a/src/libraries/System.Console/tests/ManualTests/ManualTests.cs b/src/libraries/System.Console/tests/ManualTests/ManualTests.cs index cf0ea897254d3..6c4b0195fc568 100644 --- a/src/libraries/System.Console/tests/ManualTests/ManualTests.cs +++ b/src/libraries/System.Console/tests/ManualTests/ManualTests.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Diagnostics; using System.Threading.Tasks; using System.IO; using Xunit; @@ -243,6 +244,33 @@ public static void CursorPositionAndArrowKeys() AssertUserExpectedResults("the arrow keys move around the screen as expected with no other bad artifacts"); } + [ConditionalFact(nameof(ManualTestsEnabled))] + [PlatformSpecific(TestPlatforms.AnyUnix)] // .NET echo handling is Unix specific. + public static void EchoWorksDuringAndAfterProcessThatUsesTerminal() + { + Console.WriteLine($"Please type \"test\" without the quotes and press Enter."); + string line = Console.ReadLine(); + Assert.Equal("test", line); + AssertUserExpectedResults("the characters you typed properly echoed as you typed"); + + Console.WriteLine($"Now type \"test\" without the quotes and press Ctrl+D twice."); + using Process p = Process.Start(new ProcessStartInfo + { + FileName = "cat", + RedirectStandardOutput = true, + }); + string stdout = p.StandardOutput.ReadToEnd(); + p.WaitForExit(); + Assert.Equal("test", stdout); + Console.WriteLine(); + AssertUserExpectedResults("the characters you typed properly echoed as you typed"); + + Console.WriteLine($"Please type \"test\" without the quotes and press Enter."); + line = Console.ReadLine(); + Assert.Equal("test", line); + AssertUserExpectedResults("the characters you typed properly echoed as you typed"); + } + [ConditionalFact(nameof(ManualTestsEnabled))] public static void EncodingTest() { diff --git a/src/libraries/System.Console/tests/ManualTests/System.Console.Manual.Tests.csproj b/src/libraries/System.Console/tests/ManualTests/System.Console.Manual.Tests.csproj index e1daf6a964ea9..ddd698d0d0200 100644 --- a/src/libraries/System.Console/tests/ManualTests/System.Console.Manual.Tests.csproj +++ b/src/libraries/System.Console/tests/ManualTests/System.Console.Manual.Tests.csproj @@ -6,4 +6,7 @@ + + + \ No newline at end of file From e4cef4728eba3d6fd21d601f9c976bbce67db204 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 30 Jun 2021 08:40:46 +0200 Subject: [PATCH 52/65] Maintain flags and mask of original handler --- .../Native/Unix/System.Native/pal_signal.c | 36 +++++++++++++++---- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index 395aff2f5e39b..284e95c1f16ff 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -47,6 +47,20 @@ static int GetSignalMax() // Returns the highest usable signal number. #endif } +static bool IsSigIgn(struct sigaction* action) +{ + assert(action); + return (action->sa_flags & SA_SIGINFO) == 0 && + action->sa_handler == SIG_IGN; +} + +static bool IsSigDfl(struct sigaction* action) +{ + assert(action); + return (action->sa_flags & SA_SIGINFO) == 0 && + action->sa_handler == SIG_DFL; +} + // For these signals, the default handler is expected to terminate the application. static bool IsSignalThatTerminates(int signalCode) { @@ -257,7 +271,7 @@ int32_t SystemNative_HandleNonCanceledPosixSignal(int32_t signalCode, int32_t ha } // Disposition changed back to SIG_DFL. resendSignal = !g_handlerIsInstalled[signalCode - 1] && - OrigActionFor(signalCode)->sa_handler == SIG_DFL; + IsSigDfl(OrigActionFor(signalCode)); } if (resendSignal) { @@ -311,7 +325,7 @@ static void* SignalHandlerLoop(void* arg) { // When the original disposition is SIG_IGN, children that terminated did not become zombies. // Since we overwrote the disposition, we have become responsible for reaping those processes. - bool reapAll = (void*)OrigActionFor(signalCode)->sa_sigaction == (void*)SIG_IGN; + bool reapAll = IsSigIgn(OrigActionFor(signalCode)); SigChldCallback callback = g_sigChldCallback; // double-checked locking @@ -392,19 +406,27 @@ static bool InstallSignalHandler(int sig, int flags) { return false; } - if ((void*)orig->sa_sigaction == (void*)SIG_IGN) + if (IsSigIgn(orig)) { *isInstalled = true; return true; } - struct sigaction newAction; - memset(&newAction, 0, sizeof(struct sigaction)); #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wsign-conversion" // sa_flags is unsigned on Android. - newAction.sa_flags = flags | SA_SIGINFO; + struct sigaction newAction; + if (!IsSigDfl(orig)) + { + // Maintain flags and mask of original handler. + newAction = *orig; + newAction.sa_flags = orig->sa_flags & ~(SA_RESTART | SA_RESETHAND); + } + else + { + memset(&newAction, 0, sizeof(struct sigaction)); + } + newAction.sa_flags |= flags | SA_SIGINFO; #pragma clang diagnostic pop - sigemptyset(&newAction.sa_mask); newAction.sa_sigaction = &SignalHandler; rv = sigaction(sig, &newAction, orig); From aa549a38eee266d6596cbc87063f7c0cec3eff4d Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 30 Jun 2021 09:37:22 +0200 Subject: [PATCH 53/65] PR feedback --- src/libraries/Native/Unix/System.Native/pal_signal.c | 8 +++++--- src/libraries/Native/Unix/System.Native/pal_signal.h | 1 + 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index 284e95c1f16ff..907639ad3f630 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -154,6 +154,9 @@ int32_t SystemNative_GetPlatformSignalNumber(PosixSignal signal) case PosixSignalSIGTSTP: return SIGTSTP; + + case PosixSignalInvalid: + break; } if (signal > 0 && signal <= GetSignalMax()) @@ -179,8 +182,7 @@ static struct sigaction* OrigActionFor(int sig) static void RestoreSignalHandler(int sig) { - bool* isInstalled = &g_handlerIsInstalled[sig - 1]; - *isInstalled = false; + g_handlerIsInstalled[sig - 1] = false; sigaction(sig, OrigActionFor(sig), NULL); } @@ -363,7 +365,7 @@ static void* SignalHandlerLoop(void* arg) PosixSignal signal; if (!TryConvertSignalCodeToPosixSignal(signalCode, &signal)) { - signal = (PosixSignal)0; + signal = PosixSignalInvalid; } usePosixSignalHandler = g_posixSignalHandler(signalCode, signal) != 0; } diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.h b/src/libraries/Native/Unix/System.Native/pal_signal.h index 7bbd279a66d4a..138f37835a8ca 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.h +++ b/src/libraries/Native/Unix/System.Native/pal_signal.h @@ -34,6 +34,7 @@ PALEXPORT void SystemNative_SetTerminalInvalidationHandler(TerminalInvalidationC typedef enum { + PosixSignalInvalid = 0, PosixSignalSIGHUP = -1, PosixSignalSIGINT = -2, PosixSignalSIGQUIT = -3, From 324db13ebc8c2028186fb606084cb5fdab67136d Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 30 Jun 2021 12:00:33 +0200 Subject: [PATCH 54/65] SystemNative_HandleNonCanceledPosixSignal: update handling based on default dispositions. --- .../Native/Unix/System.Native/pal_signal.c | 93 +++++++++---------- 1 file changed, 44 insertions(+), 49 deletions(-) diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index 907639ad3f630..7fc012bb4694f 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -61,14 +61,6 @@ static bool IsSigDfl(struct sigaction* action) action->sa_handler == SIG_DFL; } -// For these signals, the default handler is expected to terminate the application. -static bool IsSignalThatTerminates(int signalCode) -{ - return signalCode == SIGINT || - signalCode == SIGQUIT || - signalCode == SIGTERM; -} - static bool TryConvertSignalCodeToPosixSignal(int signalCode, PosixSignal* posixSignal) { assert(posixSignal != NULL); @@ -197,9 +189,13 @@ static void SignalHandler(int sig, siginfo_t* siginfo, void* context) } } - // Signals that can be canceled through the PosixSignal API are handled later. + // For these signals, the runtime original sa_sigaction/sa_handler will terminate the app. + // This termination can be canceled using the PosixSignal API. // For other signals, we immediately invoke the original handler. - if (!IsSignalThatTerminates(sig)) + bool isCancellableTeminationSignal = sig == SIGINT || + sig == SIGQUIT || + sig == SIGTERM; + if (!isCancellableTeminationSignal) { struct sigaction* origHandler = OrigActionFor(sig); #pragma clang diagnostic push @@ -231,54 +227,53 @@ static void SignalHandler(int sig, siginfo_t* siginfo, void* context) int32_t SystemNative_HandleNonCanceledPosixSignal(int32_t signalCode, int32_t handlersDisposed) { - // Performs action the runtime performs on signal that is cancelable - // using the PosixSignal API. - if (IsSignalThatTerminates(signalCode)) + switch (signalCode) { - // Terminate the process using the original handler. + case SIGCONT: + // Default disposition is Continue. #ifdef HAS_CONSOLE_SIGNALS - UninitializeTerminal(); + ReinitializeTerminal(); #endif - // Restore the original signal handler and invoke it. - RestoreSignalHandler(signalCode); - kill(getpid(), signalCode); - } - else if (signalCode == SIGCHLD) - { - if (g_sigChldConsoleConfigurationDelayed) - { - g_sigChldConsoleConfigurationDelayed = false; + break; + case SIGTSTP: + case SIGTTIN: + case SIGTTOU: + // Default disposition is Stop. + // no-op. + break; + case SIGCHLD: + // Default disposition is Ignore. + if (g_sigChldConsoleConfigurationDelayed) + { + g_sigChldConsoleConfigurationDelayed = false; - assert(g_sigChldConsoleConfigurationCallback); - g_sigChldConsoleConfigurationCallback(); - } - } - else if (signalCode == SIGCONT) - { -#ifdef HAS_CONSOLE_SIGNALS - ReinitializeTerminal(); -#endif - } - else if (handlersDisposed) - { - // All PosixSignalRegistrations got Disposed by the time - // we wanted to call their handlers. - bool resendSignal; - pthread_mutex_lock(&lock); - { - if (g_hasPosixSignalRegistrations[signalCode - 1]) + assert(g_sigChldConsoleConfigurationCallback); + g_sigChldConsoleConfigurationCallback(); + } + break; + case SIGURG: + case SIGWINCH: + // Default disposition is Ignore. + // no-op. + break; + default: + // Default disposition is Terminate. + if (handlersDisposed && g_hasPosixSignalRegistrations[signalCode - 1]) { // New handlers got registered. return 0; } - // Disposition changed back to SIG_DFL. - resendSignal = !g_handlerIsInstalled[signalCode - 1] && - IsSigDfl(OrigActionFor(signalCode)); - } - if (resendSignal) - { + // Restore and invoke the original handler. + pthread_mutex_lock(&lock); + { + RestoreSignalHandler(signalCode); + } + pthread_mutex_unlock(&lock); +#ifdef HAS_CONSOLE_SIGNALS + UninitializeTerminal(); +#endif kill(getpid(), signalCode); - } + break; } return 1; } From 8e830fefb5ac57320a1e7686520455655e482e7a Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 1 Jul 2021 09:57:47 +0200 Subject: [PATCH 55/65] Apply suggestions from code review Co-authored-by: Stephen Toub --- .../InteropServices/PosixSignalRegistration.Unix.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs index 7575107fd2c6a..bf85ce9ea1426 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs @@ -77,6 +77,7 @@ private unsafe void Register() // Microsoft.Win32.Primitives. Interop.CheckIo(-1); } + Interop.Sys.SetPosixSignalHandler(&OnPosixSignal); s_initialized = true; } @@ -87,6 +88,7 @@ private unsafe void Register() signalRegistrations = new List>(); s_registrations.Add(_signo, signalRegistrations); } + if (signalRegistrations.Count == 0) { if (!Interop.Sys.EnablePosixSignalHandling(_signo)) @@ -96,6 +98,7 @@ private unsafe void Register() Interop.CheckIo(-1); } } + signalRegistrations.Add(new WeakReference(this)); } _registered = true; @@ -211,15 +214,18 @@ private static void HandleSignal((int signo, PosixSignalRegistration?[]? registr } } } + if (ctx.Cancel) { return; } } + if (Interop.Sys.HandleNonCanceledPosixSignal(state.signo, handlersCalled ? 0 : 1)) { return; } + // HandleNonCanceledPosixSignal returns false when handlers got registered. state.registrations = GetRegistrations(state.signo); } while (true); @@ -252,6 +258,7 @@ private void Dispose(bool disposing) i--; } } + if (signalRegistrations.Count == 0) { Interop.Sys.DisablePosixSignalHandling(_signo); From 85439d6ff0741a6a749a413e403185c8d958b24c Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 1 Jul 2021 10:39:43 +0200 Subject: [PATCH 56/65] Don't call sa_sigaction/sa_handler twice from SystemNative_HandleNonCanceledPosixSignal. --- .../Native/Unix/System.Native/pal_signal.c | 17 +++++++++++++---- .../PosixSignalRegistration.Unix.cs | 2 +- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index 7fc012bb4694f..5d0df6ec5fe15 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -47,6 +47,13 @@ static int GetSignalMax() // Returns the highest usable signal number. #endif } +static bool IsCancelableTerminationSignal(int sig) +{ + return sig == SIGINT || + sig == SIGQUIT || + sig == SIGTERM; +} + static bool IsSigIgn(struct sigaction* action) { assert(action); @@ -192,10 +199,7 @@ static void SignalHandler(int sig, siginfo_t* siginfo, void* context) // For these signals, the runtime original sa_sigaction/sa_handler will terminate the app. // This termination can be canceled using the PosixSignal API. // For other signals, we immediately invoke the original handler. - bool isCancellableTeminationSignal = sig == SIGINT || - sig == SIGQUIT || - sig == SIGTERM; - if (!isCancellableTeminationSignal) + if (!IsCancelableTerminationSignal(sig)) { struct sigaction* origHandler = OrigActionFor(sig); #pragma clang diagnostic push @@ -258,6 +262,11 @@ int32_t SystemNative_HandleNonCanceledPosixSignal(int32_t signalCode, int32_t ha break; default: // Default disposition is Terminate. + if (!IsCancelableTerminationSignal(signalCode) && !IsSigDfl(OrigActionFor(signalCode))) + { + // We've already called the original handler in SignalHandler. + break; + } if (handlersDisposed && g_hasPosixSignalRegistrations[signalCode - 1]) { // New handlers got registered. diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs index bf85ce9ea1426..3d655e8e97435 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs @@ -77,7 +77,7 @@ private unsafe void Register() // Microsoft.Win32.Primitives. Interop.CheckIo(-1); } - + Interop.Sys.SetPosixSignalHandler(&OnPosixSignal); s_initialized = true; } From b8a0b5d4a9c9acf702aafbe5b66ed2f30a69a5e8 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 1 Jul 2021 11:18:22 +0200 Subject: [PATCH 57/65] Don't remove WeakReferences while iterating. --- .../PosixSignalRegistration.Unix.cs | 35 +++++++++++++------ 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs index 3d655e8e97435..000ba46483272 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs @@ -83,9 +83,9 @@ private unsafe void Register() } lock (s_registrations) { - if (!s_registrations.TryGetValue(_signo, out List>? signalRegistrations)) + if (!s_registrations.TryGetValue(_signo, out List?>? signalRegistrations)) { - signalRegistrations = new List>(); + signalRegistrations = new List?>(); s_registrations.Add(_signo, signalRegistrations); } @@ -154,15 +154,16 @@ private static int OnPosixSignal(int signo, PosixSignal signal) { lock (s_registrations) { - if (s_registrations.TryGetValue(signo, out List>? signalRegistrations)) + 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)) + if (signalRegistrations[i]!.TryGetTarget(out PosixSignalRegistration? registration)) { registrations[i] = registration; hasRegistrations = true; @@ -170,10 +171,16 @@ private static int OnPosixSignal(int signo, PosixSignal signal) else { // WeakReference no longer holds an object. PosixSignalRegistration got finalized. - signalRegistrations.RemoveAt(i); - i--; + signalRegistrations[i] = null; + pruneWeakReferences = true; } } + + if (pruneWeakReferences) + { + signalRegistrations.RemoveAll(item => item is null); + } + if (hasRegistrations) { return registrations; @@ -240,10 +247,11 @@ private void Dispose(bool disposing) { lock (s_registrations) { - List> signalRegistrations = s_registrations[_signo]; + List?> signalRegistrations = s_registrations[_signo]; + bool pruneWeakReferences = false; for (int i = 0; i < signalRegistrations.Count; i++) { - if (signalRegistrations[i].TryGetTarget(out PosixSignalRegistration? registration)) + if (signalRegistrations[i]!.TryGetTarget(out PosixSignalRegistration? registration)) { if (object.ReferenceEquals(this, registration)) { @@ -254,11 +262,16 @@ private void Dispose(bool disposing) else { // WeakReference no longer holds an object. PosixSignalRegistration got finalized. - signalRegistrations.RemoveAt(i); - i--; + signalRegistrations[i] = null; + pruneWeakReferences = true; } } + if (pruneWeakReferences) + { + signalRegistrations.RemoveAll(item => item is null); + } + if (signalRegistrations.Count == 0) { Interop.Sys.DisablePosixSignalHandling(_signo); @@ -274,6 +287,6 @@ private void Dispose(bool disposing) } private static volatile bool s_initialized; - private static readonly Dictionary>> s_registrations = new(); + private static readonly Dictionary?>> s_registrations = new(); } } From 18b7139cc2f6b0051b68f2f09b54235569b81619 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 1 Jul 2021 11:19:45 +0200 Subject: [PATCH 58/65] Move static fields to the top of the class --- .../Runtime/InteropServices/PosixSignalRegistration.Unix.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs index 000ba46483272..9a3144f5e8163 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs @@ -13,6 +13,9 @@ namespace System.Runtime.InteropServices /// public sealed class PosixSignalRegistration : IDisposable { + private static volatile bool s_initialized; + private static readonly Dictionary?>> s_registrations = new(); + private readonly Action _handler; private readonly PosixSignal _signal; private readonly int _signo; @@ -285,8 +288,5 @@ private void Dispose(bool disposing) } } } - - private static volatile bool s_initialized; - private static readonly Dictionary?>> s_registrations = new(); } } From d136c5b700eaefccbeb9b44983fdb7649c0b4610 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 1 Jul 2021 11:23:02 +0200 Subject: [PATCH 59/65] Remove -Wcast-qual ignore --- src/libraries/Native/Unix/System.Native/pal_signal.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index 5d0df6ec5fe15..390a640fdadc2 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -532,10 +532,7 @@ int32_t InitializeSignalHandlingCore() { free(g_origSigHandler); free(g_handlerIsInstalled); -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wcast-qual" - free((void*)g_hasPosixSignalRegistrations); -#pragma clang diagnostic pop + free((void*)(size_t)g_hasPosixSignalRegistrations); g_origSigHandler = NULL; g_handlerIsInstalled = NULL; g_hasPosixSignalRegistrations = NULL; From be93c17cb7dd380e11b56393893a7cb6ae1c3033 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 1 Jul 2021 11:32:56 +0200 Subject: [PATCH 60/65] PosixSignalContext: add XML docs --- .../InteropServices/PosixSignalContext.cs | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalContext.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalContext.cs index 21e283dd54061..2ed89948b5160 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalContext.cs +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalContext.cs @@ -3,8 +3,22 @@ namespace System.Runtime.InteropServices { + /// + /// Provides data for a event. + /// public sealed class PosixSignalContext { + /// + /// Initializes a new instance of the class. + /// + public PosixSignalContext(PosixSignal signal) + { + Signal = signal; + } + + /// + /// Gets the signal that occurred. + /// public PosixSignal Signal { get; @@ -12,7 +26,7 @@ public PosixSignal Signal } /// - /// Cancels default handling of the signal. + /// Gets or sets a value that indicates whether to cancel the default handling of the signal. The default is is . /// public bool Cancel { @@ -20,11 +34,6 @@ public bool Cancel set; } - public PosixSignalContext(PosixSignal signal) - { - Signal = signal; - } - internal PosixSignalContext() { } } From 4de318a8c4949d7c1211a60dbb318440e02783a6 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 1 Jul 2021 13:58:17 +0200 Subject: [PATCH 61/65] Fix Android compilation --- src/libraries/Native/Unix/System.Native/pal_signal.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index 390a640fdadc2..891627d9268ca 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -57,14 +57,14 @@ static bool IsCancelableTerminationSignal(int sig) static bool IsSigIgn(struct sigaction* action) { assert(action); - return (action->sa_flags & SA_SIGINFO) == 0 && + return (!(action->sa_flags & SA_SIGINFO)) && action->sa_handler == SIG_IGN; } static bool IsSigDfl(struct sigaction* action) { assert(action); - return (action->sa_flags & SA_SIGINFO) == 0 && + return (!(action->sa_flags & SA_SIGINFO)) && action->sa_handler == SIG_DFL; } From 68984025642164e40d1a1bddc178786790de8fbc Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 1 Jul 2021 15:34:10 +0200 Subject: [PATCH 62/65] Fix Android compilation, take II --- .../Native/Unix/System.Native/pal_signal.c | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index 891627d9268ca..1729a912bf8d7 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -54,18 +54,25 @@ static bool IsCancelableTerminationSignal(int sig) sig == SIGTERM; } +static bool IsSaSigInfo(struct sigaction* action) +{ + assert(action); +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wsign-conversion" // sa_flags is unsigned on Android. + return (action->sa_flags & SA_SIGINFO) != 0; +#pragma clang diagnostic pop +} + static bool IsSigIgn(struct sigaction* action) { assert(action); - return (!(action->sa_flags & SA_SIGINFO)) && - action->sa_handler == SIG_IGN; + return !IsSaSigInfo(action) && action->sa_handler == SIG_IGN; } static bool IsSigDfl(struct sigaction* action) { assert(action); - return (!(action->sa_flags & SA_SIGINFO)) && - action->sa_handler == SIG_DFL; + return !IsSaSigInfo(action) && action->sa_handler == SIG_DFL; } static bool TryConvertSignalCodeToPosixSignal(int signalCode, PosixSignal* posixSignal) @@ -202,10 +209,7 @@ static void SignalHandler(int sig, siginfo_t* siginfo, void* context) if (!IsCancelableTerminationSignal(sig)) { struct sigaction* origHandler = OrigActionFor(sig); -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wsign-conversion" // sa_flags is unsigned on Android. - if (origHandler->sa_flags & SA_SIGINFO) -#pragma clang diagnostic pop + if (IsSaSigInfo(origHandler)) { assert(origHandler->sa_sigaction); origHandler->sa_sigaction(sig, siginfo, context); From b8234bd888a276662a713a2076ae0ca6ef31774a Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 7 Jul 2021 09:10:22 +0200 Subject: [PATCH 63/65] Fix comment Co-authored-by: Stephen Toub --- .../src/System/Runtime/InteropServices/PosixSignalContext.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalContext.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalContext.cs index 2ed89948b5160..19ba4f9686849 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalContext.cs +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/PosixSignalContext.cs @@ -26,7 +26,7 @@ public PosixSignal Signal } /// - /// Gets or sets a value that indicates whether to cancel the default handling of the signal. The default is is . + /// Gets or sets a value that indicates whether to cancel the default handling of the signal. The default is . /// public bool Cancel { From f1aee19bf124f4a0e35eb500588596c37473921c Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 7 Jul 2021 10:01:05 +0200 Subject: [PATCH 64/65] Add PosixSignalContext constructor test --- ...ystem.Runtime.InteropServices.Tests.csproj | 1 + .../PosixSignalContextTests.cs | 22 +++++++++++++++++++ .../PosixSignalRegistrationTests.Unix.cs | 2 +- 3 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalContextTests.cs diff --git a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.Tests.csproj b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.Tests.csproj index a30422e68cbc4..4bcb75b0b810c 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.Tests.csproj +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.Tests.csproj @@ -150,6 +150,7 @@ + diff --git a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalContextTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalContextTests.cs new file mode 100644 index 0000000000000..3d8ca635ac20f --- /dev/null +++ b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalContextTests.cs @@ -0,0 +1,22 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Xunit; +using System.Runtime.InteropServices; + +namespace System.Tests +{ + public class PosixSignalContextTests + { + [Theory] + [InlineData(0)] + [InlineData(3)] + [InlineData(-1000)] + [InlineData(1000)] + public void Constructor(int value) + { + var ctx = new PosixSignalContext((PosixSignal)value); + Assert.Equal(value, (int)ctx.Signal); + } + } +} diff --git a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs index 87d9853cb10e4..b006af50150d3 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/PosixSignalRegistrationTests.Unix.cs @@ -11,7 +11,7 @@ namespace System.Tests { - public class PosixSignalTests + public class PosixSignalRegistrationTests { private static TimeSpan Timeout => TimeSpan.FromSeconds(30); From 005cddd3736aef0714877c03ae51cf0c92b96121 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 7 Jul 2021 10:33:29 +0200 Subject: [PATCH 65/65] SystemNative_HandleNonCanceledPosixSignal: add SIG_IGN case --- src/libraries/Native/Unix/System.Native/pal_signal.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index 1729a912bf8d7..4c4017f654746 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -271,6 +271,11 @@ int32_t SystemNative_HandleNonCanceledPosixSignal(int32_t signalCode, int32_t ha // We've already called the original handler in SignalHandler. break; } + if (IsSigIgn(OrigActionFor(signalCode))) + { + // Original handler doesn't do anything. + break; + } if (handlersDisposed && g_hasPosixSignalRegistrations[signalCode - 1]) { // New handlers got registered.