misc/dashboard/builder: add timeout to all external command invocations

Fixes #4083.

R=golang-dev, r
CC=golang-dev
https://golang.org/cl/6498136

diff --git a/misc/dashboard/builder/exec.go b/misc/dashboard/builder/exec.go
index 802d5f079f6a8be6a961a6638731b0b298b69fc6..6ebe7b8bf4748bf68797e3c28089a14ebb800c4e 100644 (file)
--- a/misc/dashboard/builder/exec.go
+++ b/misc/dashboard/builder/exec.go
@@ -6,14 +6,16 @@ package main
 
 import (
        "bytes"
+       "fmt"
        "io"
        "log"
        "os"
        "os/exec"
+       "time"
 )
 
 // run is a simple wrapper for exec.Run/Close
-func run(envv []string, dir string, argv ...string) error {
+func run(timeout time.Duration, envv []string, dir string, argv ...string) error {
        if *verbose {
                log.Println("run", argv)
        }
@@ -21,7 +23,10 @@ func run(envv []string, dir string, argv ...string) error {
        cmd.Dir = dir
        cmd.Env = envv
        cmd.Stderr = os.Stderr
-       return cmd.Run()
+       if err := cmd.Start(); err != nil {
+               return err
+       }
+       return waitWithTimeout(timeout, cmd)
 }
 
 // runLog runs a process and returns the combined stdout/stderr, 
@@ -29,7 +34,7 @@ func run(envv []string, dir string, argv ...string) error {
 // process combined stdout and stderr output, exit status and error.
 // The error returned is nil, if process is started successfully,
 // even if exit status is not successful.
-func runLog(envv []string, logfile, dir string, argv ...string) (string, int, error) {
+func runLog(timeout time.Duration, envv []string, logfile, dir string, argv ...string) (string, int, error) {
        if *verbose {
                log.Println("runLog", argv)
        }
@@ -56,8 +61,23 @@ func runLog(envv []string, logfile, dir string, argv ...string) (string, int, er
                return "", 1, startErr
        }
        exitStatus := 0
-       if err := cmd.Wait(); err != nil {
+       if err := waitWithTimeout(timeout, cmd); err != nil {
                exitStatus = 1 // TODO(bradfitz): this is fake. no callers care, so just return a bool instead.
        }
        return b.String(), exitStatus, nil
 }
+
+func waitWithTimeout(timeout time.Duration, cmd *exec.Cmd) error {
+       errc := make(chan error, 1)
+       go func() {
+               errc <- cmd.Wait()
+       }()
+       var err error
+       select {
+       case <-time.After(timeout):
+               cmd.Process.Kill()
+               err = fmt.Errorf("timed out after %v", timeout)
+       case err = <-errc:
+       }
+       return err
+}

diff --git a/misc/dashboard/builder/main.go b/misc/dashboard/builder/main.go
index 68acb0600bb2945560952b0d4aefcfe17921847c..59e0f3c1dbaa5fc60c90e7fdaea48ba823f96b21 100644 (file)
--- a/misc/dashboard/builder/main.go
+++ b/misc/dashboard/builder/main.go
@@ -54,6 +54,8 @@ var (
        buildCmd      = flag.String("cmd", filepath.Join(".", allCmd), "Build command (specify relative to go/src/)")
        failAll       = flag.Bool("fail", false, "fail all builds")
        parallel      = flag.Bool("parallel", false, "Build multiple targets in parallel")
+       buildTimeout  = flag.Duration("buildTimeout", 60*time.Minute, "Maximum time to wait for builds and tests")
+       cmdTimeout    = flag.Duration("cmdTimeout", 2*time.Minute, "Maximum time to wait for an external command")
        verbose       = flag.Bool("v", false, "verbose")
 )
 
@@ -220,7 +222,7 @@ func (b *Builder) build() bool {
        // Look for hash locally before running hg pull.
        if _, err := fullHash(goroot, hash[:12]); err != nil {
                // Don't have hash, so run hg pull.
-               if err := run(nil, goroot, hgCmd("pull")...); err != nil {
+               if err := run(*cmdTimeout, nil, goroot, hgCmd("pull")...); err != nil {
                        log.Println("hg pull failed:", err)
                        return false
                }
@@ -243,12 +245,12 @@ func (b *Builder) buildHash(hash string) error {
        defer os.RemoveAll(workpath)
 
        // clone repo
-       if err := run(nil, workpath, hgCmd("clone", goroot, "go")...); err != nil {
+       if err := run(*cmdTimeout, nil, workpath, hgCmd("clone", goroot, "go")...); err != nil {
                return err
        }
 
        // update to specified revision
-       if err := run(nil, filepath.Join(workpath, "go"), hgCmd("update", hash)...); err != nil {
+       if err := run(*cmdTimeout, nil, filepath.Join(workpath, "go"), hgCmd("update", hash)...); err != nil {
                return err
        }
 
@@ -261,7 +263,7 @@ func (b *Builder) buildHash(hash string) error {
                cmd = filepath.Join(srcDir, cmd)
        }
        startTime := time.Now()
-       buildLog, status, err := runLog(b.envv(), logfile, srcDir, cmd)
+       buildLog, status, err := runLog(*buildTimeout, b.envv(), logfile, srcDir, cmd)
        runTime := time.Now().Sub(startTime)
        if err != nil {
                return fmt.Errorf("%s: %s", *buildCmd, err)
@@ -353,28 +355,22 @@ func (b *Builder) buildSubrepo(goRoot, pkg, hash string) (string, error) {
        }
 
        // fetch package and dependencies
-       log, status, err := runLog(env, "", goRoot, goTool, "get", "-d", pkg)
+       log, status, err := runLog(*cmdTimeout, env, "", goRoot, goTool, "get", "-d", pkg)
        if err == nil && status != 0 {
                err = fmt.Errorf("go exited with status %d", status)
        }
        if err != nil {
-               // 'go get -d' will fail for a subrepo because its top-level
-               // directory does not contain a go package. No matter, just
-               // check whether an hg directory exists and proceed.
-               hgDir := filepath.Join(goRoot, "src/pkg", pkg, ".hg")
-               if fi, e := os.Stat(hgDir); e != nil || !fi.IsDir() {
-                       return log, err
-               }
+               return log, err
        }
 
        // hg update to the specified hash
        pkgPath := filepath.Join(goRoot, "src/pkg", pkg)
-       if err := run(nil, pkgPath, hgCmd("update", hash)...); err != nil {
+       if err := run(*cmdTimeout, nil, pkgPath, hgCmd("update", hash)...); err != nil {
                return "", err
        }
 
        // test the package
-       log, status, err = runLog(env, "", goRoot, goTool, "test", "-short", pkg+"/...")
+       log, status, err = runLog(*buildTimeout, env, "", goRoot, goTool, "test", "-short", pkg+"/...")
        if err == nil && status != 0 {
                err = fmt.Errorf("go exited with status %d", status)
        }
@@ -475,7 +471,7 @@ func commitWatcher() {
 }
 
 func hgClone(url, path string) error {
-       return run(nil, *buildroot, hgCmd("clone", url, path)...)
+       return run(*cmdTimeout, nil, *buildroot, hgCmd("clone", url, path)...)
 }
 
 func hgRepoExists(path string) bool {
@@ -532,14 +528,14 @@ func commitPoll(key, pkg string) {
                }
        }
 
-       if err := run(nil, pkgRoot, hgCmd("pull")...); err != nil {
+       if err := run(*cmdTimeout, nil, pkgRoot, hgCmd("pull")...); err != nil {
                log.Printf("hg pull: %v", err)
                return
        }
 
        const N = 50 // how many revisions to grab
 
-       data, _, err := runLog(nil, "", pkgRoot, hgCmd("log",
+       data, _, err := runLog(*cmdTimeout, nil, "", pkgRoot, hgCmd("log",
                "--encoding=utf-8",
                "--limit="+strconv.Itoa(N),
                "--template="+xmlLogTemplate)...,
@@ -627,7 +623,7 @@ func addCommit(pkg, hash, key string) bool {
 
 // fullHash returns the full hash for the given Mercurial revision.
 func fullHash(root, rev string) (string, error) {
-       s, _, err := runLog(nil, "", root,
+       s, _, err := runLog(*cmdTimeout, nil, "", root,
                hgCmd("log",
                        "--encoding=utf-8",
                        "--rev="+rev,