Commit 6042b09dfc
Verified · cmc ci/build: success ci/test: success ci/vuln: success
Layout: unified · split
cmd/gitbay-runner/main.go +13 −4
| @@ -204,11 +204,22 @@ func (r *runner) run(j job) bool { | |||
| 204 | return false, "cancelled" | 204 | return false, "cancelled" |
| 205 | default: | 205 | default: |
| 206 | } | 206 | } |
| 207 | ownProcessGroup(cmd) | ||
| 207 | if err := cmd.Start(); err != nil { | 208 | if err := cmd.Start(); err != nil { |
| 208 | return false, fmt.Sprintf("start: %v", err) | 209 | return false, fmt.Sprintf("start: %v", err) |
| 209 | } | 210 | } |
| 210 | done := make(chan error, 1) | 211 | done := make(chan error, 1) |
| 211 | go func() { done <- cmd.Wait() }() | 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 | select { | 223 | select { |
| 213 | case err := <-done: | 224 | case err := <-done: |
| 214 | if err != nil { | 225 | if err != nil { |
| @@ -216,12 +227,10 @@ func (r *runner) run(j job) bool { | |||
| 216 | } | 227 | } |
| 217 | return true, "" | 228 | return true, "" |
| 218 | case <-cancelled: | 229 | case <-cancelled: |
| 219 | cmd.Process.Kill() | 230 | reap() |
| 220 | <-done | ||
| 221 | return false, "cancelled" | 231 | return false, "cancelled" |
| 222 | case <-time.After(time.Until(deadline)): | 232 | case <-time.After(time.Until(deadline)): |
| 223 | cmd.Process.Kill() | 233 | reap() |
| 224 | <-done | ||
| 225 | return false, fmt.Sprintf("build timed out after %s", r.timeout) | 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 | mustGit(t, work, env, "clone", inst.sshURL("alice/slow"), "w") | 105 | mustGit(t, work, env, "clone", inst.sshURL("alice/slow"), "w") |
| 106 | dir := filepath.Join(work, "w") | 106 | dir := filepath.Join(work, "w") |
| 107 | os.MkdirAll(filepath.Join(dir, ".gitbay"), 0o755) | 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 | mustGit(t, dir, env, "checkout", "-q", "-b", "main") | 109 | mustGit(t, dir, env, "checkout", "-q", "-b", "main") |
| 110 | mustGit(t, dir, env, "add", ".") | 110 | mustGit(t, dir, env, "add", ".") |
| 111 | mustGit(t, dir, env, "commit", "-q", "-m", "slow") | 111 | mustGit(t, dir, env, "commit", "-q", "-m", "slow") |
| @@ -117,6 +117,8 @@ func TestBuildCancelRunning(t *testing.T) { | |||
| 117 | } | 117 | } |
| 118 | 118 | ||
| 119 | // A real runner, in the background, claims the build and sits in sleep. | 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 | opts := fmt.Sprintf("-p %d -i %s -o IdentitiesOnly=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=%s -o BatchMode=yes", | 122 | opts := fmt.Sprintf("-p %d -i %s -o IdentitiesOnly=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=%s -o BatchMode=yes", |
| 121 | inst.port, runnerKey, filepath.Join(inst.sshDir, "known_hosts")) | 123 | inst.port, runnerKey, filepath.Join(inst.sshDir, "known_hosts")) |
| 122 | runner := exec.Command(inst.runner, "-once", "-remote", "git@127.0.0.1", "-ssh-opts", opts, | 124 | runner := exec.Command(inst.runner, "-once", "-remote", "git@127.0.0.1", "-ssh-opts", opts, |