botobag / artemis

GraphQL implementation for Go
Other
10 stars 0 forks source link

https://circleci.com/gh/botobag/artemis/971 #137

Closed zonr closed 5 years ago

zonr commented 5 years ago

Commit: 1d896ab


Read at 0x00c00029a2a0 by goroutine 7:
  github.com/botobag/artemis/concurrent.(*workerPoolTask).hasResult()
      /home/circleci/project/concurrent/worker_pool_executor.go:238 +0x56
  github.com/botobag/artemis/concurrent.(*workerPoolTask).AwaitResult()
      /home/circleci/project/concurrent/worker_pool_executor.go:244 +0x61
  github.com/botobag/artemis/concurrent_test.glob..func1.2()
      /home/circleci/project/concurrent/worker_pool_executor_test.go:57 +0x3ee
  github.com/onsi/ginkgo/internal/leafnodes.(*runner).runSync()
      /go/pkg/mod/github.com/onsi/ginkgo@v1.6.0/internal/leafnodes/runner.go:113 +0xbd
  github.com/onsi/ginkgo/internal/leafnodes.(*runner).run()
      /go/pkg/mod/github.com/onsi/ginkgo@v1.6.0/internal/leafnodes/runner.go:64 +0x16a
  github.com/onsi/ginkgo/internal/leafnodes.(*ItNode).Run()
      /go/pkg/mod/github.com/onsi/ginkgo@v1.6.0/internal/leafnodes/it_node.go:26 +0xa2
  github.com/onsi/ginkgo/internal/spec.(*Spec).runSample()
      /go/pkg/mod/github.com/onsi/ginkgo@v1.6.0/internal/spec/spec.go:203 +0x766
  github.com/onsi/ginkgo/internal/spec.(*Spec).Run()
      /go/pkg/mod/github.com/onsi/ginkgo@v1.6.0/internal/spec/spec.go:138 +0x145
  github.com/onsi/ginkgo/internal/specrunner.(*SpecRunner).runSpec()
      /go/pkg/mod/github.com/onsi/ginkgo@v1.6.0/internal/specrunner/spec_runner.go:200 +0x172
  github.com/onsi/ginkgo/internal/specrunner.(*SpecRunner).runSpecs()
      /go/pkg/mod/github.com/onsi/ginkgo@v1.6.0/internal/specrunner/spec_runner.go:170 +0x4b6
  github.com/onsi/ginkgo/internal/specrunner.(*SpecRunner).Run()
      /go/pkg/mod/github.com/onsi/ginkgo@v1.6.0/internal/specrunner/spec_runner.go:66 +0x149
  github.com/onsi/ginkgo/internal/suite.(*Suite).Run()
      /go/pkg/mod/github.com/onsi/ginkgo@v1.6.0/internal/suite/suite.go:62 +0x3e1
  github.com/onsi/ginkgo.RunSpecsWithCustomReporters()
      /go/pkg/mod/github.com/onsi/ginkgo@v1.6.0/ginkgo_dsl.go:221 +0x367
  github.com/onsi/ginkgo.RunSpecs()
      /go/pkg/mod/github.com/onsi/ginkgo@v1.6.0/ginkgo_dsl.go:202 +0x99
  github.com/botobag/artemis/concurrent_test.TestConcurrent()
      /home/circleci/project/concurrent/concurrent_suite_test.go:28 +0xf6
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:827 +0x162

Previous write at 0x00c00029a2a0 by goroutine 21:
  github.com/botobag/artemis/concurrent.(*workerPoolTask).setResult()
      /home/circleci/project/concurrent/worker_pool_executor.go:229 +0xf4
  github.com/botobag/artemis/concurrent.workerPoolExecutorWorker.run()
      /home/circleci/project/concurrent/worker_pool_executor.go:486 +0x60

Goroutine 7 (running) created at:
  testing.(*T).Run()
      /usr/local/go/src/testing/testing.go:878 +0x659
  testing.runTests.func1()
      /usr/local/go/src/testing/testing.go:1119 +0xa8
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:827 +0x162
  testing.runTests()
      /usr/local/go/src/testing/testing.go:1117 +0x4ee
  testing.(*M).Run()
      /usr/local/go/src/testing/testing.go:1034 +0x2ee
  main.main()
      _testmain.go:96 +0x342

Goroutine 21 (running) created at:
  github.com/botobag/artemis/concurrent.workerPoolExecutorWorker.Start()
      /home/circleci/project/concurrent/worker_pool_executor.go:464 +0x78
  github.com/botobag/artemis/concurrent.(*WorkerPoolExecutor).addWorker()
      /home/circleci/project/concurrent/worker_pool_executor.go:712 +0x165
  github.com/botobag/artemis/concurrent.(*WorkerPoolExecutor).addTask()
      /home/circleci/project/concurrent/worker_pool_executor.go:761 +0x194
  github.com/botobag/artemis/concurrent.(*WorkerPoolExecutor).Submit()
      /home/circleci/project/concurrent/worker_pool_executor.go:665 +0x175
  github.com/botobag/artemis/concurrent_test.glob..func1.2()
      /home/circleci/project/concurrent/worker_pool_executor_test.go:53 +0x23a
  github.com/onsi/ginkgo/internal/leafnodes.(*runner).runSync()
      /go/pkg/mod/github.com/onsi/ginkgo@v1.6.0/internal/leafnodes/runner.go:113 +0xbd
  github.com/onsi/ginkgo/internal/leafnodes.(*runner).run()
      /go/pkg/mod/github.com/onsi/ginkgo@v1.6.0/internal/leafnodes/runner.go:64 +0x16a
  github.com/onsi/ginkgo/internal/leafnodes.(*ItNode).Run()
      /go/pkg/mod/github.com/onsi/ginkgo@v1.6.0/internal/leafnodes/it_node.go:26 +0xa2
  github.com/onsi/ginkgo/internal/spec.(*Spec).runSample()
      /go/pkg/mod/github.com/onsi/ginkgo@v1.6.0/internal/spec/spec.go:203 +0x766
  github.com/onsi/ginkgo/internal/spec.(*Spec).Run()
      /go/pkg/mod/github.com/onsi/ginkgo@v1.6.0/internal/spec/spec.go:138 +0x145
  github.com/onsi/ginkgo/internal/specrunner.(*SpecRunner).runSpec()
      /go/pkg/mod/github.com/onsi/ginkgo@v1.6.0/internal/specrunner/spec_runner.go:200 +0x172
  github.com/onsi/ginkgo/internal/specrunner.(*SpecRunner).runSpecs()
      /go/pkg/mod/github.com/onsi/ginkgo@v1.6.0/internal/specrunner/spec_runner.go:170 +0x4b6
  github.com/onsi/ginkgo/internal/specrunner.(*SpecRunner).Run()
      /go/pkg/mod/github.com/onsi/ginkgo@v1.6.0/internal/specrunner/spec_runner.go:66 +0x149
  github.com/onsi/ginkgo/internal/suite.(*Suite).Run()
      /go/pkg/mod/github.com/onsi/ginkgo@v1.6.0/internal/suite/suite.go:62 +0x3e1
  github.com/onsi/ginkgo.RunSpecsWithCustomReporters()
      /go/pkg/mod/github.com/onsi/ginkgo@v1.6.0/ginkgo_dsl.go:221 +0x367
  github.com/onsi/ginkgo.RunSpecs()
      /go/pkg/mod/github.com/onsi/ginkgo@v1.6.0/ginkgo_dsl.go:202 +0x99
  github.com/botobag/artemis/concurrent_test.TestConcurrent()
      /home/circleci/project/concurrent/concurrent_suite_test.go:28 +0xf6
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:827 +0x162
==================```
zonr commented 5 years ago

This is because AwaitResult performs fast check on task.hasResult without acquiring mutex. This is benign. And we recheck it with lock to make sure that the result is not ready and block the goroutine on cond. We have to find a way to disable the error.

zonr commented 5 years ago

We can use atomic.{Load,Store}Pointer to access task.cond in task.hasResult to suppress the error but we decided to remove the quick path instead. We should show that there're non-negligible overheads before adding complexity