Skip to content

Commit 33fac98

Browse files
authored
GODRIVER-2109 Prevent a data race between connecting and checking out a connection from the resourcePool. (#728)
1 parent 3a0c0a5 commit 33fac98

File tree

3 files changed

+23
-2
lines changed

3 files changed

+23
-2
lines changed

mongo/integration/client_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
package integration
88

99
import (
10+
"context"
1011
"fmt"
1112
"os"
1213
"reflect"
@@ -457,6 +458,13 @@ func TestClient(t *testing.T) {
457458
mode := modeVal.StringValue()
458459
assert.Equal(mt, mode, "primaryPreferred", "expected read preference mode primaryPreferred, got %v", mode)
459460
})
461+
462+
// Test that using a client with minPoolSize set doesn't cause a data race.
463+
mtOpts = mtest.NewOptions().ClientOptions(options.Client().SetMinPoolSize(5))
464+
mt.RunOpts("minPoolSize", mtOpts, func(mt *mtest.T) {
465+
err := mt.Client.Ping(context.Background(), readpref.Primary())
466+
assert.Nil(t, err, "unexpected error calling Ping: %v", err)
467+
})
460468
}
461469

462470
type proxyMessage struct {

x/mongo/driver/topology/connection.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ type connection struct {
4646
idleDeadline atomic.Value // Stores a time.Time
4747
readTimeout time.Duration
4848
writeTimeout time.Duration
49+
descMu sync.RWMutex // Guards desc. TODO: Remove with or after GODRIVER-2038.
4950
desc description.Server
5051
helloRTT time.Duration
5152
compressor wiremessage.CompressorID
@@ -228,7 +229,9 @@ func (c *connection) connect(ctx context.Context) {
228229
if err == nil {
229230
// We only need to retain the Description field as the connection's description. The authentication-related
230231
// fields in handshakeInfo are tracked by the handshaker if necessary.
232+
c.descMu.Lock()
231233
c.desc = handshakeInfo.Description
234+
c.descMu.Unlock()
232235
c.helloRTT = time.Since(handshakeStartTime)
233236

234237
// If the application has indicated that the cluster is load balanced, ensure the server has included serviceId

x/mongo/driver/topology/pool.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,14 @@ func newPool(config poolConfig, connOpts ...ConnectionOption) (*pool, error) {
196196

197197
// stale checks if a given connection's generation is below the generation of the pool
198198
func (p *pool) stale(c *connection) bool {
199-
return c == nil || p.generation.stale(c.desc.ServiceID, c.generation)
199+
if c == nil {
200+
return true
201+
}
202+
203+
c.descMu.RLock()
204+
serviceID := c.desc.ServiceID
205+
c.descMu.RUnlock()
206+
return p.generation.stale(serviceID, c.generation)
200207
}
201208

202209
// connect puts the pool into the connected state, allowing it to be used and will allow items to begin being processed from the wait queue
@@ -532,7 +539,10 @@ func (p *pool) removeConnection(c *connection, reason string) error {
532539
// Only update the generation numbers map if the connection has retrieved its generation number. Otherwise, we'd
533540
// decrement the count for the generation even though it had never been incremented.
534541
if c.hasGenerationNumber() {
535-
p.generation.removeConnection(c.desc.ServiceID)
542+
c.descMu.RLock()
543+
serviceID := c.desc.ServiceID
544+
c.descMu.RUnlock()
545+
p.generation.removeConnection(serviceID)
536546
}
537547

538548
if publishEvent && p.monitor != nil {

0 commit comments

Comments
 (0)