Avoid leaking pooled-stdout goroutine when cmd.Start() fails

Ensure that in case of cmd.Start() failure for pipelines that use pooled
buffers, we don't leak a pooled-buffer-copy goroutine.

Could be triggered/detected by using a command stage that fails on
command-not-found under the race detector.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: znull

pipe/command.go

@@ -162,6 +162,19 @@ func (s *commandStage) Start(
 		}
 	}
 
+	closeEarlyClosers := func() {
+		for _, closer := range earlyClosers {
+			_ = closer.Close()
+		}
+	}
+
+	// On error, Close any pipes we created and wait for the goroutines to
+	// exit before propagating the error.
+	cleanupOnStartFailure := func() {
+		closeEarlyClosers()
+		_ = s.wg.Wait()
+	}
+
 	// If the caller hasn't arranged otherwise, read the command's
 	// standard error into our `stderr` field:
 	if s.cmd.Stderr == nil {
@@ -171,6 +184,7 @@ func (s *commandStage) Start(
 		// can be sure.
 		p, err := s.cmd.StderrPipe()
 		if err != nil {
+			cleanupOnStartFailure()
 			return err
 		}
 		s.wg.Go(func() error {
@@ -188,12 +202,11 @@ func (s *commandStage) Start(
 	s.runInOwnProcessGroup()
 
 	if err := s.cmd.Start(); err != nil {
+		cleanupOnStartFailure()
 		return err
 	}
 
-	for _, closer := range earlyClosers {
-		_ = closer.Close()
-	}
+	closeEarlyClosers()
 
 	// Arrange for the process to be killed (gently) if the context
 	// expires before the command exits normally:

pipe/command_starterror_test.go

@@ -0,0 +1,30 @@
+package pipe_test
+
+import (
+	"bytes"
+	"context"
+	"os/exec"
+	"testing"
+
+	"github.com/github/go-pipe/pipe"
+)
+
+// TestCommandStageStartFailureNoRace verifies that when `cmd.Start()`
+// fails (e.g. command not found), the goroutine that
+// `setupPooledStdout` spawned does not leak past `Pipeline.Run()`.
+// `bytes.Buffer.ReadFrom` writes to the buffer's slice header via
+// `grow()` before its first `Read()`, so a leaked goroutine races
+// with the caller's access to the destination buffer once Run
+// returns the error. Run a tight loop so `-race` is likely to catch
+// any regression.
+func TestCommandStageStartFailureNoRace(t *testing.T) {
+	for i := 0; i < 50; i++ {
+		var buf bytes.Buffer
+		p := pipe.New(pipe.WithStdout(&buf))
+		p.Add(pipe.CommandStage("nope", exec.Command("this-binary-does-not-exist-xyz123")))
+		if err := p.Run(context.Background()); err == nil {
+			t.Fatalf("expected error from non-existent command, got nil")
+		}
+		_ = buf.String()
+	}
+}