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.

#307 noted the issue and recommended ignoring SIGINT. The resulting PR #322 fixed the issue by ignoring SIGINT but has a few related problems

The relevant bazelisk code:

bazelisk/core/core.go

Line 579 in 89e6693

// Only forward SIGTERM to our child process.

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.