x/tools/go/packages: data race in (*loader).refine #31749

bradfitz · 2019-04-29T20:56:56Z

https://storage.googleapis.com/go-build-log/2b8cbc38/linux-amd64-race_ad38befa.log

ok  	golang.org/x/tools/go/loader	31.561s
==================
WARNING: DATA RACE
Write at 0x00c0024e4480 by goroutine 1290:
  runtime.slicecopy()
      /workdir/go/src/runtime/slice.go:197 +0x0
  math/big.(*Rat).Float64()
      /workdir/go/src/math/big/nat.go:95 +0x1e1
  go/constant.Float64Val()
      /workdir/go/src/go/constant/value.go:554 +0x33c
  go/types.roundFloat64()
      /workdir/go/src/go/types/expr.go:175 +0x46
  go/types.representableConst()
      /workdir/go/src/go/types/expr.go:279 +0x89d
  go/types.(*Checker).representable()
      /workdir/go/src/go/types/expr.go:335 +0xb7
  go/types.(*Checker).convertUntyped()
      /workdir/go/src/go/types/expr.go:517 +0xe35
  go/types.(*Checker).binary()
      /workdir/go/src/go/types/expr.go:801 +0x229
  go/types.(*Checker).exprInternal()
      /workdir/go/src/go/types/expr.go:1504 +0x2d6f
  go/types.(*Checker).rawExpr()
      /workdir/go/src/go/types/expr.go:983 +0x91
  go/types.(*Checker).multiExpr()
      /workdir/go/src/go/types/expr.go:1600 +0x68
  go/types.(*Checker).initVars.func1()
      /workdir/go/src/go/types/assignments.go:209 +0xa3
  go/types.unpack()
      /workdir/go/src/go/types/call.go:181 +0xbb
  go/types.(*Checker).initVars()
      /workdir/go/src/go/types/assignments.go:209 +0xea
  go/types.(*Checker).shortVarDecl()
      /workdir/go/src/go/types/assignments.go:322 +0x3a8
  go/types.(*Checker).stmt()
      /workdir/go/src/go/types/stmt.go:398 +0x5986
  go/types.(*Checker).stmtList()
      /workdir/go/src/go/types/stmt.go:120 +0xee
  go/types.(*Checker).funcBody()
      /workdir/go/src/go/types/stmt.go:42 +0x36e
  go/types.(*Checker).funcDecl.func1()
      /workdir/go/src/go/types/decl.go:561 +0xee
  go/types.(*Checker).processDelayed()
      /workdir/go/src/go/types/resolver.go:615 +0x59
  go/types.(*Checker).checkFiles()
      /workdir/go/src/go/types/check.go:256 +0xc9
  golang.org/x/tools/go/packages.(*loader).loadPackage()
      /workdir/go/src/go/types/check.go:245 +0xca9
  golang.org/x/tools/go/packages.(*loader).loadRecursive.func1()
      /workdir/gopath/src/golang.org/x/tools/go/packages/packages.go:683 +0x258
  sync.(*Once).doSlow()
      /workdir/go/src/sync/once.go:52 +0xbf
  sync.(*Once).Do()
      /workdir/go/src/sync/once.go:43 +0x68
  golang.org/x/tools/go/packages.(*loader).loadRecursive()
      /workdir/gopath/src/golang.org/x/tools/go/packages/packages.go:670 +0x74
  golang.org/x/tools/go/packages.(*loader).refine.func2()
      /workdir/gopath/src/golang.org/x/tools/go/packages/packages.go:606 +0x42

Previous write at 0x00c0024e4480 by goroutine 235:
  [failed to restore the stack]

Goroutine 1290 (running) created at:
  golang.org/x/tools/go/packages.(*loader).refine()
      /workdir/gopath/src/golang.org/x/tools/go/packages/packages.go:605 +0x218f
  golang.org/x/tools/go/packages.Load()
      /workdir/gopath/src/golang.org/x/tools/go/packages/packages.go:224 +0x1b8
  golang.org/x/tools/go/packages_test.TestStdlibMetadata()
      /workdir/gopath/src/golang.org/x/tools/go/packages/stdlib_test.go:34 +0x1af
  testing.tRunner()
      /workdir/go/src/testing/testing.go:865 +0x162

Goroutine 235 (running) created at:
  golang.org/x/tools/go/packages.(*loader).refine()
      /workdir/gopath/src/golang.org/x/tools/go/packages/packages.go:605 +0x218f
  golang.org/x/tools/go/packages.Load()
      /workdir/gopath/src/golang.org/x/tools/go/packages/packages.go:224 +0x1b8
  golang.org/x/tools/go/packages_test.TestStdlibMetadata()
      /workdir/gopath/src/golang.org/x/tools/go/packages/stdlib_test.go:34 +0x1af
  testing.tRunner()
      /workdir/go/src/testing/testing.go:865 +0x162
==================
--- FAIL: TestStdlibMetadata (20.99s)
    stdlib_test.go:47: Loaded 199 packages
    stdlib_test.go:55: GOMAXPROCS:  4
    stdlib_test.go:56: Metadata:    20.339881289s
    stdlib_test.go:57: #MB:         273
    testing.go:809: race detected during execution of test
FAIL
FAIL	golang.org/x/tools/go/packages	30.525s

The text was updated successfully, but these errors were encountered:

dominikh · 2019-05-20T03:42:17Z

Edit: my race seems to be different from this race. Will pull into its own issue (#32154).

Simple reproducer:

package main

import (
	"golang.org/x/tools/go/packages"
)

func main() {
	cfg := &packages.Config{
		Mode: packages.LoadAllSyntax,
		Tests: true,
	}

	for {
		packages.Load(cfg, "sandbox/bar")
	}
}

Where sandbox/bar is a package in GOPATH, as follows:

$ cat bar.go
package pkg

import . "fmt"

var _ = Println

$ cat bar_test.go 
package pkg

import . "fmt"

var _ = Println

As far as I've been able to determine, the presence of dot imports as well as the presence of a test package are relevant. This looks like a proper bug in go/packages, not "just" an issue with its tests.

/cc @ianthehat and @matloob again, for good measure.

bcmills · 2019-06-28T14:17:48Z

Here's another one:
https://build.golang.org/log/771a162eb38e5defcbe67e34b910a1e3a695be79

bcmills · 2019-06-28T14:38:20Z

I'm not sure why this has the Testing label. This looks like a straight-up bug in go/packages — the race doesn't seem to be introduced by the test itself.

bcmills · 2019-06-28T14:41:27Z

This looks like the same race that was reported in #30107.

bcmills · 2019-08-29T18:04:07Z

https://build.golang.org/log/2c3d2505bf5145815ce4fc57a449c39a46a32cf2

bcmills · 2019-08-30T17:30:06Z

https://build.golang.org/log/5b0f113bf6114088e575043c333165e97ee7c2bb

bcmills · 2019-10-11T14:33:06Z

This is causing a lot of test failures in the TryBots for x/tools changes, and having racy code in a user-facing API like go/packages is very risky in the first place.

@ianthehat, @matloob: can someone prioritize a fix for this?

bcmills · 2019-10-11T14:35:02Z

Some recent examples:

https://storage.googleapis.com/go-build-log/03ef105d/linux-amd64-race_5947a7d5.log
https://storage.googleapis.com/go-build-log/d0f10a6e/freebsd-amd64-12_0_2652cf0e.log (a crash in refine, presumably induced by this race?)
https://storage.googleapis.com/go-build-log/d0f10a6e/linux-amd64-race_b8480f30.log
https://storage.googleapis.com/go-build-log/46b75578/linux-amd64-race_2335ce7b.log

matloob · 2019-10-11T19:18:39Z

Looking at this now

matloob · 2019-10-11T22:30:01Z

It's not obvious to me what the issue is so I'm going to disable the tests on (tip, race) while I debug.

gopherbot · 2019-10-11T22:30:14Z

Change https://golang.org/cl/200819 mentions this issue: go/packages: temporarily disable some tests running on go tip with -race

A change in go/types has uncovered a race in go/packages. While we debug, ignore those tests when running on tip with -race to make the builders green. This change will be reverted as soon as the issue is fixed. Updates golang/go#31749 Change-Id: I96f0b30a1bc203a5c36a2ce30c943583b7f8035a Reviewed-on: https://go-review.googlesource.com/c/tools/+/200819 Run-TryBot: Rebecca Stambler <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Jay Conrod <[email protected]>

rsc · 2019-10-15T15:33:22Z

The original race reported is #34919. I am not sure about the storage.googleapis.com links that Bryan posted - some of those look different.

rsc · 2019-10-15T15:55:15Z

In the newer races Bryan posted, the three race detector failures are all #34921. The non-race panic about the algorithm going wrong could easily have been caused by those races too.

I think combined #34919 and #34921 explain all the failures in go/packages linked in this thread.

/cc @griesemer

gopherbot · 2019-10-17T23:31:41Z

Change https://golang.org/cl/201838 mentions this issue: go/types: don't update the underlying type of an imported type

griesemer · 2019-10-18T03:06:14Z

Was closed by mistake, https://golang.org/cl/201838 mentioned #34921 by mistake.

Though now that #34921 and #34919 are fixed, this bug may have been fixed as well.

@matloob Could you look into this and see if you can still reproduce this issue (and if not, close it)? Thanks.

bcmills · 2019-10-21T18:57:37Z

Still racy; filed #35049.

bcmills · 2019-10-28T14:33:59Z

Seems to be fixed now. (At least, CL 202538 passed in the TryBots.)

bradfitz added Testing An issue that has been verified to require only test changes, not just a test failure. NeedsFix The path to resolution is known, but the work has not been done. labels Apr 29, 2019

bradfitz added this to the Go1.13 milestone Apr 29, 2019

bradfitz assigned ianthehat and matloob Apr 29, 2019

bcmills mentioned this issue Jun 28, 2019

x/tools/go/packages: panic in testParseFileModifyAST #32833

Open

bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed Testing An issue that has been verified to require only test changes, not just a test failure. labels Jun 28, 2019

gopherbot removed the NeedsFix The path to resolution is known, but the work has not been done. label Jun 28, 2019

bcmills changed the title ~~x/tools/go/packages: data race in tests~~ x/tools/go/packages: data race in (*loader).refine Jun 28, 2019

andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019

bcmills mentioned this issue Aug 29, 2019

x/tools/internal/lsp: frequent out-of-memory errors on windows-amd64-race builder #33951

Closed

rsc modified the milestones: Go1.14, Backlog Oct 9, 2019

bcmills added the Soon This needs action soon. (recent regressions, service outages, unusual time-sensitive situations) label Oct 11, 2019

rsc mentioned this issue Oct 15, 2019

math/big: Rat.Float64 not safe for concurrent use #34919

Closed

rsc mentioned this issue Oct 15, 2019

go/types: data race in Checker.underlying #34921

Closed

gopherbot closed this as completed in 8d2ea29 Oct 18, 2019

griesemer reopened this Oct 18, 2019

bcmills mentioned this issue Oct 21, 2019

go/types: data race in validType #35049

Closed

bcmills closed this as completed Oct 28, 2019

golang locked and limited conversation to collaborators Jan 20, 2021

gopherbot added the FrozenDueToAge label Jan 20, 2021

rsc unassigned ianthehat and matloob Jun 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x/tools/go/packages: data race in (*loader).refine #31749

x/tools/go/packages: data race in (*loader).refine #31749

bradfitz commented Apr 29, 2019

dominikh commented May 20, 2019 •

edited

Loading

bcmills commented Jun 28, 2019

bcmills commented Jun 28, 2019

bcmills commented Jun 28, 2019

bcmills commented Aug 29, 2019

bcmills commented Aug 30, 2019

bcmills commented Oct 11, 2019

bcmills commented Oct 11, 2019

matloob commented Oct 11, 2019

matloob commented Oct 11, 2019

gopherbot commented Oct 11, 2019

rsc commented Oct 15, 2019

rsc commented Oct 15, 2019

gopherbot commented Oct 17, 2019

griesemer commented Oct 18, 2019

bcmills commented Oct 21, 2019

bcmills commented Oct 28, 2019 •

edited by dmitshur

Loading

x/tools/go/packages: data race in (*loader).refine #31749

x/tools/go/packages: data race in (*loader).refine #31749

Comments

bradfitz commented Apr 29, 2019

dominikh commented May 20, 2019 • edited Loading

bcmills commented Jun 28, 2019

bcmills commented Jun 28, 2019

bcmills commented Jun 28, 2019

bcmills commented Aug 29, 2019

bcmills commented Aug 30, 2019

bcmills commented Oct 11, 2019

bcmills commented Oct 11, 2019

matloob commented Oct 11, 2019

matloob commented Oct 11, 2019

gopherbot commented Oct 11, 2019

rsc commented Oct 15, 2019

rsc commented Oct 15, 2019

gopherbot commented Oct 17, 2019

griesemer commented Oct 18, 2019

bcmills commented Oct 21, 2019

bcmills commented Oct 28, 2019 • edited by dmitshur Loading

dominikh commented May 20, 2019 •

edited

Loading

bcmills commented Oct 28, 2019 •

edited by dmitshur

Loading