Browse code

Fix race in RunCommandWithOutputForDuration

This function was starting a goroutine that modifies one of its return
values. The intent is for the goroutine to only influence the return
value when it's causing the function to return, but it's racy and can
also modify the return value when the function is returning due to the
timeout. Fix the goroutine to not modify return values directly.

Also, give the channel a buffer so that the goroutine doesn't block
forever after a timeout.

Fixes #18305

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>

Aaron Lehmann authored on 2015/12/04 09:04:58
Showing 1 changed files
... ...
@@ -104,19 +104,25 @@ func RunCommandWithOutputForDuration(cmd *exec.Cmd, duration time.Duration) (out
104 104
 	}
105 105
 	cmd.Stderr = &outputBuffer
106 106
 
107
-	done := make(chan error)
108
-
109 107
 	// Start the command in the main thread..
110 108
 	err = cmd.Start()
111 109
 	if err != nil {
112 110
 		err = fmt.Errorf("Fail to start command %v : %v", cmd, err)
113 111
 	}
114 112
 
113
+	type exitInfo struct {
114
+		exitErr  error
115
+		exitCode int
116
+	}
117
+
118
+	done := make(chan exitInfo, 1)
119
+
115 120
 	go func() {
116 121
 		// And wait for it to exit in the goroutine :)
117
-		exitErr := cmd.Wait()
118
-		exitCode = ProcessExitCode(exitErr)
119
-		done <- exitErr
122
+		info := exitInfo{}
123
+		info.exitErr = cmd.Wait()
124
+		info.exitCode = ProcessExitCode(info.exitErr)
125
+		done <- info
120 126
 	}()
121 127
 
122 128
 	select {
... ...
@@ -126,9 +132,9 @@ func RunCommandWithOutputForDuration(cmd *exec.Cmd, duration time.Duration) (out
126 126
 			fmt.Printf("failed to kill (pid=%d): %v\n", cmd.Process.Pid, killErr)
127 127
 		}
128 128
 		timedOut = true
129
-		break
130
-	case err = <-done:
131
-		break
129
+	case info := <-done:
130
+		err = info.exitErr
131
+		exitCode = info.exitCode
132 132
 	}
133 133
 	output = outputBuffer.String()
134 134
 	return