Skip to content

Commit

Permalink
don't call into CombinedOutput for subprocesses
Browse files Browse the repository at this point in the history
The documentation for `exec.Cmd` says this about `Stdout` and `Stderr`:

    // If either is an *os.File, the corresponding output from the process
    // is connected directly to that file.
    //
    // Otherwise, during the execution of the command a separate goroutine
    // reads from the process over a pipe and delivers that data to the
    // corresponding Writer. In this case, Wait does not complete until the
    // goroutine reaches EOF or encounters an error.

When calling `CombinedOutput()`, `Stdout` and `Stderr` are
`bytes.Buffer`s and are therefore not `*os.File`s so they fall into
this second group. This resulted in a race condition where cancelling
the context when `maxtime` has passed could cause `CombinedOutput()` to
hang indefinitely waiting for the (finished) subprocess to "finish"
writing to its pipes.

This has been reported as an issue several times. The tracking issue is
golang/go#23019 which itself links to several
other issues that are duplicates.

To work around the issue we simply force the other behavior by creating
a temporary `*os.File` for the combined `stdout` and `stderr`.
  • Loading branch information
rickystewart committed Mar 8, 2022
1 parent 43d99a9 commit d14e8fa
Showing 1 changed file with 25 additions and 4 deletions.
29 changes: 25 additions & 4 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,12 +187,33 @@ func run() error {
}).Stop()
}
}
combinedOutput, err := ioutil.TempFile("", "stress-stdouterr")
if err != nil {
panic(err)
}
defer func() { _ = os.Remove(combinedOutput.Name()) }()
cmd = exec.CommandContext(ctx, flags.Args()[0], flags.Args()[1:]...)
cmd.Env = subenviron
out, err := cmd.CombinedOutput()
result.output = out
if err != nil && (failureRe == nil || failureRe.Match(out)) && (ignoreRe == nil || !ignoreRe.Match(out)) {
result.output = append(out, fmt.Sprintf("\n\nERROR: %v\n", err)...)
cmd.Stdout = combinedOutput
cmd.Stderr = combinedOutput
cmdErr := cmd.Run()
_, err = combinedOutput.Seek(0, 0)
if err != nil {
result.output = []byte("stress: could not seek to beginning of stdout/stderr for test")
} else {
out, err := ioutil.ReadAll(combinedOutput)
if err != nil {
result.output = []byte("stress: could not read stdout/stderr for test")
} else {
result.output = out
}
}
err = combinedOutput.Close()
if err != nil {
panic(err)
}
if cmdErr != nil && (failureRe == nil || failureRe.Match(result.output)) && (ignoreRe == nil || !ignoreRe.Match(result.output)) {
result.output = append(result.output, fmt.Sprintf("\n\nERROR: %v\n", err)...)
} else {
result.success = true
}
Expand Down

0 comments on commit d14e8fa

Please sign in to comment.