Skip to content

Commit

Permalink
Improve signal handling. (#642)
Browse files Browse the repository at this point in the history
Every signal specially recognized by Bazel that doesn't cause an immediate exit
shouldn't cause Bazelisk to exit either, as this risks orphaning the Bazel
process and making it uncontrollable by the terminal. (You can see this in action
by sending a Ctrl-\ to Bazelisk, which causes it to immediately exit and makes it
impossible to abort the still running Bazel invocation with Ctrl-C.)

Note that forwarding is not necessary for a signal delivered by the terminal
(i.e. through Ctrl-C or Ctrl-\) because the terminal already delivers it to the
entire process group. For manually delivered signals, users should arguably
know what they're doing, so let's not go out of our way to forward them.

This also disables the printing of a Go stack dump upon SIGQUIT, which is
unhelpful: users tend to report it in place of the far more useful Java thread
dump.
  • Loading branch information
tjgq authored Nov 28, 2024
1 parent c3aa711 commit edab5e0
Showing 1 changed file with 15 additions and 10 deletions.
25 changes: 15 additions & 10 deletions core/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -619,16 +619,21 @@ func runBazel(bazel string, args []string, out io.Writer, config config.Config)
return 1, fmt.Errorf("could not start Bazel: %v", err)
}

c := make(chan os.Signal)
signal.Notify(c, os.Interrupt, syscall.SIGTERM)
go func() {
s := <-c

// Only forward SIGTERM to our child process.
if s != os.Interrupt {
cmd.Process.Kill()
}
}()
// Ignore signals recognized by the Bazel client.
// The Bazel client implements its own handling of these signals by
// forwarding them to the Bazel server, and they don't necessarily cause
// the invocation to be immediately aborted. In particular, SIGINT and
// SIGTERM are handled gracefully and may cause a delayed exit, while
// SIGQUIT requests a Java thread dump from the Bazel server, but doesn't
// abort the invocation. Normally, these signals are delivered by the
// terminal to the entire process group led by Bazelisk. If Bazelisk were
// to immediately exit in response to one of these signals, it would cause
// the still running Bazel client to become an orphan and uncontrollable
// by the terminal. As a side effect, we also suppress the printing of a
// Go stack trace upon receiving SIGQUIT, which is unhelpful as users tend
// to report it instead of the far more valuable Java thread dump.
// TODO(#512): We may want to treat a `bazel run` commmand differently.
signal.Ignore(syscall.SIGINT, syscall.SIGQUIT, syscall.SIGTERM)

err = cmd.Wait()
if err != nil {
Expand Down

0 comments on commit edab5e0

Please sign in to comment.