fix(race-conditon): fix data race condition when accessing stderr on pipe

.github/workflows/ci.yml

@@ -13,7 +13,7 @@ jobs:
         with:
           go-version: ${{ matrix.go-version }}
       - uses: actions/checkout@v3
-      - run: go test ./...
+      - run: go test -race ./...
 
   gocritic:
     runs-on: ubuntu-latest

script.go

@@ -31,7 +31,7 @@ type Pipe struct {
 	stdout, stderr io.Writer
 	httpClient     *http.Client
 
-	// because pipe stages are concurrent, protect 'err'
+	// because pipe stages are concurrent, protect 'err' and 'stderr'
 	mu  *sync.Mutex
 	err error
 }
@@ -385,8 +385,9 @@ func (p *Pipe) Exec(cmdLine string) *Pipe {
 		cmd.Stdin = r
 		cmd.Stdout = w
 		cmd.Stderr = w
-		if p.stderr != nil {
-			cmd.Stderr = p.stderr
+		pipeStderr := p.getStderr()
+		if pipeStderr != nil {
+			cmd.Stderr = pipeStderr
 		}
 		err = cmd.Start()
 		if err != nil {
@@ -425,8 +426,9 @@ func (p *Pipe) ExecForEach(cmdLine string) *Pipe {
 			cmd := exec.Command(args[0], args[1:]...)
 			cmd.Stdout = w
 			cmd.Stderr = w
-			if p.stderr != nil {
-				cmd.Stderr = p.stderr
+			pipeStderr := p.getStderr()
+			if pipeStderr != nil {
+				cmd.Stderr = pipeStderr
 			}
 			err = cmd.Start()
 			if err != nil {
@@ -762,6 +764,30 @@ func (p *Pipe) SetError(err error) {
 	p.err = err
 }
 
+// setStderr sets the stderr writer on the pipe. This field
+// is protected by a mutex since stderr is accessed inside a
+// goroutine from [Pipe.Exec].
+func (p *Pipe) setStderr(stderr io.Writer) {
+	if p.mu == nil { // uninitialised pipe
+		return
+	}
+	p.mu.Lock()
+	defer p.mu.Unlock()
+	p.stderr = stderr
+}
+
+// getStderr obtains the stderr writer on the pipe. This field
+// is protected by a mutex since stderr is accessed inside a
+// goroutine from [Pipe.Exec].
+func (p *Pipe) getStderr() io.Writer {
+	if p.mu == nil { // uninitialised pipe
+		return nil
+	}
+	p.mu.Lock()
+	defer p.mu.Unlock()
+	return p.stderr
+}
+
 // SHA256Sum returns the hex-encoded SHA-256 hash of the entire contents of the
 // pipe, or an error.
 func (p *Pipe) SHA256Sum() (string, error) {
@@ -887,7 +913,7 @@ func (p *Pipe) WithReader(r io.Reader) *Pipe {
 // [Pipe.Exec] or [Pipe.ExecForEach] to the writer w, instead of going to the
 // pipe as it normally would.
 func (p *Pipe) WithStderr(w io.Writer) *Pipe {
-	p.stderr = w
+	p.setStderr(w)
 	return p
 }
 

script_test.go

@@ -1850,6 +1850,19 @@ func TestReadReturnsErrorGivenReadErrorOnPipe(t *testing.T) {
 	}
 }
 
+// TestWithStdErrAfterExec is a regression test that was added to test against
+// a race condition for Pipe.stderr.
+func TestWithStdErrAfterExec(t *testing.T) {
+	t.Parallel()
+	stdOut := new(bytes.Buffer)
+	stdErr := new(bytes.Buffer)
+
+	_, err := script.Exec("echo").WithStdout(stdOut).WithStderr(stdErr).Stdout()
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
 func ExampleArgs() {
 	script.Args().Stdout()
 	// prints command-line arguments