os/exec: avoid atomic.Bool for Cmd.startCalled

An atomic.Bool isn't necessary here since, unless otherwise
specified, the methods of an object are not concurrency-safe
w.r.t. each other. Using an atomic causes the copylocks vet
check to warn about copying of Cmd, which is not wrong, because
one shouldn't be copying opaque complex structs from other
packages, but it is a nuisance in the absence of any safe way
to copy a Cmd.

If and when we add a Clone method to Cmd (see #77075) then
it would be appropriate to revert this change so that we get
the benefit of the static check (though ideally we would make
a more explicit tool-readable declaration of the "do not copy"
attribute than merely happening to use an atomic.Bool).

For #77075

Change-Id: I982d4e86623ca165a3e76bbf648fd44041d5f6bb
Reviewed-on: https://go-review.googlesource.com/c/go/+/734200
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This commit is contained in:
Alan Donovan 2026-01-06 14:48:31 -05:00
parent 9b2e3b9a02
commit d1d0fc7a97

View file

@ -357,7 +357,9 @@ type Cmd struct {
cachedLookExtensions struct{ in, out string }
// startCalled records that Start was attempted, regardless of outcome.
startCalled atomic.Bool
// (Until go.dev/issue/77075 is resolved, we use atomic.SwapInt32,
// not atomic.Bool.Swap, to avoid triggering the copylocks vet check.)
startCalled int32
}
// A ctxResult reports the result of watching the Context associated with a
@ -640,7 +642,7 @@ 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.
// It is an error to call Start twice even if the first call did not create a process.
if c.startCalled.Swap(true) {
if atomic.SwapInt32(&c.startCalled, 1) != 0 {
return errors.New("exec: already started")
}