Browse code

Fix a deadlock in CmdStream.

As per FIXME, CmdStream could have deadlocked if a command printed
enough on stderr. This commit fixes that, but still keeps all of
the stderr output in memory.

Robert Obryk authored on 2013/03/29 20:42:17
Showing 2 changed files
... ...
@@ -52,17 +52,21 @@ func CmdStream(cmd *exec.Cmd) (io.Reader, error) {
52 52
 		return nil, err
53 53
 	}
54 54
 	pipeR, pipeW := io.Pipe()
55
+	errChan := make(chan []byte)
55 56
 	go func() {
56
-		_, err := io.Copy(pipeW, stdout)
57
-		if err != nil {
58
-			pipeW.CloseWithError(err)
59
-		}
60 57
 		errText, e := ioutil.ReadAll(stderr)
61 58
 		if e != nil {
62 59
 			errText = []byte("(...couldn't fetch stderr: " + e.Error() + ")")
63 60
 		}
61
+		errChan <- errText
62
+	}()
63
+	go func() {
64
+		_, err := io.Copy(pipeW, stdout)
65
+		if err != nil {
66
+			pipeW.CloseWithError(err)
67
+		}
68
+		errText := <-errChan
64 69
 		if err := cmd.Wait(); err != nil {
65
-			// FIXME: can this block if stderr outputs more than the size of StderrPipe()'s buffer?
66 70
 			pipeW.CloseWithError(errors.New(err.Error() + ": " + string(errText)))
67 71
 		} else {
68 72
 			pipeW.Close()
... ...
@@ -1,12 +1,26 @@
1 1
 package docker
2 2
 
3 3
 import (
4
+	"io"
4 5
 	"io/ioutil"
5 6
 	"os"
6 7
 	"os/exec"
7 8
 	"testing"
8 9
 )
9 10
 
11
+func TestCmdStreamLargeStderr(t *testing.T) {
12
+	// This test checks for deadlock; thus, the main failure mode of this test is deadlocking.
13
+	cmd := exec.Command("/bin/sh", "-c", "dd if=/dev/zero bs=1k count=1000 of=/dev/stderr; echo hello")
14
+	out, err := CmdStream(cmd)
15
+	if err != nil {
16
+		t.Fatalf("Failed to start command: " + err.Error())
17
+	}
18
+	_, err = io.Copy(ioutil.Discard, out)
19
+	if err != nil {
20
+		t.Fatalf("Command should not have failed (err=%s...)", err.Error()[:100])
21
+	}
22
+}
23
+
10 24
 func TestCmdStreamBad(t *testing.T) {
11 25
 	badCmd := exec.Command("/bin/sh", "-c", "echo hello; echo >&2 error couldn\\'t reverse the phase pulser; exit 1")
12 26
 	out, err := CmdStream(badCmd)