Bazelisk run doesn't propagate signals to the child process

[jschaf]

Reproduction

The situation to reproduce is a bit complex:

1. I have Go binary at //erp/dev.
2. //erp/dev runs other processes via bazelisk run, notably a Go-wrapper around Postgres.

The resulting process tree:

bazelisk run //erp/dev 
  # Starts a Go binary
  # The Go binary in turn calls the Go-wrapper for Postgres
  bazelisk run //db/go_postgres
    # go_postgres is a Go binary that calls the Postgres binary
    postgres

The problem is that if //erp/dev sends SIGINT directly to the //db/go_postgres using the Go stdlib exec.Cmd, bazelisk run swallows the signal.

However, if I type ctrl-c at the terminal, Postgres receives the signal. I think this is because the shell wires up the process attributes differently from Go (maybe https://stackoverflow.com/a/35435038/30900?)

Context

As for why bazelisk run has this behavior:

Previously, when running bazelisk run under a shell, typing ctrl-c would send SIGINT twice to the child process. I think that's the same behavior I saw--processes started with Go propagate signals differently from processes started by the shell.

https://github.com/bazelbuild/bazelisk/issues/307 noted the issue and recommended ignoring SIGINT. The resulting PR https://github.com/bazelbuild/bazelisk/pull/322 fixed the issue by ignoring SIGINT but has a few related problems

The relevant bazelisk code: https://github.com/bazelbuild/bazelisk/blob/89e6693df1e9a9d49fe95f2814d7139d258a2f2c/core/core.go#L579

func runBazel(/*omitted*/) (int, error) {
    cmd := makeBazelCmd()
    cmd.Start()

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

The two related problems:

1. The channel isn't buffered, meaning the signal delivery isn't guaranteed. From https://pkg.go.dev/os/signal#Notify:

   Package signal will not block sending to c: the caller must ensure that c has sufficient buffer space to keep up with the expected signal rate. For a channel used for notification of just one signal value, a buffer of size 1 is sufficient.

   See also: https://stackoverflow.com/a/68593935/30900

2. The code calls cmd.Process.Kill(), which sends SIGKILL; it doesn't forward the caught signal.

Design sketch

For my use case, I'd like to be able to forward whatever signal I want when starting processes via bazelisk run from Go. This is particularly important for Postgres, which has robust signal handling:

• SIGTERM: smart shutdown
• SIGINT: fast shutdown
• SIGQUIT: immediate shutdown
• SIGHUP: reload config

As a path forward (not yet validated) that avoids the double signaling:

1. bazelisk run starts processes so that automatic signal propagation is disabled:

cmd.SysProcAttr = &syscall.SysProcAttr{
    Setpgid: true,
    Pgid:    0,
}

2. bazelisk run forwards all signals to the child process. We avoid the double SIGINT via step 1.