runner: a step is a process group, and ending it ends its children !176

merged merged by cmc on 2026-09-02 05:06 UTC · krz/gitbay:runner-kill-group into main

4 files changed, +55 −5

Layout: unified · split

cmd/gitbay-runner/main.go +13 −4

		@@ -204,11 +204,22 @@ func (r *runner) run(j job) bool {
204	204	return false, "cancelled"
205	205	default:
206	206	}
	207	ownProcessGroup(cmd)
207	208	if err := cmd.Start(); err != nil {
208	209	return false, fmt.Sprintf("start: %v", err)
209	210	}
210	211	done := make(chan error, 1)
211	212	go func() { done <- cmd.Wait() }()
	213	// After a kill, Wait returns once every holder of the log pipe is
	214	// gone; the group kill makes that prompt, and the cap makes sure a
	215	// straggler cannot hold the build open.
	216	reap := func() {
	217	killTree(cmd)
	218	select {
	219	case <-done:
	220	case <-time.After(10 * time.Second):
	221	}
	222	}
212	223	select {
213	224	case err := <-done:
214	225	if err != nil {
		@@ -216,12 +227,10 @@ func (r *runner) run(j job) bool {
216	227	}
217	228	return true, ""
218	229	case <-cancelled:
219		cmd.Process.Kill()
220		<-done
	230	reap()
221	231	return false, "cancelled"
222	232	case <-time.After(time.Until(deadline)):
223		cmd.Process.Kill()
224		<-done
	233	reap()
225	234	return false, fmt.Sprintf("build timed out after %s", r.timeout)
226	235	}
227	236	}

cmd/gitbay-runner/proc_other.go added +13

		@@ -0,0 +1,13 @@
	1	//go:build !unix
	2
	3	package main
	4
	5	import "os/exec"
	6
	7	func ownProcessGroup(cmd *exec.Cmd) {}
	8
	9	func killTree(cmd *exec.Cmd) {
	10	if cmd.Process != nil {
	11	cmd.Process.Kill()
	12	}
	13	}

cmd/gitbay-runner/proc_unix.go added +26

		@@ -0,0 +1,26 @@
	1	//go:build unix
	2
	3	package main
	4
	5	import (
	6	"os/exec"
	7	"syscall"
	8	)
	9
	10	// A step runs as its own process group, so ending it ends everything it
	11	// started. Killing only the shell leaves its children alive and holding
	12	// the log pipe, and a wait on that pipe lasts as long as the longest
	13	// child: dash forks a single command rather than exec'ing it, so on a
	14	// Debian host "sh -c 'sleep 120'" survived its shell by two minutes.
	15	func ownProcessGroup(cmd *exec.Cmd) {
	16	cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
	17	}
	18
	19	func killTree(cmd *exec.Cmd) {
	20	if cmd.Process == nil {
	21	return
	22	}
	23	if err := syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL); err != nil {
	24	cmd.Process.Kill()
	25	}
	26	}

e2e/build_cancel_test.go +3 −1

		@@ -105,7 +105,7 @@ func TestBuildCancelRunning(t *testing.T) {
105	105	mustGit(t, work, env, "clone", inst.sshURL("alice/slow"), "w")
106	106	dir := filepath.Join(work, "w")
107	107	os.MkdirAll(filepath.Join(dir, ".gitbay"), 0o755)
108		os.WriteFile(filepath.Join(dir, ".gitbay", "ci.yml"), []byte("jobs:\n slow:\n steps:\n - echo starting\n - sleep 120\n - echo never\n"), 0o644)
	108	os.WriteFile(filepath.Join(dir, ".gitbay", "ci.yml"), []byte("jobs:\n slow:\n steps:\n - echo starting\n - sleep 120 & wait\n - echo never\n"), 0o644)
109	109	mustGit(t, dir, env, "checkout", "-q", "-b", "main")
110	110	mustGit(t, dir, env, "add", ".")
111	111	mustGit(t, dir, env, "commit", "-q", "-m", "slow")
		@@ -117,6 +117,8 @@ func TestBuildCancelRunning(t *testing.T) {
117	117	}
118	118
119	119	// A real runner, in the background, claims the build and sits in sleep.
	120	// The step forks sleep as a child of the shell, the way dash runs
	121	// every command, so the cancel must reach past the shell to end it.
120	122	opts := fmt.Sprintf("-p %d -i %s -o IdentitiesOnly=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=%s -o BatchMode=yes",
121	123	inst.port, runnerKey, filepath.Join(inst.sshDir, "known_hosts"))
122	124	runner := exec.Command(inst.runner, "-once", "-remote", "git@127.0.0.1", "-ssh-opts", opts,