From 3d77a0b15ea2a6d2f7b3e2ba483f148d7c6ee174 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Tue, 9 Dec 2025 10:06:23 -0500 Subject: [PATCH] os/exec: second call to Cmd.Start is always an error Previously it would return an error only if the first call resulted in process creation, contra the intent of the comment at exec.Cmd: // A Cmd cannot be reused after calling its [Cmd.Start], [Cmd.Run], // [Cmd.Output], or [Cmd.CombinedOutput] methods. Also, clear the Cmd.goroutines slice in case of failure to start a process, so that the closures can be GC'd and their pipe fds finalized and closed. Fixes #76746 Change-Id: Ic63a4dced0aa52c2d4be7d44f6dcfc84ee22282c Reviewed-on: https://go-review.googlesource.com/c/go/+/728642 LUCI-TryBot-Result: Go LUCI Reviewed-by: Damien Neil --- src/os/exec/exec.go | 8 +++++++- src/os/exec/exec_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/os/exec/exec.go b/src/os/exec/exec.go index e84ebfc453..e0c509cbef 100644 --- a/src/os/exec/exec.go +++ b/src/os/exec/exec.go @@ -102,6 +102,7 @@ import ( "runtime" "strconv" "strings" + "sync/atomic" "syscall" "time" ) @@ -354,6 +355,9 @@ type Cmd struct { // the work of resolving the extension, so Start doesn't need to do it again. // This is only used on Windows. cachedLookExtensions struct{ in, out string } + + // startCalled records that Start was attempted, regardless of outcome. + startCalled atomic.Bool } // A ctxResult reports the result of watching the Context associated with a @@ -635,7 +639,8 @@ func (c *Cmd) Run() error { func (c *Cmd) Start() error { // Check for doubled Start calls before we defer failure cleanup. If the prior // call to Start succeeded, we don't want to spuriously close its pipes. - if c.Process != nil { + // It is an error to call Start twice even if the first call did not create a process. + if c.startCalled.Swap(true) { return errors.New("exec: already started") } @@ -647,6 +652,7 @@ func (c *Cmd) Start() error { if !started { closeDescriptors(c.parentIOPipes) c.parentIOPipes = nil + c.goroutine = nil // aid GC, finalization of pipe fds } }() diff --git a/src/os/exec/exec_test.go b/src/os/exec/exec_test.go index 1decebdc22..bf2f3da535 100644 --- a/src/os/exec/exec_test.go +++ b/src/os/exec/exec_test.go @@ -1839,3 +1839,29 @@ func TestAbsPathExec(t *testing.T) { } }) } + +// Calling Start twice is an error, regardless of outcome. +func TestStart_twice(t *testing.T) { + testenv.MustHaveExec(t) + + cmd := exec.Command("/bin/nonesuch") + for i, want := range []string{ + cond(runtime.GOOS == "windows", + `exec: "/bin/nonesuch": executable file not found in %PATH%`, + "fork/exec /bin/nonesuch: no such file or directory"), + "exec: already started", + } { + err := cmd.Start() + if got := fmt.Sprint(err); got != want { + t.Errorf("Start call #%d return err %q, want %q", i+1, got, want) + } + } +} + +func cond[T any](cond bool, t, f T) T { + if cond { + return t + } else { + return f + } +} -- 2.52.0