Skip to content

Commit 118c31b

Browse files
committed
gracefulswitch: Wait for all goroutines on close
Goroutines spawned during balancer swaps could outlive the call to Balancer.Close(). Monitor these via a wait group and wait for them to finish before returning from Close(). This prevents any noticeable side effects that could otherwise occur after Close() returns. RELEASE NOTES: - Closing a graceful switch balancer will now block until all pending goroutines complete. Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>
1 parent 647162c commit 118c31b

File tree

1 file changed

+12
-0
lines changed

1 file changed

+12
-0
lines changed

internal/balancer/gracefulswitch/gracefulswitch.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,10 @@ type Balancer struct {
6767
// balancerCurrent before the UpdateSubConnState is called on the
6868
// balancerCurrent.
6969
currentMu sync.Mutex
70+
71+
// activeGoroutines tracks all the goroutines that this balancer has started
72+
// and that should be waited on when the balancer closes.
73+
activeGoroutines sync.WaitGroup
7074
}
7175

7276
// swap swaps out the current lb with the pending lb and updates the ClientConn.
@@ -76,7 +80,9 @@ func (gsb *Balancer) swap() {
7680
cur := gsb.balancerCurrent
7781
gsb.balancerCurrent = gsb.balancerPending
7882
gsb.balancerPending = nil
83+
gsb.activeGoroutines.Add(1)
7984
go func() {
85+
defer gsb.activeGoroutines.Done()
8086
gsb.currentMu.Lock()
8187
defer gsb.currentMu.Unlock()
8288
cur.Close()
@@ -274,6 +280,7 @@ func (gsb *Balancer) Close() {
274280

275281
currentBalancerToClose.Close()
276282
pendingBalancerToClose.Close()
283+
gsb.activeGoroutines.Wait()
277284
}
278285

279286
// balancerWrapper wraps a balancer.Balancer, and overrides some Balancer
@@ -324,7 +331,12 @@ func (bw *balancerWrapper) UpdateState(state balancer.State) {
324331
defer bw.gsb.mu.Unlock()
325332
bw.lastState = state
326333

334+
// If Close() acquires the mutex before UpdateState(), the balancer
335+
// will already have been removed from the current or pending state when
336+
// reaching this point.
327337
if !bw.gsb.balancerCurrentOrPending(bw) {
338+
// Returning here ensures that (*Balancer).swap() is not invoked after
339+
// (*Balancer).Close() and therefore prevents "use after close".
328340
return
329341
}
330342

0 commit comments

Comments
 (0)