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

Fixes issue 4083.

R=golang-dev, r
CC=golang-dev
http://codereview.appspot.com/6498136

2 files changed, 38 insertions, 22 deletions

diff --git a/misc/dashboard/builder/exec.go b/misc/dashboard/builder/exec.go
index 802d5f079..6ebe7b8bf 100644
--- 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 68acb0600..59e0f3c1d 100644
--- 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,