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

Data race in tests #871

Closed
Deleplace opened this issue Feb 22, 2021 · 2 comments · Fixed by #872
Closed

Data race in tests #871

Deleplace opened this issue Feb 22, 2021 · 2 comments · Fixed by #872

Comments

@Deleplace
Copy link
Contributor

Observed in my workstation Linux amd64, with all versions of Go1.13+

$ go test -race github.com/evanw/esbuild/internal/bundler
==================
WARNING: DATA RACE
Read at 0x00c0001247e0 by goroutine 20:
  github.com/evanw/esbuild/internal/bundler.(*scanner).preprocessInjectedFiles.func2()
      esbuild/internal/bundler/bundler.go:1085 +0x75

Previous write at 0x00c0001247e0 by goroutine 63:
  github.com/evanw/esbuild/internal/bundler.(*scanner).preprocessInjectedFiles()
      esbuild/internal/bundler/bundler.go:1078 +0x13f7
  github.com/evanw/esbuild/internal/bundler.ScanBundle()
      esbuild/internal/bundler/bundler.go:891 +0x4f6
  github.com/evanw/esbuild/internal/bundler.(*suite).expectBundled.func1()
      esbuild/internal/bundler/bundler_test.go:96 +0x597
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1194 +0x202

Goroutine 20 (running) created at:
  github.com/evanw/esbuild/internal/bundler.(*scanner).preprocessInjectedFiles()
      esbuild/internal/bundler/bundler.go:1084 +0x15b1
  github.com/evanw/esbuild/internal/bundler.ScanBundle()
      esbuild/internal/bundler/bundler.go:891 +0x4f6
  github.com/evanw/esbuild/internal/bundler.(*suite).expectBundled.func1()
      esbuild/internal/bundler/bundler_test.go:96 +0x597
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1194 +0x202

Goroutine 63 (running) created at:
  testing.(*T).Run()
      /usr/local/go/src/testing/testing.go:1239 +0x5d7
  github.com/evanw/esbuild/internal/bundler.(*suite).expectBundled()
      esbuild/internal/bundler/bundler_test.go:84 +0x1ae
  github.com/evanw/esbuild/internal/bundler.TestInject()
      esbuild/internal/bundler/bundler_default_test.go:3393 +0x8bc
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1194 +0x202
==================
--- FAIL: TestInject (0.00s)
    --- FAIL: TestInject/#00 (0.00s)
        testing.go:1093: race detected during execution of test
    testing.go:1093: race detected during execution of test
FAIL
FAIL	github.com/evanw/esbuild/internal/bundler	1.317s
FAIL
@Deleplace
Copy link
Contributor Author

I'll see what I can do.

If I find anything, would you be open to a PR?

@Deleplace
Copy link
Contributor Author

I ❤️ that results are computed concurrently, to maximize performance.

It's a bit tricky to be doing all of these at once:

  • call append in a loop, to make room for the result to be computed soon,
  • but skip some iterations, depending if already visited per duplicateInjectedFiles
  • store each computation result in its final memory location in the result slice

Each call to append may reallocate the whole slice, invalidating the memory where goroutines started by previous iterations are expected to write their result.

I fixed this with a fixed-sized results slice: #872

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant