diff --git a/src/cmd/go/internal/script/cmds.go b/src/cmd/go/internal/script/cmds.go index 9fb092e0d8a1d0..393f565733d499 100644 --- a/src/cmd/go/internal/script/cmds.go +++ b/src/cmd/go/internal/script/cmds.go @@ -6,7 +6,6 @@ package script import ( "cmd/go/internal/robustio" - "context" "errors" "fmt" "internal/diff" @@ -36,7 +35,7 @@ func DefaultCmds() map[string]Cmd { "cp": Cp(), "echo": Echo(), "env": Env(), - "exec": Exec(os.Interrupt, 100*time.Millisecond), // arbitrary grace period + "exec": Exec(func(cmd *exec.Cmd) error { return cmd.Process.Signal(os.Interrupt) }, 100*time.Millisecond), // arbitrary grace period "exists": Exists(), "grep": Grep(), "help": Help(), @@ -400,7 +399,7 @@ func Env() Cmd { // When the Script's context is canceled, Exec sends the interrupt signal, then // waits for up to the given delay for the subprocess to flush output before // terminating it with os.Kill. -func Exec(interrupt os.Signal, delay time.Duration) Cmd { +func Exec(cancel func(*exec.Cmd) error, waitDelay time.Duration) Cmd { return Command( CmdUsage{ Summary: "run an executable program with arguments", @@ -428,13 +427,19 @@ func Exec(interrupt os.Signal, delay time.Duration) Cmd { } } - return startCommand(s, name, path, args[1:], interrupt, delay) + return startCommand(s, name, path, args[1:], cancel, waitDelay) }) } -func startCommand(s *State, name, path string, args []string, interrupt os.Signal, gracePeriod time.Duration) (WaitFunc, error) { +func startCommand(s *State, name, path string, args []string, cancel func(*exec.Cmd) error, waitDelay time.Duration) (WaitFunc, error) { var stdoutBuf, stderrBuf strings.Builder - cmd := exec.Command(path, args...) + cmd := exec.CommandContext(s.Context(), path, args...) + if cancel == nil { + cmd.Cancel = nil + } else { + cmd.Cancel = func() error { return cancel(cmd) } + } + cmd.WaitDelay = waitDelay cmd.Args[0] = name cmd.Dir = s.Getwd() cmd.Env = s.env @@ -444,16 +449,9 @@ func startCommand(s *State, name, path string, args []string, interrupt os.Signa return nil, err } - var waitErr error - done := make(chan struct{}) - go func() { - waitErr = waitOrStop(s.Context(), cmd, interrupt, gracePeriod) - close(done) - }() - wait := func(s *State) (stdout, stderr string, err error) { - <-done - return stdoutBuf.String(), stderrBuf.String(), waitErr + err = cmd.Wait() + return stdoutBuf.String(), stderrBuf.String(), err } return wait, nil } @@ -535,67 +533,6 @@ func pathEnvName() string { } } -// waitOrStop waits for the already-started command cmd by calling its Wait method. -// -// If cmd does not return before ctx is done, waitOrStop sends it the given interrupt signal. -// If killDelay is positive, waitOrStop waits that additional period for Wait to return before sending os.Kill. -// -// This function is copied from the one added to x/playground/internal in -// http://golang.org/cl/228438. -func waitOrStop(ctx context.Context, cmd *exec.Cmd, interrupt os.Signal, killDelay time.Duration) error { - if cmd.Process == nil { - panic("waitOrStop called with a nil cmd.Process — missing Start call?") - } - if interrupt == nil { - panic("waitOrStop requires a non-nil interrupt signal") - } - - errc := make(chan error) - go func() { - select { - case errc <- nil: - return - case <-ctx.Done(): - } - - err := cmd.Process.Signal(interrupt) - if err == nil { - err = ctx.Err() // Report ctx.Err() as the reason we interrupted. - } else if err == os.ErrProcessDone { - errc <- nil - return - } - - if killDelay > 0 { - timer := time.NewTimer(killDelay) - select { - // Report ctx.Err() as the reason we interrupted the process... - case errc <- ctx.Err(): - timer.Stop() - return - // ...but after killDelay has elapsed, fall back to a stronger signal. - case <-timer.C: - } - - // Wait still hasn't returned. - // Kill the process harder to make sure that it exits. - // - // Ignore any error: if cmd.Process has already terminated, we still - // want to send ctx.Err() (or the error from the Interrupt call) - // to properly attribute the signal that may have terminated it. - _ = cmd.Process.Kill() - } - - errc <- err - }() - - waitErr := cmd.Wait() - if interruptErr := <-errc; interruptErr != nil { - return interruptErr - } - return waitErr -} - // Exists checks that the named file(s) exist. func Exists() Cmd { return Command( @@ -834,7 +771,7 @@ func Mv() Cmd { // Program returns a new command that runs the named program, found from the // host process's PATH (not looked up in the script's PATH). -func Program(name string, interrupt os.Signal, gracePeriod time.Duration) Cmd { +func Program(name string, cancel func(*exec.Cmd) error, waitDelay time.Duration) Cmd { var ( shortName string summary string @@ -864,7 +801,7 @@ func Program(name string, interrupt os.Signal, gracePeriod time.Duration) Cmd { if pathErr != nil { return nil, pathErr } - return startCommand(s, shortName, path, args, interrupt, gracePeriod) + return startCommand(s, shortName, path, args, cancel, waitDelay) }) } diff --git a/src/cmd/go/internal/vcweb/script.go b/src/cmd/go/internal/vcweb/script.go index b0a4087661f0e2..6e8f1589137f00 100644 --- a/src/cmd/go/internal/vcweb/script.go +++ b/src/cmd/go/internal/vcweb/script.go @@ -16,6 +16,7 @@ import ( "log" "net/http" "os" + "os/exec" "path/filepath" "runtime" "strconv" @@ -31,7 +32,7 @@ import ( func newScriptEngine() *script.Engine { conds := script.DefaultConds() - interrupt := os.Interrupt + interrupt := func(cmd *exec.Cmd) error { return cmd.Process.Signal(os.Interrupt) } gracePeriod := 1 * time.Second // arbitrary cmds := script.DefaultCmds() diff --git a/src/cmd/go/scriptcmds_test.go b/src/cmd/go/scriptcmds_test.go index 2a9900782ba5e7..db5e6cafdafe51 100644 --- a/src/cmd/go/scriptcmds_test.go +++ b/src/cmd/go/scriptcmds_test.go @@ -11,15 +11,23 @@ import ( "errors" "fmt" "os" + "os/exec" "strings" "time" ) -func scriptCommands(interrupt os.Signal, gracePeriod time.Duration) map[string]script.Cmd { +func scriptCommands(interrupt os.Signal, waitDelay time.Duration) map[string]script.Cmd { cmds := scripttest.DefaultCmds() // Customize the "exec" interrupt signal and grace period. - cmdExec := script.Exec(quitSignal(), gracePeriod) + var cancel func(cmd *exec.Cmd) error + if interrupt != nil { + cancel = func(cmd *exec.Cmd) error { + return cmd.Process.Signal(interrupt) + } + } + + cmdExec := script.Exec(cancel, waitDelay) cmds["exec"] = cmdExec add := func(name string, cmd script.Cmd) { @@ -30,7 +38,7 @@ func scriptCommands(interrupt os.Signal, gracePeriod time.Duration) map[string]s } add("cc", scriptCC(cmdExec)) - cmdGo := scriptGo(interrupt, gracePeriod) + cmdGo := scriptGo(cancel, waitDelay) add("go", cmdGo) add("stale", scriptStale(cmdGo)) @@ -62,8 +70,8 @@ func scriptCC(cmdExec script.Cmd) script.Cmd { } // scriptGo runs the go command. -func scriptGo(interrupt os.Signal, gracePeriod time.Duration) script.Cmd { - return script.Program(testGo, interrupt, gracePeriod) +func scriptGo(cancel func(*exec.Cmd) error, waitDelay time.Duration) script.Cmd { + return script.Program(testGo, cancel, waitDelay) } // scriptStale checks that the named build targets are stale.