buildkite / buildkite/agent

Change default shell signal from SIGTERM to SIGINT

Open
#1,392 2 comments 2 reactions 0 assignees View on GitHub
breaking
Dominant language
Go
Stars
1.1k
Forks
378
Avg merge
2d 6h
Merged PRs (30d)
74

Description

For historical reasons when we send interrupts to process groups, we default to using `SIGTERM`:

https://github.com/buildkite/agent/blob/b9bc5ecc8e67fd6734d8930fb5ebb848902d99f2/process/signal.go#L35-L38

When we send `SIGTERM` to a process group, bash exits without waiting on its child processes to exit safely. In many cases, the child processes end from other effects, such as `SIGPIPE` caused by trying to read or write over an orphaned pipeline. This unreliability can lead to lost output or incorrect shutdowns of subprocesses. A script author could potentially work around this using a signal handler. A better approach would be to send a `SIGINT` signal instead, which will be followed by a `SIGKILL` if the group takes too long to respond. `SIGINT` only gets propagated to foreground child processes, so special case handling will still be required for users who are backgrounding processes.

Most processes treat `SIGINT` the same as `SIGTERM`, but we can't be sure if anyone is relying on this behaviour. Therefore, this should be considered a breaking change. A user can experiment with this change ahead of time by using the configurable cancel signals introduced by #1041 and #1390.

I've tried to carefully research this and get the above details correct, but please feel free to contribute any corrections or details I might have missed.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.