From 87e5b819e43b11f9f721790db9ff9360f111b32b Mon Sep 17 00:00:00 2001 From: Dawn Minion Date: Tue, 27 Feb 2024 12:15:24 -0800 Subject: [PATCH] server: Fix deadlock if StopBgp is called when conn queue is full Fixes a deadlock condition that can happen if StopBgp is called when the pending connection queue is full. During teardown, StopBgp calls a mgmtOp on the server goroutine which attempts to stop the goroutine accepting inbound connections, and waits for it to finish before continuing. This connection goroutine can block if the connection queue is full, which is read by the same goroutine that processes all mgmtOps. This means that if the queue is full and the goroutine is currently blocked, then calling StopBgp will lead to a complete deadlock, as the connection goroutine will never close as it is trying to send to the queue, but the queue will not be read as the server goroutine is currently waiting for the connection goroutine to exit. To correct this, a context has been added that gets passed to the connection goroutine. This is then checked in a new select statement on the connection queue which gets cancelled by tcpListener.Close() ensuring the goroutine exits correctly even if the queue is full. --- pkg/server/server.go | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/pkg/server/server.go b/pkg/server/server.go index 835b727a1..5f9fb6399 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -44,14 +44,16 @@ import ( ) type tcpListener struct { - l *net.TCPListener - ch chan struct{} + l *net.TCPListener + ch chan struct{} + cancel context.CancelFunc } func (l *tcpListener) Close() error { if err := l.l.Close(); err != nil { return err } + l.cancel() <-l.ch return nil } @@ -106,6 +108,7 @@ func newTCPListener(logger log.Logger, address string, port uint32, bindToDev st } closeCh := make(chan struct{}) + listenerCtx, listenerCancel := context.WithCancel(context.Background()) go func() error { for { conn, err := listener.AcceptTCP() @@ -120,12 +123,16 @@ func newTCPListener(logger log.Logger, address string, port uint32, bindToDev st } return err } - ch <- conn + select { + case ch <- conn: + case <-listenerCtx.Done(): + } } }() return &tcpListener{ - l: listener, - ch: closeCh, + l: listener, + ch: closeCh, + cancel: listenerCancel, }, nil }