Skip to content

Commit 4f9770a

Browse files
committed
cmd/gitmirror: kill child processes upon timeout
We've seen evidence that we're not terminating Git subprocesses on timeout. This is probably an instance of https://golang.org/issue/23019, which we can deal with by killing all the children rather than just the one we started. Updates golang/go#38887. Change-Id: Ie7999122f9c063f04de1a107a57fd307e7a5c1d2 Reviewed-on: https://go-review.googlesource.com/c/build/+/347294 Trust: Heschi Kreinick <[email protected]> Run-TryBot: Heschi Kreinick <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]>
1 parent 3b206e5 commit 4f9770a

File tree

2 files changed

+54
-2
lines changed

2 files changed

+54
-2
lines changed

cmd/gitmirror/gitmirror.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -723,8 +723,12 @@ func handleDebugEnv(w http.ResponseWriter, r *http.Request) {
723723
}
724724
}
725725

726-
// runCommandContext runs cmd controlled by ctx.
727-
func runCmdContext(ctx context.Context, cmd *exec.Cmd) error {
726+
// runCmdContext allows OS-specific overrides of process execution behavior.
727+
// See runCmdContextLinux.
728+
var runCmdContext = runCmdContextDefault
729+
730+
// runCommandContextDefault runs cmd controlled by ctx.
731+
func runCmdContextDefault(ctx context.Context, cmd *exec.Cmd) error {
728732
if err := cmd.Start(); err != nil {
729733
return err
730734
}

cmd/gitmirror/gitmirror_linux.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
package main
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"os"
7+
"os/exec"
8+
"syscall"
9+
"time"
10+
)
11+
12+
func init() {
13+
runCmdContext = runCmdContextLinux
14+
}
15+
16+
// runCommandContext runs cmd controlled by ctx, killing it and all its
17+
// children if necessary. cmd.SysProcAttr must be unset.
18+
func runCmdContextLinux(ctx context.Context, cmd *exec.Cmd) error {
19+
if cmd.SysProcAttr != nil {
20+
return fmt.Errorf("cmd.SysProcAttr must be nil")
21+
}
22+
cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
23+
if err := cmd.Start(); err != nil {
24+
return err
25+
}
26+
resChan := make(chan error, 1)
27+
go func() {
28+
resChan <- cmd.Wait()
29+
}()
30+
31+
select {
32+
case err := <-resChan:
33+
return err
34+
case <-ctx.Done():
35+
}
36+
// Canceled. Interrupt and see if it ends voluntarily.
37+
cmd.Process.Signal(os.Interrupt)
38+
select {
39+
case <-resChan:
40+
return ctx.Err()
41+
case <-time.After(time.Second):
42+
}
43+
// Didn't shut down in response to interrupt. It may have child processes
44+
// holding stdout/sterr open. Kill its process group hard.
45+
syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL)
46+
<-resChan
47+
return ctx.Err()
48+
}

0 commit comments

Comments
 (0)