Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/go/packages: data race in tests #31749

Open
bradfitz opened this issue Apr 29, 2019 · 1 comment

Comments

Projects
None yet
4 participants
@bradfitz
Copy link
Member

commented Apr 29, 2019

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

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

@dominikh

This comment has been minimized.

Copy link
Member

commented May 20, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.