Skip to content

Commit de5cc66

Browse files
yishuTchaochn47
authored andcommitted
client: call .Endpoints() in dial() in client/v3/client.go instead of accessing cfg.Endpoints directly
https://github.com/etcd-io/etcd/blob/0cdd558361c6bdbbd9e4023558e2f6ece71c18ad/client/v3/client.go#L299 accesses endpoints without acquiring lock. Fix it to call Endpoints() Fix #13201
1 parent 0fb0045 commit de5cc66

File tree

1 file changed

+13
-10
lines changed

1 file changed

+13
-10
lines changed

clientv3/client.go

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,9 @@ type Client struct {
5656
cfg Config
5757
creds grpccredentials.TransportCredentials
5858
resolver *resolver.EtcdManualResolver
59-
mu *sync.RWMutex
59+
60+
epMu *sync.RWMutex
61+
endpoints []string
6062

6163
ctx context.Context
6264
cancel context.CancelFunc
@@ -140,18 +142,18 @@ func (c *Client) Ctx() context.Context { return c.ctx }
140142
// Endpoints lists the registered endpoints for the client.
141143
func (c *Client) Endpoints() []string {
142144
// copy the slice; protect original endpoints from being changed
143-
c.mu.RLock()
144-
defer c.mu.RUnlock()
145-
eps := make([]string, len(c.cfg.Endpoints))
146-
copy(eps, c.cfg.Endpoints)
145+
c.epMu.RLock()
146+
defer c.epMu.RUnlock()
147+
eps := make([]string, len(c.endpoints))
148+
copy(eps, c.endpoints)
147149
return eps
148150
}
149151

150152
// SetEndpoints updates client's endpoints.
151153
func (c *Client) SetEndpoints(eps ...string) {
152-
c.mu.Lock()
153-
defer c.mu.Unlock()
154-
c.cfg.Endpoints = eps
154+
c.epMu.Lock()
155+
defer c.epMu.Unlock()
156+
c.endpoints = eps
155157

156158
c.resolver.SetEndpoints(eps)
157159
}
@@ -279,7 +281,7 @@ func (c *Client) dial(creds grpccredentials.TransportCredentials, dopts ...grpc.
279281
defer cancel() // TODO: Is this right for cases where grpc.WithBlock() is not set on the dial options?
280282
}
281283

282-
initialEndpoints := strings.Join(c.cfg.Endpoints, ";")
284+
initialEndpoints := strings.Join(c.Endpoints(), ";")
283285
target := fmt.Sprintf("%s://%p/#initially=[%s]", resolver.Schema, c, initialEndpoints)
284286
conn, err := grpc.DialContext(dctx, target, opts...)
285287
if err != nil {
@@ -327,7 +329,7 @@ func newClient(cfg *Config) (*Client, error) {
327329
creds: creds,
328330
ctx: ctx,
329331
cancel: cancel,
330-
mu: new(sync.RWMutex),
332+
epMu: new(sync.RWMutex),
331333
callOpts: defaultCallOpts,
332334
lgMu: new(sync.RWMutex),
333335
}
@@ -369,6 +371,7 @@ func newClient(cfg *Config) (*Client, error) {
369371
if len(cfg.Endpoints) < 1 {
370372
return nil, fmt.Errorf("at least one Endpoint must is required in client config")
371373
}
374+
client.SetEndpoints(cfg.Endpoints...)
372375

373376
// Use a provided endpoint target so that for https:// without any tls config given, then
374377
// grpc will assume the certificate server name is the endpoint host.

0 commit comments

Comments
 (0)