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.

Sign up for GitHub

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

Jump to bottom

clientv3: fix racy writes to context key #11706

Merged
merged 2 commits into from
Mar 19, 2020
Merged

clientv3: fix racy writes to context key #11706

merged 2 commits into from
Mar 19, 2020

Conversation

gyuho
Copy link
Contributor

@gyuho gyuho commented Mar 19, 2020 

=== RUN   TestWatchOverlapContextCancel
==================
WARNING: DATA RACE
Write at 0x00c42110dd40 by goroutine 99:
  runtime.mapassign()
      /usr/local/go/src/runtime/hashmap.go:485 +0x0
  github.com/coreos/etcd/clientv3.metadataSet()
      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/ctx.go:61 +0x8c
  github.com/coreos/etcd/clientv3.withVersion()
      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/ctx.go:47 +0x137
  github.com/coreos/etcd/clientv3.newStreamClientInterceptor.func1()
      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/client.go:309 +0x81
  google.golang.org/grpc.NewClientStream()
      /go/src/github.com/coreos/etcd/gopath/src/google.golang.org/grpc/stream.go:101 +0x10e
  github.com/coreos/etcd/etcdserver/etcdserverpb.(*watchClient).Watch()
      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/etcdserver/etcdserverpb/rpc.pb.go:3193 +0xe9
  github.com/coreos/etcd/clientv3.(*watchGrpcStream).openWatchClient()
      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:788 +0x143
  github.com/coreos/etcd/clientv3.(*watchGrpcStream).newWatchClient()
      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:700 +0x5c3
  github.com/coreos/etcd/clientv3.(*watchGrpcStream).run()
      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:431 +0x12b

Previous read at 0x00c42110dd40 by goroutine 130:
  reflect.maplen()
      /usr/local/go/src/runtime/hashmap.go:1165 +0x0
  reflect.Value.MapKeys()
      /usr/local/go/src/reflect/value.go:1090 +0x43b
  fmt.(*pp).printValue()
      /usr/local/go/src/fmt/print.go:741 +0x1885
  fmt.(*pp).printArg()
      /usr/local/go/src/fmt/print.go:682 +0x1b1
  fmt.(*pp).doPrintf()
      /usr/local/go/src/fmt/print.go:998 +0x1cad
  fmt.Sprintf()
      /usr/local/go/src/fmt/print.go:196 +0x77
  github.com/coreos/etcd/clientv3.streamKeyFromCtx()
      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:825 +0xc8
  github.com/coreos/etcd/clientv3.(*watcher).Watch()
      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:265 +0x426
  github.com/coreos/etcd/clientv3/integration.testWatchOverlapContextCancel.func1()
      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/integration/watch_test.go:959 +0x23e

Goroutine 99 (running) created at:
  github.com/coreos/etcd/clientv3.(*watcher).newWatcherGrpcStream()
      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:236 +0x59d
  github.com/coreos/etcd/clientv3.(*watcher).Watch()
      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:278 +0xbb6
  github.com/coreos/etcd/clientv3/integration.testWatchOverlapContextCancel.func1()
      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/integration/watch_test.go:959 +0x23e

Goroutine 130 (running) created at:
  github.com/coreos/etcd/clientv3/integration.testWatchOverlapContextCancel()
      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/integration/watch_test.go:979 +0x76d
  github.com/coreos/etcd/clientv3/integration.TestWatchOverlapContextCancel()
      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/integration/watch_test.go:922 +0x44
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:657 +0x107

gyuho added 2 commits March 18, 2020 17:05
@gyuho
clientv3: fix racy writes to context key
eeb371b 
=== RUN   TestWatchOverlapContextCancel

==================

WARNING: DATA RACE

Write at 0x00c42110dd40 by goroutine 99:

  runtime.mapassign()

      /usr/local/go/src/runtime/hashmap.go:485 +0x0

  github.com/coreos/etcd/clientv3.metadataSet()

      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/ctx.go:61 +0x8c

  github.com/coreos/etcd/clientv3.withVersion()

      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/ctx.go:47 +0x137

  github.com/coreos/etcd/clientv3.newStreamClientInterceptor.func1()

      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/client.go:309 +0x81

  google.golang.org/grpc.NewClientStream()

      /go/src/github.com/coreos/etcd/gopath/src/google.golang.org/grpc/stream.go:101 +0x10e

  github.com/coreos/etcd/etcdserver/etcdserverpb.(*watchClient).Watch()

      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/etcdserver/etcdserverpb/rpc.pb.go:3193 +0xe9

  github.com/coreos/etcd/clientv3.(*watchGrpcStream).openWatchClient()

      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:788 +0x143

  github.com/coreos/etcd/clientv3.(*watchGrpcStream).newWatchClient()

      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:700 +0x5c3

  github.com/coreos/etcd/clientv3.(*watchGrpcStream).run()

      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:431 +0x12b

Previous read at 0x00c42110dd40 by goroutine 130:

  reflect.maplen()

      /usr/local/go/src/runtime/hashmap.go:1165 +0x0

  reflect.Value.MapKeys()

      /usr/local/go/src/reflect/value.go:1090 +0x43b

  fmt.(*pp).printValue()

      /usr/local/go/src/fmt/print.go:741 +0x1885

  fmt.(*pp).printArg()

      /usr/local/go/src/fmt/print.go:682 +0x1b1

  fmt.(*pp).doPrintf()

      /usr/local/go/src/fmt/print.go:998 +0x1cad

  fmt.Sprintf()

      /usr/local/go/src/fmt/print.go:196 +0x77

  github.com/coreos/etcd/clientv3.streamKeyFromCtx()

      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:825 +0xc8

  github.com/coreos/etcd/clientv3.(*watcher).Watch()

      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:265 +0x426

  github.com/coreos/etcd/clientv3/integration.testWatchOverlapContextCancel.func1()

      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/integration/watch_test.go:959 +0x23e

Goroutine 99 (running) created at:

  github.com/coreos/etcd/clientv3.(*watcher).newWatcherGrpcStream()

      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:236 +0x59d

  github.com/coreos/etcd/clientv3.(*watcher).Watch()

      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:278 +0xbb6

  github.com/coreos/etcd/clientv3/integration.testWatchOverlapContextCancel.func1()

      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/integration/watch_test.go:959 +0x23e

Goroutine 130 (running) created at:

  github.com/coreos/etcd/clientv3/integration.testWatchOverlapContextCancel()

      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/integration/watch_test.go:979 +0x76d

  github.com/coreos/etcd/clientv3/integration.TestWatchOverlapContextCancel()

      /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/integration/watch_test.go:922 +0x44

  testing.tRunner()

      /usr/local/go/src/testing/testing.go:657 +0x107

==================

Signed-off-by: Gyuho Lee <[email protected]>
@gyuho
words: whitelist "racey"
b6d9871 
Signed-off-by: Gyuho Lee <[email protected]>
@gyuho
Copy link
Contributor Author

gyuho commented Mar 19, 2020

Already backported in release branches.

jingyih
jingyih reviewed Mar 19, 2020
View reviewed changes
Copy link
Contributor

@jingyih jingyih left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

spzala
spzala reviewed Mar 19, 2020
View reviewed changes
Copy link
Member

@spzala spzala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm
Thanks @gyuho

@spzala spzala merged commit 45a45d3 into etcd-io:master Mar 19, 2020
@gyuho gyuho deleted the fix-race branch March 19, 2020 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jingyih jingyih jingyih left review comments

@spzala spzala spzala left review comments

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gyuho @spzala @jingyih