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>

diff --git a/src/os/exec/exec.go b/src/os/exec/exec.go
index e0c509cbefe59841ffe4e4fb5fd328ecc1a91072..aa7a6be7f07c1c578952cfe0be3cc8aae2dace42 100644 (file)
--- a/src/os/exec/exec.go
+++ b/src/os/exec/exec.go
@@ -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")
        }