Skip to content

Commit

Permalink
server: Fix deadlock if StopBgp is called when conn queue is full
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Dawn Minion committed Feb 27, 2024
1 parent 8fdda5d commit 87e5b81
Showing 1 changed file with 12 additions and 5 deletions.
17 changes: 12 additions & 5 deletions pkg/server/server.go
Expand Up @@ -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
}
Expand Down Expand Up @@ -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()
Expand All @@ -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
}

Expand Down

0 comments on commit 87e5b81

Please sign in to comment.