os/exec: do not close pipes on a double-Start error

This fixes a bug introduced in CL 401834 in which calling Start twice
with pipes attached to a command would spuriously close those pipes
when returning the error from the second Start call.

For #50436.

Change-Id: I3563cc99c0a0987752190fef95da3e9927a76fda
Reviewed-on: https://go-review.googlesource.com/c/go/+/436095
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>

1 files changed, 43 insertions, 0 deletions

diff --git a/src/os/exec/exec_test.go b/src/os/exec/exec_test.go
index 52001bf9e3..07ac0cf3d4 100644
--- a/src/os/exec/exec_test.go
+++ b/src/os/exec/exec_test.go
@@ -1094,3 +1094,46 @@ func TestNoPath(t *testing.T) {
 		t.Errorf("new(Cmd).Start() = %v, want %q", err, want)
 	}
 }
+
+// TestDoubleStartLeavesPipesOpen checks for a regression in which calling
+// Start twice, which returns an error on the second call, would spuriously
+// close the pipes established in the first call.
+func TestDoubleStartLeavesPipesOpen(t *testing.T) {
+	cmd := helperCommand(t, "pipetest")
+	in, err := cmd.StdinPipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	out, err := cmd.StdoutPipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := cmd.Start(); err != nil {
+		t.Fatal(err)
+	}
+	if err := cmd.Start(); err == nil || !strings.HasSuffix(err.Error(), "already started") {
+		t.Fatalf("second call to Start returned a nil; want an 'already started' error")
+	}
+
+	outc := make(chan []byte, 1)
+	go func() {
+		b, err := io.ReadAll(out)
+		if err != nil {
+			t.Error(err)
+		}
+		outc <- b
+	}()
+
+	const msg = "O:Hello, pipe!\n"
+
+	_, err = io.WriteString(in, msg)
+	if err != nil {
+		t.Fatal(err)
+	}
+	in.Close()
+
+	b := <-outc
+	if !bytes.Equal(b, []byte(msg)) {
+		t.Fatalf("read %q from stdout pipe; want %q", b, msg)
+	}
+}