From d14e8fa1a164e6321d58f06f02ae9d88b2b56ba6 Mon Sep 17 00:00:00 2001
From: Ricky Stewart <ricky@cockroachlabs.com>
Date: Tue, 8 Mar 2022 10:09:52 -0600
Subject: [PATCH] don't call into `CombinedOutput` for subprocesses

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
https://github.com/golang/go/issues/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`.
---
 main.go | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/main.go b/main.go
index 9fbd0450..cac5fda6 100644
--- a/main.go
+++ b/main.go
@@ -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
 					}