[CLI] Fix interrupt handling for bb run #8378
Conversation
| // Force kill the child process if the CLI receives an interrupt. | ||
| sigs := make(chan os.Signal, 1) | ||
| signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM) | ||
| go func() { | ||
| select { | ||
| case <-sigs: | ||
| syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) | ||
| case <-done: | ||
| } | ||
| }() |
There was a problem hiding this comment.
I think this will break some things, e.g. when pressing Ctrl+C when running the app with bb run, it should start a graceful shutdown, but I think this will force-kill the process?
I think it's surprising that we're not handling Ctrl+C properly since the current code is handing off control to the child process using exec(), replacing the current process with the child. And I believe the generated --run_script should be doing the same thing - exec()-ing the child process so that the child process replaces it. So when e.g. doing bb run server, I think the desired process tree is bash > buildbuddy instead of something like bash > bb > run_script.sh > buildbuddy. So buildbuddy should be directly connected to the terminal, and receive Ctrl+C directly.
Maybe there's some unexpected process in the process tree that's intercepting signals? (Might be easy to check with a tool like pstree - can see what the process tree looks like after the binary is started, and compare the output with bazel vs bb)
When invoking a run script from the CLI, force kill the child if the CLI is interrupted.
Fixes #8326 (comment)