Skip to content

Commit 391b6c5

Browse files
committed
address pr comments
1 parent e03396e commit 391b6c5

File tree

4 files changed

+26
-11
lines changed

4 files changed

+26
-11
lines changed

internal/auth/conn_reauth_credentials_listener.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,18 +54,27 @@ func (c *ConnReAuthCredentialsListener) OnNext(credentials auth2.Credentials) {
5454
case <-timeout:
5555
err = pool.ErrConnUnusableTimeout
5656
default:
57+
// small sleep to avoid busy looping
58+
time.Sleep(100 * time.Microsecond)
59+
// yield the thread to allow other goroutines to run
5760
runtime.Gosched()
5861
}
5962
}
6063
if err == nil {
6164
defer c.conn.SetUsable(true)
6265
}
6366

67+
// This check just verifies that the connection is not in use.
68+
// If the connection is in use, we don't want to interfere with that.
69+
// As soon as the connection is not in use, we mark it as in use.
6470
for err == nil && !c.conn.Used.CompareAndSwap(false, true) {
6571
select {
6672
case <-timeout:
6773
err = pool.ErrConnUnusableTimeout
6874
default:
75+
// small sleep to avoid busy looping
76+
time.Sleep(100 * time.Microsecond)
77+
// yield the thread to allow other goroutines to run
6978
runtime.Gosched()
7079
}
7180
}

internal/pool/buffer_size_test.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -123,16 +123,19 @@ var _ = Describe("Buffer Size Configuration", func() {
123123
})
124124

125125
// Helper functions to extract buffer sizes using unsafe pointers
126-
// The struct layout must match pool.Conn exactly to avoid checkptr violations
126+
// The struct layout must match pool.Conn exactly to avoid checkptr violations.
127+
// checkptr is Go's pointer safety checker, which ensures that unsafe pointer
128+
// conversions are valid. If the struct layouts do not match exactly, this can
129+
// cause runtime panics or incorrect memory access due to invalid pointer dereferencing.
127130
func getWriterBufSizeUnsafe(cn *pool.Conn) int {
128131
// Import required for atomic types
129132
type atomicBool struct{ _ uint32 }
130133
type atomicInt64 struct{ _ int64 }
131134

132135
cnPtr := (*struct {
133-
id uint64 // First field in pool.Conn
134-
usedAt int64 // Second field (atomic)
135-
netConnAtomic interface{} // atomic.Value (interface{} has same size)
136+
id uint64 // First field in pool.Conn
137+
usedAt int64 // Second field (atomic)
138+
netConnAtomic interface{} // atomic.Value (interface{} has same size)
136139
rd *proto.Reader
137140
bw *bufio.Writer
138141
wr *proto.Writer
@@ -156,9 +159,9 @@ func getWriterBufSizeUnsafe(cn *pool.Conn) int {
156159

157160
func getReaderBufSizeUnsafe(cn *pool.Conn) int {
158161
cnPtr := (*struct {
159-
id uint64 // First field in pool.Conn
160-
usedAt int64 // Second field (atomic)
161-
netConnAtomic interface{} // atomic.Value (interface{} has same size)
162+
id uint64 // First field in pool.Conn
163+
usedAt int64 // Second field (atomic)
164+
netConnAtomic interface{} // atomic.Value (interface{} has same size)
162165
rd *proto.Reader
163166
bw *bufio.Writer
164167
wr *proto.Writer

internal/pool/conn.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -493,10 +493,13 @@ func (cn *Conn) MarkQueuedForHandoff() error {
493493

494494
// first we need to mark the connection as not usable
495495
// to prevent the pool from returning it to the caller
496-
if !connAcquired && !cn.Usable.CompareAndSwap(true, false) {
497-
continue
496+
if !connAcquired {
497+
if cn.Usable.CompareAndSwap(true, false) {
498+
connAcquired = true
499+
} else {
500+
continue
501+
}
498502
}
499-
connAcquired = true
500503

501504
currentState := cn.getHandoffState()
502505
// Check if marked for handoff

redis.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ func (c *baseClient) connReAuthCredentialsListener(poolCn *pool.Conn) (auth.Cred
319319
// Design decision: The main case we expect the connection to be in a non-usable state is when it is being reconnected
320320
// during a handoff from maintnotifications.
321321
// Setting the checkUsableTimeout to the handoff timeout if maintnotifications are enabled
322-
// the default timeout if no maintnotifications are disabled is going to PoolTimeout.
322+
// the default timeout if maintnotifications are disabled is going to PoolTimeout.
323323
//
324324
// Note: Due to the auto by default mode of MaintNotificationsConfig
325325
// the timeout for the first connection will probably be the value of MaintNotificationsConfig.HandoffTimeout

0 commit comments

Comments
 (0)