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

go race detector reporting race conditions #5

Closed
jchiu0 opened this issue Aug 30, 2016 · 3 comments
Closed

go race detector reporting race conditions #5

jchiu0 opened this issue Aug 30, 2016 · 3 comments

Comments

@jchiu0
Copy link

jchiu0 commented Aug 30, 2016

We're from dgraph (github.com/dgraph-io/dgraph) and we have been using your lock-free hash.

One thing that puzzled us recently is why the Go race detector detects race conditions in some of our Go packages, for example "query".

go test -race github.com/dgraph-io/dgraph/query/...

We thought the lock-free hash use atomics and should not have race conditions. Are these expected? Here is the output for the above run. Any suggestions would be most welcome!

==================
WARNING: DATA RACE
Read at 0x00c42032cd90 by goroutine 69:
  github.com/dgraph-io/dgraph/vendor/github.com/zond/gotomic.(*Hash).getBucketByIndex()
      /home/jchiu/go/src/github.com/dgraph-io/dgraph/vendor/github.com/zond/gotomic/hash.go:407 +0x131
  github.com/dgraph-io/dgraph/vendor/github.com/zond/gotomic.(*Hash).getBucketByIndex()
      /home/jchiu/go/src/github.com/dgraph-io/dgraph/vendor/github.com/zond/gotomic/hash.go:417 +0x285
  github.com/dgraph-io/dgraph/vendor/github.com/zond/gotomic.(*Hash).getBucketByIndex()
      /home/jchiu/go/src/github.com/dgraph-io/dgraph/vendor/github.com/zond/gotomic/hash.go:417 +0x285
  github.com/dgraph-io/dgraph/vendor/github.com/zond/gotomic.(*Hash).getBucketByIndex()
      /home/jchiu/go/src/github.com/dgraph-io/dgraph/vendor/github.com/zond/gotomic/hash.go:417 +0x285
  github.com/dgraph-io/dgraph/vendor/github.com/zond/gotomic.(*Hash).getBucketByIndex()
      /home/jchiu/go/src/github.com/dgraph-io/dgraph/vendor/github.com/zond/gotomic/hash.go:417 +0x285
  github.com/dgraph-io/dgraph/vendor/github.com/zond/gotomic.(*Hash).getBucketByHashCode()
      /home/jchiu/go/src/github.com/dgraph-io/dgraph/vendor/github.com/zond/gotomic/hash.go:393 +0x66
  github.com/dgraph-io/dgraph/vendor/github.com/zond/gotomic.(*Hash).GetHC()
      /home/jchiu/go/src/github.com/dgraph-io/dgraph/vendor/github.com/zond/gotomic/hash.go:247 +0x98
  github.com/dgraph-io/dgraph/vendor/github.com/zond/gotomic.(*Hash).Get()
      /home/jchiu/go/src/github.com/dgraph-io/dgraph/vendor/github.com/zond/gotomic/hash.go:260 +0x6c
  github.com/dgraph-io/dgraph/posting.GetOrCreate()
      /home/jchiu/go/src/github.com/dgraph-io/dgraph/posting/lists.go:251 +0x107
  github.com/dgraph-io/dgraph/worker.processTask()
      /home/jchiu/go/src/github.com/dgraph-io/dgraph/worker/task.go:106 +0x561
  github.com/dgraph-io/dgraph/worker.ProcessTaskOverNetwork()
      /home/jchiu/go/src/github.com/dgraph-io/dgraph/worker/task.go:59 +0x5f0
  github.com/dgraph-io/dgraph/query.ProcessGraph()
      /home/jchiu/go/src/github.com/dgraph-io/dgraph/query/query.go:645 +0x12d5

Previous write at 0x00c42032cd90 by goroutine 70:
  sync/atomic.CompareAndSwapInt64()
      /usr/lib/go-1.7/src/runtime/race_amd64.s:298 +0xb
  sync/atomic.CompareAndSwapPointer()
      /usr/lib/go-1.7/src/runtime/atomic_pointer.go:67 +0x43
  github.com/dgraph-io/dgraph/vendor/github.com/zond/gotomic.(*Hash).getBucketByHashCode()
      /home/jchiu/go/src/github.com/dgraph-io/dgraph/vendor/github.com/zond/gotomic/hash.go:393 +0x66
  github.com/dgraph-io/dgraph/vendor/github.com/zond/gotomic.(*Hash).GetHC()
      /home/jchiu/go/src/github.com/dgraph-io/dgraph/vendor/github.com/zond/gotomic/hash.go:247 +0x98
  github.com/dgraph-io/dgraph/vendor/github.com/zond/gotomic.(*Hash).Get()
      /home/jchiu/go/src/github.com/dgraph-io/dgraph/vendor/github.com/zond/gotomic/hash.go:260 +0x6c
  github.com/dgraph-io/dgraph/posting.GetOrCreate()
      /home/jchiu/go/src/github.com/dgraph-io/dgraph/posting/lists.go:251 +0x107
  github.com/dgraph-io/dgraph/worker.processTask()
      /home/jchiu/go/src/github.com/dgraph-io/dgraph/worker/task.go:106 +0x561
  github.com/dgraph-io/dgraph/worker.ProcessTaskOverNetwork()
      /home/jchiu/go/src/github.com/dgraph-io/dgraph/worker/task.go:59 +0x5f0
  github.com/dgraph-io/dgraph/query.ProcessGraph()
      /home/jchiu/go/src/github.com/dgraph-io/dgraph/query/query.go:645 +0x12d5

Goroutine 69 (running) created at:
  github.com/dgraph-io/dgraph/query.ProcessGraph()
      /home/jchiu/go/src/github.com/dgraph-io/dgraph/query/query.go:692 +0x4db

Goroutine 70 (finished) created at:
  github.com/dgraph-io/dgraph/query.ProcessGraph()
      /home/jchiu/go/src/github.com/dgraph-io/dgraph/query/query.go:692 +0x4db
==================
@zond
Copy link
Owner

zond commented Aug 30, 2016

Good catch!

I wrote this lib before I knew about the race detector (maybe before it existed?).

I ran the unit tests with -race, and found a lot of errors, and I'm moving through them quite quickly now since I don't have to change any logic, just add more atomic.Store/LoadXXX calls.

A new version will come very soon.

@zond
Copy link
Owner

zond commented Aug 30, 2016

Fixed with 0c6a291

@zond zond closed this as completed Aug 30, 2016
@jchiu0
Copy link
Author

jchiu0 commented Sep 1, 2016

Thanks for the very quick fix!! Let us try it out.

@jchiu0 jchiu0 mentioned this issue Sep 12, 2016
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

No branches or pull requests

2 participants