Skip to content

Commit f14095b

Browse files
committed
only async reauth
1 parent c715185 commit f14095b

File tree

4 files changed

+18
-23
lines changed

4 files changed

+18
-23
lines changed

internal/auth/streaming/conn_reauth_credentials_listener.go

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -35,25 +35,11 @@ func (c *ConnReAuthCredentialsListener) OnNext(credentials auth.Credentials) {
3535
return
3636
}
3737

38-
// this connection is not in use, so we can re-authenticate it
39-
if c.conn.Used.CompareAndSwap(false, true) {
40-
// try to acquire the connection for background operation
41-
if c.conn.Usable.CompareAndSwap(true, false) {
42-
err := c.reAuth(c.conn, credentials)
43-
if err != nil {
44-
c.OnError(err)
45-
}
46-
c.conn.Usable.Store(true)
47-
c.conn.Used.Store(false)
48-
return
49-
}
50-
c.conn.Used.Store(false)
51-
}
52-
// else if the connection is in use, mark it for re-authentication
53-
// and connection pool hook will re-authenticate it when it is returned to the pool
54-
// or in case the connection WAS in the pool, but handoff is in progress, the pool hook
55-
// will re-authenticate it when the handoff is complete
56-
// and the connection is acquired from the pool
38+
// Always use async reauth to avoid complex pool semaphore issues
39+
// The synchronous path can cause deadlocks in the pool's semaphore mechanism
40+
// when called from the Subscribe goroutine, especially with small pool sizes.
41+
// The connection pool hook will re-authenticate the connection when it is
42+
// returned to the pool in a clean, idle state.
5743
c.manager.MarkForReAuth(c.conn, func(err error) {
5844
if err != nil {
5945
c.OnError(err)

internal/auth/streaming/pool_hook.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,16 +92,24 @@ func (r *ReAuthPoolHook) OnPut(_ context.Context, conn *pool.Conn) (bool, bool,
9292
var err error
9393
timeout := time.After(r.reAuthTimeout)
9494

95-
// Try to acquire the connection (set Usable to false)
96-
for !conn.Usable.CompareAndSwap(true, false) {
95+
// Try to acquire the connection
96+
// We need to ensure the connection is both Usable and not Used
97+
// to prevent data races with concurrent operations
98+
acquired := false
99+
for !acquired {
97100
select {
98101
case <-timeout:
99102
// Timeout occurred, cannot acquire connection
100103
err = pool.ErrConnUnusableTimeout
101104
reAuthFn(err)
102105
return
103106
default:
104-
time.Sleep(time.Millisecond)
107+
// Try to acquire: set Usable=false only if Used=false
108+
if !conn.Used.Load() && conn.Usable.CompareAndSwap(true, false) {
109+
acquired = true
110+
} else {
111+
time.Sleep(time.Millisecond)
112+
}
105113
}
106114
}
107115

pubsub.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,6 @@ func (c *PubSub) ReceiveTimeout(ctx context.Context, timeout time.Duration) (int
465465
}
466466

467467
// Don't hold the lock to allow subscriptions and pings.
468-
469468
cn, err := c.connWithLock(ctx)
470469
if err != nil {
471470
return nil, err

redis.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,8 @@ func (c *baseClient) reAuthConnection() func(poolCn *pool.Conn, credentials auth
305305
return func(poolCn *pool.Conn, credentials auth.Credentials) error {
306306
var err error
307307
username, password := credentials.BasicAuth()
308+
309+
// Use background context - timeout is handled by ReadTimeout in WithReader/WithWriter
308310
ctx := context.Background()
309311

310312
connPool := pool.NewSingleConnPool(c.connPool, poolCn)

0 commit comments

Comments
 (0)