Opt-out of signal handling

[MarcoPolo]

Is your feature request related to a problem? Please describe.

A user can't make use of the default behavior of signals in Go programs because we always register signal handlers.

I would like to use the app lifecycle without it registering a signal handler.

Describe the solution you'd like A clear and concise description of what you want to happen.

• Add an Option to App that opts-out of registering the signal receiver.

This should be an easy change:

1. Add an option that sets signalReceivers.{notify,stopNotify} to no-ops.
2. Add docs around option.

Happy to add once their's consensus.

Describe alternatives you've considered

I use fx as part of my library go-libp2p. An alternative is to expose the app.Done in my constructed object and let the user handle the signal explicitly this way. The problem with this approach is that it changes the standard way signal handling works in Go. User's should just concern themselves with signal.Notify rather than having to know that app.Done has registered a signal handler for them. And if the user hasn't registered with signal.Notify they would expect the default behavior from Go.

Is this a breaking change? We do not accept breaking changes to the existing API. Please consider if your proposed solution is backwards compatible. If not, we can help you make it backwards compatible, but this must be considered when we consider new features.

No. It can be provided as an option. The default can remain the same.

Additional context

See related issues in go-libp2p:

• https://github.com/libp2p/go-libp2p/issues/2785
• One proposed PR, but I'd favor the ability to opt-out in fx: https://github.com/libp2p/go-libp2p/pull/2829#issuecomment-2161199081

[JacobOaks]

Hey @MarcoPolo - thanks for the issue.

Can you help me understand why this feature would be necessary, especially given the room it opens up for user error?

Not registering a signal handler for Fx is slightly dangerous in that unless a user makes sure to handle signals themselves and gracefully close the Fx application, OnStop hooks simply won't run, meaning important cleanup won't happen. It is undesirable to simply exit immediately in the case of a signal. I understand it may not be exactly what a user expects in the case of Fx being used as part of a larger library/application - but it is a necessary safeguard against dirty shutdowns, and using Fx in such a way is definitely a sort of non-traditional way such as within a library.

Conversely, as far as I can tell, signals can have multiple handlers registered, so there isn't anything stopping somebody from registering their own signal handlers if they do have some legitimate reasoning for doing so.

A user can't make use of the default behavior of signals in Go programs because we always register signal handlers. I would like to use the app lifecycle without it registering a signal handler.

Can you provide a specific example of how Fx's registration of signal handlers for app cleanup conflicts with how a user may want their program to respond to signals?

[ccoVeille]

I'm sharing the same concern, that's why I raised the point in the review by asking if it was something related to test only

https://github.com/uber-go/fx/pull/1215#discussion_r1644870789

[JacobOaks]

Just to expand on my previous comment, here's a trivial example of how you could allow Fx to handle the signal to gracefully shutdown, but still close the entire Go process in a sort of non-traditional use case of Fx as part of a larger library/framework.

sig := make(chan os.Signal, 1)
signal.Notify(sig, syscall.SIGINT, syscall.SIGTERM)

app.Run()

select {
case <-sig:
    os.Exit(0)
default:
}

// my application continues running
time.Sleep(time.Second * 5)

[MarcoPolo]

Thanks for taking the time @JacobOaks, let me try to explain a bit more:

Not registering a signal handler for Fx is slightly dangerous in that unless a user makes sure to handle signals themselves and gracefully close the Fx application.

I can understand the desire to provide these guard rails, but nothing guarantees that the application will get a chance to gracefully shut down. The OS can simply crash. I think "crash-only design" is the better approach for building reliable systems. To quote George Candea and Armando Fox, Crash-Only Software:

Since crashes are unavoidable, software must be at least as well prepared for a crash as it is for a clean shutdown. But then—in the spirit of Occam’s Razor—if software is crash-safe, why support additional, non-crash mechanisms for shutting down?

I'll link this blog post that gives the topic more justice than I would in this comment: https://secretartofscience.com/crash-only.

Conversely, as far as I can tell, signals can have multiple handlers registered, so there isn't anything stopping somebody from registering their own signal handlers if they do have some legitimate reasoning for doing so.

This is true, but the current behavior, that you can't opt-out of, is that fx prevents users from using the default Go signal handling. There's nothing I can do as a library author and a user of fx to give my users that choice.

In other words, fx doesn't compose well with other libraries. It's assuming it owns the application and is dealing with the end-user and registering signal handlers on their behalf. But when composed with other libraries it's surprising to end-users that a sub-dependency of theirs is overriding Go's default signal handling semantics. And, to be clear, this issue is just about providing an option to opt-out of this. I don't mean to change the defaults.

Can you provide a specific example of how Fx's registration of signal handlers for app cleanup conflicts with how a user may want their program to respond to signals?

Here's a simple example using go-libp2p:

func main() {
    h, err := libp2p.New()
    if err != nil {
        panic(err)
    }
    defer h.Close()

    fmt.Println("Host ID:", h.ID())
    fmt.Println("Listening on:", h.Addrs())
    select {}
}

(A slightly more realistic example here.)

A user expects that ctrl-c should quit the program, because that's what would happen if they didn't call libp2p.New(). That's Go's default behavior. This isn't just my own opinion, here are multiple users who have been surprised by this behavior: 1 2.

You could argue that I should tell all my users that they need to handle their signals directly if they want to exit the process. But that has a trickle down effect, because my library might just be a dependency of someone else's library. Now that library author has to explain to their users why Go's default signal handling no longer works and they have to handle their signals directly...

---

But look, I get the hesitance of adding new features, especially when it's something that feels like it's not the main use case (fx being used by a library rather than owning the application). I'm suggesting this feature because I've gotten a lot of benefit from fx, and would like to see it improved.

[MarcoPolo]

One more thing, I'm not attached to my particular implementation as a solution to this issue. Maybe what makes more sense is if we don't register signal handlers until the user essentially requests it. This means if a user calls app.Start, no signal handler is registered, but as soon as they call app.Wait or app.Done fx registers the signal handler since the user has communicated their intent to have fx listen for signals. This should just work with app.Run since it calls app.Wait. I don't think this is any different than what we're doing now, but gives users more options.

[JacobOaks]

Hey @MarcoPolo - thanks for the clarification. I am really happy to hear you like using Fx and care about its improvement 😁. Let me jot down my thoughts on this.

fx doesn't compose well with other libraries. It's assuming it owns the application and is dealing with the end-user and registering signal handlers on their behalf

I understand how this is pretty inconvenient for your use case, but to be purely pedantic, Fx is not just a library, it is a framework. It isn't designed to be a small transitive dependency of an application. It is designed to be the application, exactly as you're describing (If you're looking for a DI tool that is better as a general-purpose library, I'd recommend dig). In order to be successful at that, it is opinionated and provides guardrails to make the fast path of developing an app using it safe and easy. I think we should avoid adding new APIs that unravel these opinions and have potentially quite dangerous side effects.

nothing guarantees that the application will get a chance to gracefully shut down.

I'm not sure I understand. Are you arguing that because ungraceful shutdowns are technically always possible, we should not concern ourselves with protecting against them when possible?

A user expects that ctrl-c should quit the program, because that's what would happen if they didn't call libp2p.New(). That's Go's default behavior.

Go is an extremely general purpose programming language, while Fx is a highly opinionated application framework. I don't think Fx is necessary obligated to uphold any one behavior just because it occurs in a framework-less environment. After all, if it did this to the fullest extent, there would be no framework at all.

There's nothing I can do as a library author and a user of fx to give my users that choice.

There may not be anything you can do to revert back to Go's default behavior in the way you want, but I think I would argue that's an unfortunate price paid by trying to fit a highly opinionated framework into a more general library. However, I do think there are things you could do to make this UX better for your users. E.g., you could add your own signal handler at the library level that calls os.Exit like the example I showed above, overridable by some user-provided config/option. Then they would basically experience Go's default behavior, with the ability to override if they wish in a first-class manner supported by your framework.

I'm curious to hear your thoughts, and I want to hear from other contributors & maintainers (cc @tchung1118 @abhinav) about this, but yeah right now I'm not sure it's an addition I'd recommend to Fx.

[abhinav]

So, I was confused by why someone couldn't just call App.Start directly if they didn't want Fx to register signal handlers because my memory said we didn't register signal handlers until someone called Wait/Done.

Looks like this regressed in https://github.com/uber-go/fx/pull/984. The prior behavior was as described: Start and Stop do not register signal handlers. Run and Done do.

I'm not sure how easy it is to change it back and still support the custom shutdown functionality we have. Thoughts on this, @JacobOaks @tchung1118?

One note: If we're going to support this, it's preferable using the Start/Stop separation approach versus an option. And as we're tweaking these pieces, it's also worth thinking about how the changes will play with fx.Shutdowner.

[MarcoPolo]

The prior behavior was as described: Start and Stop do not register signal handlers. Run and Done do.

This would work for my use case. I use App.Start directly.

I'm not sure how easy it would be to fix that back.

I know you didn't ask me, but I think this simple patch would fix it: https://gist.github.com/MarcoPolo/08816c59f244c48822da6e8940a1d3d0

[JacobOaks]

I see - I wasn't aware this was actually a change in behavior on Fx's part - thanks @abhinav. In that case, I do think we should try to revert to the original behavior, rather than add a new option.

@MarcoPolo I think we can do something very similar to your patch, except I don't think we need to use the sync.Once since (*signalReceivers).Start() already has a lock, and won't allow itself to start once already running.

The other thing to consider is how we want custom Shutdown signals to interact with the app in the case where nobody is Wait()ing or Done()ing. Prior to #984, it looks like this would not actually shut down the app. Instead, the shutdown signal was saved to be sent along any Done channels to be registered after-the-fact.

I think we could probably strive to emulate this behavior again, since it remains consistent with the behavior before the degradation, and does allow for good control of when to actually shutdown/respond to shutdown signals for more complex use cases, while still "just working nicely" for the simple app.Run() case.

@MarcoPolo feel free to open up/modify your existing PR for this, otherwise I can try to do this in the coming days.

[ccoVeille]

Hey guys, I appreciate your discussion and find it valuable. The discussion is open and detailed, it's a real pleasure to see a project going this way.

[MarcoPolo]

PR #1215 has been updated and added some tests to protect against regressions.

The other thing to consider is how we want custom Shutdown signals to interact with the app in the case where nobody is Wait()ing or Done()ing.

I'm pretty sure this is already handled via the recv.last field. I've added a test to show this and protect against regressions.

[ccoVeille]

I commented and suggested a few changes