close_issue_1
/
tools
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

dashboard/builder: fix bugs 

1) Killing proceses on timeout was wrong: the os/exec
package will never return our package's error type
from its Wait.

2) fix a goroutine leak on timeout.

3) unexported an undocumented and elsewhere-unused type.

4) rename timeout type to end in "Error", per convention,
not Err.

LGTM=adg
R=adg
CC=golang-codereviews
https://golang.org/cl/74290043
This commit is contained in:
Brad Fitzpatrick 2014-03-11 18:13:26 -07:00
parent dc0772a41e
commit 305a363bdd
1 changed files with 12 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
dashboard/builder/exec.go
View File

@ -26,15 +26,11 @@ func run(d time.Duration, envv []string, dir string, argv ...string) error {
if err := cmd.Start(); err != nil { if err := cmd.Start(); err != nil {
return err return err
} }
return timeout(d, func() error { err := timeout(d, cmd.Wait)
if err := cmd.Wait(); err != nil { if _, ok := err.(timeoutError); ok {
if _, ok := err.(TimeoutErr); ok { cmd.Process.Kill()
cmd.Process.Kill() }
} return err
return err
}
return nil
})
} }
// runLog runs a process and returns the combined stdout/stderr. It returns // runLog runs a process and returns the combined stdout/stderr. It returns
@ -66,15 +62,10 @@ func runOutput(d time.Duration, envv []string, out io.Writer, dir string, argv .
return false, startErr return false, startErr
} }
if err := timeout(d, func() error { if err := timeout(d, cmd.Wait); err != nil {
if err := cmd.Wait(); err != nil { if _, ok := err.(timeoutError); ok {
if _, ok := err.(TimeoutErr); ok { cmd.Process.Kill()
cmd.Process.Kill()
}
return err
} }
return nil
}); err != nil {
return false, err return false, err
} }
return true, nil return true, nil
@ -83,7 +74,7 @@ func runOutput(d time.Duration, envv []string, out io.Writer, dir string, argv .
// timeout runs f and returns its error value, or if the function does not // timeout runs f and returns its error value, or if the function does not
// complete before the provided duration it returns a timeout error. // complete before the provided duration it returns a timeout error.
func timeout(d time.Duration, f func() error) error { func timeout(d time.Duration, f func() error) error {
errc := make(chan error) errc := make(chan error, 1)
go func() { go func() {
errc <- f() errc <- f()
}() }()
@ -91,14 +82,14 @@ func timeout(d time.Duration, f func() error) error {
defer t.Stop() defer t.Stop()
select { select {
case <-t.C: case <-t.C:
return fmt.Errorf("timed out after %v", d) return timeoutError(d)
case err := <-errc: case err := <-errc:
return err return err
} }
} }
type TimeoutErr time.Duration type timeoutError time.Duration
func (e TimeoutErr) Error() string { func (e timeoutError) Error() string {
return fmt.Sprintf("timed out after %v", time.Duration(e)) return fmt.Sprintf("timed out after %v", time.Duration(e))
} }