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

v1.7.x: race between (*ccBalancerWrapper).NewSubConn and (*addrConn).tearDown #1662

Closed
gyuho opened this issue Nov 9, 2017 · 1 comment
Closed
Assignees

Comments

@gyuho
Copy link
Contributor

gyuho commented Nov 9, 2017

What version of gRPC are you using?

v1.7.2

What version of Go are you using (go version)?

Go 1.9.2

What operating system (Linux, Windows, …) and version?

Linux 4.10.0-38-generic #42~16.04.1-Ubuntu x86_64 x86_64 x86_64 GNU/Linux

What did you do?

Run integration tests in etcd

What did you expect to see?

No race.

What did you see instead?

=== RUN   TestKVPutWithRequireLeader
==================
WARNING: DATA RACE
Read at 0x00c4258c91a8 by goroutine 1254:
  google.golang.org/grpc.(*addrConn).tearDown()
      /home/jenkins/workspace/etcd-proxy/gopath/src/google.golang.org/grpc/clientconn.go:1127 +0x232
  google.golang.org/grpc.(*ClientConn).Close()
      /home/jenkins/workspace/etcd-proxy/gopath/src/google.golang.org/grpc/clientconn.go:812 +0x27c
  github.com/coreos/etcd/clientv3.(*Client).Close()
      /home/jenkins/workspace/etcd-proxy/gopath/src/github.com/coreos/etcd/clientv3/client.go:100 +0xf2
  github.com/coreos/etcd/integration.(*ClusterV3).Terminate()
      /home/jenkins/workspace/etcd-proxy/gopath/src/github.com/coreos/etcd/integration/cluster.go:1017 +0xbb
  github.com/coreos/etcd/clientv3/integration.TestKVPutWithRequireLeader()
      /home/jenkins/workspace/etcd-proxy/gopath/src/github.com/coreos/etcd/clientv3/integration/kv_test.go:217 +0x55c
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:746 +0x16c

Previous write at 0x00c4258c91a8 by goroutine 273:
  google.golang.org/grpc.(*ccBalancerWrapper).NewSubConn()
      /home/jenkins/workspace/etcd-proxy/gopath/src/google.golang.org/grpc/balancer_conn_wrappers.go:174 +0x21b
  google.golang.org/grpc.(*balancerWrapper).lbWatcher()
      /home/jenkins/workspace/etcd-proxy/gopath/src/google.golang.org/grpc/balancer_v1_wrapper.go:202 +0x197e

Goroutine 1254 (running) created at:
  testing.(*T).Run()
      /usr/local/go/src/testing/testing.go:789 +0x568
  testing.runTests.func1()
      /usr/local/go/src/testing/testing.go:1004 +0xa7
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:746 +0x16c
  testing.runTests()
      /usr/local/go/src/testing/testing.go:1002 +0x521
  testing.(*M).Run()
      /usr/local/go/src/testing/testing.go:921 +0x206
  github.com/coreos/etcd/clientv3/integration.TestMain()
      /home/jenkins/workspace/etcd-proxy/gopath/src/github.com/coreos/etcd/clientv3/integration/main_test.go:15 +0x38
  main.main()
      github.com/coreos/etcd/clientv3/integration/_test/_testmain.go:330 +0x1cd

Goroutine 273 (running) created at:
  google.golang.org/grpc.(*balancerWrapperBuilder).Build()
      /home/jenkins/workspace/etcd-proxy/gopath/src/google.golang.org/grpc/balancer_v1_wrapper.go:53 +0x42f
  google.golang.org/grpc.newCCBalancerWrapper()
      /home/jenkins/workspace/etcd-proxy/gopath/src/google.golang.org/grpc/balancer_conn_wrappers.go:109 +0x2da
  google.golang.org/grpc.DialContext()
      /home/jenkins/workspace/etcd-proxy/gopath/src/google.golang.org/grpc/clientconn.go:449 +0xa18
  github.com/coreos/etcd/clientv3.(*Client).dial()
      /home/jenkins/workspace/etcd-proxy/gopath/src/github.com/coreos/etcd/clientv3/client.go:350 +0x28b
  github.com/coreos/etcd/clientv3.newClient()
      /home/jenkins/workspace/etcd-proxy/gopath/src/github.com/coreos/etcd/clientv3/client.go:400 +0x5e9
  github.com/coreos/etcd/clientv3.New()
      /home/jenkins/workspace/etcd-proxy/gopath/src/github.com/coreos/etcd/clientv3/client.go:78 +0xf3
  github.com/coreos/etcd/integration.newClientV3()
      /home/jenkins/workspace/etcd-proxy/gopath/src/github.com/coreos/etcd/integration/cluster_proxy.go:97 +0x65
  github.com/coreos/etcd/integration.NewClientV3()
      /home/jenkins/workspace/etcd-proxy/gopath/src/github.com/coreos/etcd/integration/cluster.go:641 +0x194
  github.com/coreos/etcd/integration.NewClusterV3()
      /home/jenkins/workspace/etcd-proxy/gopath/src/github.com/coreos/etcd/integration/cluster.go:994 +0x244
  github.com/coreos/etcd/clientv3/integration.TestKVPutWithRequireLeader()
      /home/jenkins/workspace/etcd-proxy/gopath/src/github.com/coreos/etcd/clientv3/integration/kv_test.go:192 +0x10c
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:746 +0x16c
==================
--- FAIL: TestKVPutWithRequireLeader (0.81s)
	testing.go:699: race detected during execution of test

Could we backport a353537#diff-8c7927bd3110f52de8fdc82a3e821274R192?

v1.7.x doesn't have this locks.

func (ccb *ccBalancerWrapper) NewSubConn(addrs []resolver.Address, opts balancer.NewSubConnOptions) (balancer.SubConn, error) {
grpclog.Infof("ccBalancerWrapper: new subconn: %v", addrs)
ac, err := ccb.cc.newAddrConn(addrs)
if err != nil {
return nil, err
}
acbw := &acBalancerWrapper{ac: ac}
ac.acbw = acbw
return acbw, nil
}

/cc @xiang90

@menghanl
Copy link
Contributor

fixed by #1665

@lock lock bot locked as resolved and limited conversation to collaborators Sep 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants