1
0
mirror of https://github.com/golang/go synced 2024-11-23 08:10:03 -07:00

cmd/go: work around race in fmtCmd

CL 529219 made an existing race with accessing Builder.scriptDir from
Builder.fmtcmd (and now also Builder.Showcmd) much more likely by
dropping a theoretically unnecessary condition from the call from
Builder.run to Builder.fmtcmd.

For an example race report, see
https://build.golang.org/log/c3cad62d0fc33a8381d2091661c685ea1fc525c4

The race is between

  (*Builder).cover2() -> (*Builder).run() -> (*Builder).fmtcmd()

and various other call paths of the form

  (*Builder).build() -> (*gcToolchain).*  (*Builder).Showcmd() -> (*Builder).fmtcmd()

The race can be reproduced with

  go install -race cmd/go
  stress -p 1 go test -x -cover -a log

Return this race to its existing likelihood by putting the condition
back. This isn't a "correct" solution because the race could still
happen if the "cover" tool invoked by Builder.cover2 emits output. But
this will do for a temporary fix.

Change-Id: Ifd811dea07f05e1422fd02b63cd958627727aa12
Reviewed-on: https://go-review.googlesource.com/c/go/+/536355
Auto-Submit: Austin Clements <austin@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This commit is contained in:
Austin Clements 2023-10-18 16:49:50 -04:00 committed by Gopher Robot
parent 1d400cdfc9
commit 3803c85880

View File

@ -2389,7 +2389,7 @@ var cgoTypeSigRe = lazyregexp.New(`\b_C2?(type|func|var|macro)_\B`)
// and returns a non-nil error.
func (b *Builder) run(a *Action, dir string, desc string, env []string, cmdargs ...any) error {
out, err := b.runOut(a, dir, env, cmdargs...)
if desc == "" {
if len(out) > 0 && desc == "" {
desc = b.fmtcmd(dir, "%s", strings.Join(str.StringList(cmdargs...), " "))
}
return b.reportCmd(a, desc, dir, out, err)