Make improvements to signal handling on .NET applications

[davidfowl]

Background and Motivation

Many systems use SIGTERM as a way to signal a graceful shutdown. To make this work well in container scenarios, we need to start the shutdown process and wait for some timeframe until it's done. To coordinate that, Microsoft.Extensions.Hosting sets up a bunch of events to allow the main thread to exit before continuing the process exit handler. Here's what that would look like (without the dependencies):

using System;
using System.Threading;

var waitForProcessShutdownStart = new ManualResetEventSlim();
var waitForMainExit = new ManualResetEventSlim();

AppDomain.CurrentDomain.ProcessExit += (sender, e) =>
{
    // We got a SIGTERM, signal that graceful shutdown has started
    waitForProcessShutdownStart.Set();

    Console.WriteLine("Waiting for main");
    // Don't unwind until main exists
    waitForMainExit.Wait();
};

Console.WriteLine("Waiting for shutdown");
// Wait for shutdown to start
waitForProcessShutdownStart.Wait();

// This is where the application performs graceful shutdown


// Now we're done with main, tell the shutdown handler
waitForMainExit.Set();

The above code shows how a user could set this up today. It's much more complex than the windows equivalent (handling Ctrl+C) for a couple of reasons:

• SIGTERM is the signal used to communicate starting a graceful shutdown, this means we need to stop the app from shutting down until some point.
• We want the main thread to continue executing for as long as it can until it has unwound. This lets us run any clean up code or logging before exit (see IHost.RunAsync() never completes #44086)
• It can result in deadlocks if someone calls Environment.Exit at the wrong point in the application.

This is what waiting for CTRL+C looks like:

var waitForProcessShutdownStart = new ManualResetEventSlim();
        
Console.CancelKeyPress += (sender, e) =>
{
    e.Cancel = true;

    waitForProcessShutdownStart.Set();
};

Console.WriteLine("Waiting for shutdown");

// Wait for shutdown to start
waitForProcessShutdownStart.Wait();

// Do graceful shutdown here

The runtime itself handles e.Cancel = true and doesn't shut down the process after the event handler runs. Instead the application can use this to coordinate letting the application gracefully unwind from the main thread.

The request here is to treat SIGTERM like Ctrl+C and support e.Cancel.

cc @janvorli @stephentoub @kouvel

Proposed API

namespace System.Runtime.InteropServices
{
+    public enum Signal // This may need more unique name
+    {
+        SIGHUP = 1,
+        SIGINT = 2,
+        SIGQUIT = 3,
+        SIGTERM = 15,
+    }

+    public class SignalContext
+    {
+        public Signal Signal { get; }
+        public bool Cancel { get; set; }
+    }

+    public struct SignalHandlerRegistration : IDisposable
+    {
+        public static SignalHandlerRegistration Create(Signal signal, Action<SignalContext> handler);
+    }
}

NOTES:

• SignalContext.Signal provides the signals that fired to cause the event to trigger so that can be checked in the callback to take action if the same handler was used for different registrations.
• SignalContext.Cancel cancels default processing.

We will map windows behavior to linux signal names:

• CTRL_C_EVENT = SIGINT
• CTRL_BREAK_EVENT = SIGINT
• CTRL_SHUTDOWN_EVENT - SIGTERM
• CTRL_CLOSE_EVENT - SIGTERM
• CTRL_LOGOFF_EVENT - Not mapped

Usage Examples

using var reg = SignalHandlerRegistration.Register(Signal.SIGINT, context => 
{
     context.Cancel = true;

     // Start graceful shutdown
});

Waiting synchronously on shutdown to start:

var waitForProcessShutdownStart = new ManualResetEventSlim();
        
using var reg = SignalHandlerRegistration.Register(Signal.SIGINT, context => 
{
    context.Cancel = true;

    waitForProcessShutdownStart.Set();
});

Console.WriteLine("Waiting for shutdown");

// Wait for shutdown to start
waitForProcessShutdownStart.Wait();

The hosting model in Microsoft.Extensions.Hosting will look like this in .NET 6:

public class ConsoleLifetime : IHostLifetime, IDisposable
{
    ...

    private IHostApplicationLifetime ApplicationLifetime { get; }

    private SignalHandlerRegistration _sigterm;
    private SignalHandlerRegistration _sigInt;

    public Task WaitForStartAsync(CancellationToken cancellationToken)
    {
        Action<SignalContext> handler = OnShutdownSignal;

        _sigInt = SignalHandlerRegistration.Register(Signal.SIGINT, handler);
        _sigterm = SignalHandlerRegistration.Register(Signal.SIGTERM, handler);

        // Console applications start immediately.
        return Task.CompletedTask;
    }

    private void OnShutdownSignal(SignalContext context)
    {
        context.Cancel = true;

        ApplicationLifetime.StopApplication();
    }

    public void Dispose()
    {
        _sigterm.Dispose();
        _sigInt.Dispose();
    }

    ...
}

Alternative Designs

Don't add new API but overload the existing Console.CancelKeyPress. This would cover one specific scenario but wouldn't handle the arbitrary signals.

Risks

None

[jkotas]

Change that introduced the current behavior: dotnet/coreclr#4309

I am not sure whether it is a good idea to overload the existing ProcessExit event to do this. There is a lot of code out there that uses this event for last minute cleanup during shutdown. It is not very compatible with everything running gracefully for a while until the actual shutdown happens.

[davidfowl]

I'm fine with introducing a new event if that's what it takes, I just want the problem to be solved by the runtime itself instead of the libraries (which does a poor job at it).

[davidfowl]

OK let's design an API. Today we have AppDomain.ProcessExit, this is the one that fires when it's too late. We also have Console.CancelKeyPress that means Ctrl+C. I'd like an API that handles CTRL+C or SIGINT or SIGTERM, that triggers an event, but does not end the process.

namespace System
{
    public static class Environment
    {
+        public static Task WaitForShutdownSignalAsync();
+        public static void WaitForShutdownSignal();
    }
}

Usage

using System;

// Blocking wait for shutdown to start
Environment.WaitForShutdownSignal();

// This is where the application performs graceful shutdown

Non-blocking wait

using System;

// Non-Blocking wait for shutdown to start
await Environment.WaitForShutdownSignalAsync();

// This is where the application performs graceful shutdown

I know this has the potential to get into the generic signal handling discussion but I'd like us to focus on the main requirements:

• Applications want to write the same code on across all platforms to wait on a shutdown signal
• These signals DO NOT shut the application down. They are just signals that give the application a chance to unwind and cleanup.

[jkotas]

• It should not be on Process. Process is for working with other processes, not your own process. I would put it on Environment.
• Does it need to take argument that describes the kind of signals you want to wait for and/or the kind of signal that was received? Does it need to give you an option to resume the default processing of the signal? Ideally, this API would be capable enough to allow the existing Console Control+C handler reimplemented by wrapping this API.

[davidfowl]

It should not be on Process. Process is for working with other processes, not your own process. I would put it on Environment.

Updated.

Does it need to take argument that describes the kind of signals you want to wait for and/or the kind of signal that was received?

I wanted to avoid that for this change. We can always add overloads that take an enum or flags enum with the signals to care about.

Does it need to give you an option to resume the default processing of the signal?

It could. And like today it would be confusing as there could be multiple waiters an each who can decide what the global state of resuming the signal handling should be. I think not having a callback API means it won't matter much in practice since my entire continuation will execute before signal processing resumes anyways.

Ideally, this API would be capable enough to allow the existing Console Control+C handler reimplemented by wrapping this API.

I don't think this API should be a replacement for the current ConsoleKeyProcess handler. I don't think it should initiate shutdown at all, just leave that up to the application.

[stephentoub]

I wanted to avoid that for this change. We can always add overloads that take an enum or flags enum with the signals to care about.

Why? What's special about these specific signals other than they're the ones required today? If we're going to add a general signal listening mechanism, it should be generalized.

That said, what is the expected behavior on Windows?

I don't think this API should be a replacement for the current ConsoleKeyProcess handler. I don't think it should initiate shutdown at all, just leave that up to the application.

The console handler doesn't initiate shutdown. It let's you intercept the signal that otherwise shuts down. It only has to do something special in this regard if you choose not to intercept.

I think not having a callback API means it won't matter much in practice since my entire continuation will execute before signal processing resumes anyways.

Not if it's async. Also, you can't run arbitrary managed code in signal handiers, so the callback needs to be made async from the signal handler anyway. For reference, you can see this in how we handle ctrl-C in Console. When a signal arrives, our registered signal handler queues our handling of it and then also immediately invokes the next handler in the chain:

runtime/src/libraries/Native/Unix/System.Native/pal_signal.c

Lines 43 to 68 in 4523fbf

	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
	}
	// 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)
	{
	struct sigaction* origHandler = OrigActionFor(sig);
	if (origHandler->sa_sigaction != NULL &&
	(void*)origHandler->sa_sigaction != (void*)SIG_DFL &&
	(void*)origHandler->sa_sigaction != (void*)SIG_IGN)
	{
	origHandler->sa_sigaction(sig, siginfo, context);
	}
	}
	}

There's then a thread processing this queue, which handles the SIGINT by invoking the user's callback:

runtime/src/libraries/Native/Unix/System.Native/pal_signal.c

Lines 109 to 117 in 79ae74f

	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);
	}

And if the callback says the ctrl-C shouldn't be canceled, at that point we can't still rely on the default tear-down-the-app-behavior, since the original signal is long gone, so we clear out our handler and re-send the signal, such that this time it'll tear things down:

runtime/src/libraries/Native/Unix/System.Native/pal_signal.c

Lines 191 to 199 in 79ae74f

	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);
	}

[davidfowl]

Why? What's special about these specific signals other than they're the ones required today? If we're going to add a general signal listening mechanism, it should be generalized.

My motivation is to fix bugs we cannot work around today to do with graceful shutdown. What makes these signals special is what I said here:

"Many systems use SIGTERM as a way to signal a graceful shutdown."

The pattern that repeats itself in many of these systems (Kubernetes, Docker, Systemd), send a SIGTERM to give the application some time to shutdown, then follow up with a SIGKILL, if the app hasn't shutdown in some timeframe. SIGINT, is the dev scenario where the developer hits CTRL+C on the command line.

That said, what is the expected behavior on Windows?

On windows, this usually ends up being CTRL+C.

Last time this conversation stalled at Mono.Posix. I don't have a problem if you think we should make it more general purpose, but these signals are special because of the shutdown semantics other systems are applying to these signals.

Not if it's async. Also, you can't run arbitrary managed code in signal handiers, so the callback needs to be made async from the signal handler anyway. For reference, you can see this in how we handle ctrl-C in Console. When a signal arrives, our registered signal handler queues our handling of it and then also immediately invokes the next handler in the chain:

Right, I figured this would be the case. (though I didn't look at the code). I don't have a strong opinion on if we should allow this. Again my motivation is about handling these buggy shutdown cases that are hard to workaround today without pinvoking.

[davidfowl]

I see there's a desire to make this more generic, I don't have a problem with that but I also don't have the more generic scenarios. SIGINT is already handled by CTRL+C, so there actually isn't a huge problem with not supporting that here IMO.

• https://docs.docker.com/engine/reference/commandline/stop/
• https://learnk8s.io/graceful-shutdown
• https://unix.stackexchange.com/a/605539

You can see they all follow this pattern or SIGTERM followed by SIGKILL. I had 2 goals with the original proposal:

• Write the same code on linux and windows
• Focus on the shutdown scenario, not the general signal handling scenario. This is why there was focus on SIGINT (Ctrl+C) and SIGTERM.

I do think there are 2 scenarios that I previously mentioned outside of generic signal handling:

• SIGINT and CTRL+C - Command line scenarios
• SIGTERM - Process Orchestrators that need to signal remote termination
• Windows CTRL+C is used for command line
• Windows CTRL_CLOSE_EVENT for docker stop scenarios (windows containers)

These are the shutdown cases we want to be able to handle uniformly in Microsoft.Extensions.Hosting and any console application in general.

What do we need to do to move this forward?

[jkotas]

We have been fixing this multiple times already by small incremental changes that just focused on the next problem, only to find that it still does not work well. If we continue this pattern by introducing a new API that just fixes the next problem, we are likely to find that it still does not work well. I think we need an API that does at least the following:

• Allows subscribing for a specific signal and/or tells you the type of signal received
• Allows resuming default signal processing once you have done your part
• Allows handling all shutdown signals: SIGINT, SIGQUIT, SIGTERM, SIGHUP on Unix, and CTRL_C_EVENT, CTRL_BREAK_EVENT, CTRL_CLOSE_EVENT, CTRL_LOGOFF_EVENT, CTRL_SHUTDOWN_EVENT on Windows.

[davidfowl]

If we want something this general purpose then we might want to take some inspiration from others

https://golang.org/pkg/os/signal/

https://nodejs.org/api/process.html#process_signal_events

Where we're unique in this space is typically our handling of windows. Most other platforms are *nix first so their APIs try to map *nix signals to various windows behaviors.

PS: I know we've had lengthy discussions about mono.posix and that's why this discussion worries me a bit (scope creep). My primary concern is still making sure those scenarios I mentioned get fixed.

cc @tmds because he knows all things about linux.

cc @danmoseley because this has come up before.

My main goal is to fix those bugs I closed and referenced in the issue, I see this general purpose feature as a plus to that.

[davidfowl]

Allows resuming default signal processing once you have done your part

I think this is gonna be problematic for the async case.

[jkotas]

Agree. It suggests that the API should be more like the existling Console.CancelKeyPress.

[davidfowl]

Ugh I very much dislike that. I'd prefer passing an argument to the method that informs if signal processing should continue. The async version would never has this mode and signal processing would always move to the next handler.

[jkotas]

I expect that the implementation is going to have an API like that at the lowest level internally. Why not polist it and expose it to give full control about the behavior if you need it? Then the rest is just a discussion about convenience wrappers.

[davidfowl]

Meaning we're not exposing an API that we expect the majority of users to use? We build another layer for that? That's fine with me but will result in potential duplication. It's needed because this is basically how every application that runs in a container works today. Microsoft.Extensions.Hosting tries to abstract this across different hosting environments with a lifetime abstraction. I assume we want people to be able to easily write console applications that can wait for shutdown signals without using that library right?

var running = Task.Run(() =>
{
    while (true)
    {
       Console.WriteLine(DateTime.Now);
       await Task.Delay(5000);
    }
});

var sigtermTask = Environment.WaitForSingalAsync(Signals.SIGTERM);
await Task.WhenAny(running, sigtermTask);

[davidfowl]

Actually it might be nicer to use CancellationToken here.

CancellationToken cancel = Environment.GetSingalToken(Signals.SIGTERM);

while (true)
{
    cancel.ThrowIfCancellationRequested();

    Console.WriteLine(DateTime.Now);

    await Task.Delay(5000, cancel);
}

[tmds]

command-line-api uses a CancellationToken for this: https://github.com/dotnet/command-line-api/blob/main/docs/Process-termination-handling.md.

Compared to an event, the CancellationToken is stateful.
And compared to WaitForShutdownSignal it can stop you from doing a bunch of stuff early on.

We want the main thread to continue executing for as long as it can until it has unwound. This lets us run any clean up code or logging before exit (see #44086)

I think a key part of what is requested here is that using the API, implies the runtime assumes Main will return on SIGTERM.

SIGINT or SIGTERM

The API should allow for SIGINT (CTRL+C) to do something other than terminate the app. For example, if you write a REPL, you may want it to stop the current command.

[davidfowl]

@tmds Why are you in my brain 😄

[davidfowl]

I think a key part of what is requested here is that using the API, implies the runtime assumes Main will return on SIGTERM.

YES. I want main to be the continuation.

The API should allow for SIGINT (CTRL+C) to do something other than terminate the app. For example, if you write a REPL, you may want it to stop the current command.

YEP.

[jkotas]

assume we want people to be able to easily write console applications that can wait for shutdown signals without using that library right?

I think it is rare for simple console applications to need to wait for shutdown signals. I think waiting for shutdown signals is for complex console applications or appmodels like Microsoft.Extensions.Hosting (in which case the appmodel can provide the convenience wrapper).

[davidfowl]

I think it is rare for simple console applications to need to wait for shutdown signals. I think the waiting for shutdown signals is for complex console applications or appmodels like Microsoft.Extensions.Hosting (in which case the appmodel can provide the convenience wrapper).

What would convince you that this isn't the case?

[tmds]

#44086 is about Main not completing on SIGTERM when the IHost has stopped.

[bartonjs]

Video

• "Signal" is a very generic word, how about "PosixSignal"?
• SignalHandlerRegistration should be a class instead of a struct
• SignalRegistration instead of SignalHandlerRegistration
• Use negative numbers in the PosixSignal enum and handle translation in the PAL
• Add PosixSignal enum members for all the ones we want to support, which should not include ones that we know will break the .NET runtime (SIGIO?)
• We sealed the types
• We added a constructor for PosixSignalContext for testability

namespace System.Runtime.InteropServices
{
     public enum PosixSignal
     {
         SIGHUP = -1,
         SIGINT = -2,
         SIGQUIT = -3,
         SIGTERM = -4,
         SIGCHLD = -5,
         ...
     }

     public sealed class PosixSignalContext
     {
         public PosixSignal Signal { get; }
         public bool Cancel { get; set; }

         public PosixSignalContext(PosixSignal signal) { }
     }

     public sealed class PosixSignalRegistration : IDisposable
     {
         private PosixSignalRegistration() { }

         public static PosixSignalRegistration Create(PosixSignal signal, Action<PosixSignalContext> handler);
     }
}

[doubleyewdee]

@bartonjs it would be neat if the signals enum mapped numerically to the most common values for the various signals (Linux x86/ARM or BSD, they are in broad agreement). So e.g. where you have SIGTERM as -4 I would have expected it to be -9 (SIGKILL being -4). This is an awesome proposal all around though!

[davidfowl]

The decision was to map these defaults signals understood by us to negative numbers so we can handle them specially. If you want to use other raw signals, pass the raw number through and it will bind to the OS.

[doubleyewdee]

I see. As a user I would have expected to be able to register to handle a wider variety of signals (e.g. SIGUSR1/SIGUSR2). I think SIGWINCH is also of note. Anecdotally I've seen SIGHUP used not as a terminator but as a "reload config" signal in a variety of systems as well.

If the API intent is to narrowly handle a certain class of "maybe terminate" signals then I think it needs a more specific name, the implication (at a glance) to me was that I could handle any or at least most POSIX signals if I registered for them...

[SteveL-MSFT]

For PowerShell, we would love to see SIGPIPE, SIGTTIN, and SIGTTOU. Related #452

[davidfowl]

You can map whatever signal you want, it's a cast away. Then we can discuss what things people need added to the enum and if it's possible to implement them without borking the runtime 😄:

PosixSignalRegistration.Create((PosixSignal)(10) /* SIGUSR1 */, context => ...);

[bartonjs]

For PowerShell, we would love to see SIGPIPE, SIGTTIN, and SIGTTOU.

TTIN/TTOU were on the list of things discussed. From the API Review perspective we're OK with adding anything using the standard POSIX name, using an arbitrary/incrementing negative number to get handled appropriately in a PAL. Assuming SIGPIPE isn't something that the runtime is using already in an important way, it'd be in the same bucket.

it would be neat if the signals enum mapped numerically to the most common values for the various signals

There's no real reason for symmetry. A note I took in the middle of the review was that "universal" codes (e.g. SIGTERM looks like it's always 15) would be positive and non-universal ones would be negative, but we decided to just use negative numbers and translate. (And, as @davidfowl pointed out, if you pass a positive number we just don't translate it).

Since our code will probably be something like

public static PosixSignalRegistration Create(PosixSignal signal, ...)
{
    if (signal < PosixSignal.TheLastOneWeSupported)
        throw new ArgumentOutOfRangeException(nameof(signal));

    int nativeCode = PalMap(signal);
    ...
}

PALEXPORT int32_t PalMap(PalSignal value)
{
    switch (value)
    {
        case PAL_SIGABRT:
            return SIGABRT;
        ...
        default:
           return value;
    }
}

There's not much benefit in using affinitized numbers (in fact, it makes "you used a negative value we don't understand" harder).

[davidfowl]

@tmds is this something you would want to take on?

[tmds]

@davidfowl I'll try to pick this up in June. I'll add a comment when I start working on it.

[alexrp]

@bartonjs could we consider the name UnixSignal instead? It just occurred to me that there's a lot of precedent in the BCL with this term already, whereas the term Posix doesn't appear to be used anywhere else. RIDs also use the term unix rather than posix. The 'new' Mono.Posix package/assembly is also called Mono.Unix now: https://github.com/mono/mono.posix/tree/main/src/Mono.Unix

[stephentoub]

@tmds, I merged your PR (thanks!) and I'll finish it.

[davidfowl]

@stephentoub are you going to change Microsoft.Extensions.Hosting as well?

[stephentoub]

No, just the PosixSignalRegistration Windows implementation.

[davidfowl]

OK I will work on the hosting PR