Prevent I/O deadlocks during shell testing

If the shell outputs more data than can be buffered the test will
deadlock. I noticed this when working on issue #661. The tests I was
writing would deadlock because a tty can buffer significantly less data
than a pipe. Even using a pipe, which typically buffers 8 to 128 KiB, this
is theoretically a problem. So use a goroutine to capture the output as it
is generated rather than reading it all at once when the test terminates.

pkg/prog/progtest/progtest.go

@@ -19,11 +19,20 @@ type Fixture struct {
 	dirCleanup func()
 }
 
+func captureOutput(p *pipe) {
+	b, err := ioutil.ReadAll(p.r)
+	if err != nil {
+		panic(err)
+	}
+	p.output <- b
+}
+
 // Setup sets up a test fixture. The caller is responsible for calling the
 // Cleanup method of the returned Fixture.
 func Setup() *Fixture {
 	_, dirCleanup := util.InTestDir()
-	return &Fixture{[3]*pipe{makePipe(), makePipe(), makePipe()}, dirCleanup}
+	pipes := [3]*pipe{makePipe(false), makePipe(true), makePipe(true)}
+	return &Fixture{pipes, dirCleanup}
 }
 
 // Cleanup cleans up the test fixture.
@@ -46,6 +55,7 @@ func (f *Fixture) FeedIn(s string) {
 		panic(err)
 	}
 	f.pipes[0].w.Close()
+	f.pipes[0].wClosed = true
 }
 
 // TestOut tests that the output on the given FD matches the given text.
@@ -68,29 +78,33 @@ type pipe struct {
 	r, w             *os.File
 	rClosed, wClosed bool
 	saved            string
+	output           chan []byte
 }
 
-func makePipe() *pipe {
+func makePipe(capture bool) *pipe {
 	r, w, err := os.Pipe()
 	if err != nil {
 		panic(err)
 	}
-	return &pipe{r: r, w: w}
+	if !capture {
+		return &pipe{r: r, w: w}
+	}
+	output := make(chan []byte, 1)
+	p := pipe{r: r, w: w, output: output}
+	go captureOutput(&p)
+	return &p
 }
 
 func (p *pipe) get() string {
-	if p.rClosed {
-		return p.saved
+	if !p.wClosed {
+		// Close the write side so captureOutput goroutine sees EOF and
+		// terminates allowing us to capture and cache the output.
+		p.w.Close()
+		p.wClosed = true
+		if p.output != nil {
+			p.saved = string(<-p.output)
+		}
 	}
-	p.w.Close()
-	p.wClosed = true
-	b, err := ioutil.ReadAll(p.r)
-	if err != nil {
-		panic(err)
-	}
-	p.r.Close()
-	p.rClosed = true
-	p.saved = string(b)
 	return p.saved
 }
 
@@ -98,11 +112,18 @@ func (p *pipe) close() {
 	if !p.wClosed {
 		p.w.Close()
 		p.wClosed = true
+		if p.output != nil {
+			p.saved = string(<-p.output)
+		}
 	}
 	if !p.rClosed {
 		p.r.Close()
 		p.rClosed = true
 	}
+	if p.output != nil {
+		close(p.output)
+		p.output = nil
+	}
 }
 
 // MustWriteFile writes a file with the given name and content. It panics if the

pkg/prog/progtest/progtest_test.go

@@ -0,0 +1,23 @@
+package progtest
+
+import (
+	"testing"
+)
+
+// Verify we don't deadlock if more output is written to stdout than can be
+// buffered by a pipe.
+func TestOutputCaptureDoesNotDeadlock(t *testing.T) {
+	t.Helper()
+	f := Setup()
+
+	// We need enough data to verify whether we're likely to deadlock due to
+	// filling the pipe before the test completes. Pipes typically buffer 8 to
+	// 128 KiB.
+	bytes := [16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}
+	for i := 0; i < 128*1024/len(bytes); i += 1 {
+		f.pipes[1].w.Write(bytes[:])
+	}
+	f.pipes[1].w.WriteString("hello\n")
+	f.TestOutSnippet(t, 1, "hello")
+	f.TestOut(t, 2, "")
+}