From de4696da1062fc0f40301ff8b0bbf200616b016b Mon Sep 17 00:00:00 2001 From: Josh Bleecher Snyder Date: Fri, 14 Jan 2022 13:43:24 -0800 Subject: [PATCH] wgengine/magicsock: fix deadlock on shutdown This fixes a deadlock on shutdown. One goroutine is waiting to send on c.derpRecvCh before unlocking c.mu. The other goroutine is waiting to lock c.mu before receiving from c.derpRecvCh. #3736 has a more detailed explanation of the sequence of events. Fixes #3736 Signed-off-by: Josh Bleecher Snyder --- wgengine/magicsock/magicsock.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 028019bfe..b072288df 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -265,6 +265,7 @@ type Conn struct { stunReceiveFunc atomic.Value // of func(p []byte, fromAddr *net.UDPAddr) // derpRecvCh is used by receiveDERP to read DERP messages. + // It must have buffer size > 0; see issue 3736. derpRecvCh chan derpReadResult // bind is the wireguard-go conn.Bind for Conn. @@ -529,7 +530,7 @@ func (o *Options) derpActiveFunc() func() { // of NewConn. Mostly for tests. func newConn() *Conn { c := &Conn{ - derpRecvCh: make(chan derpReadResult), + derpRecvCh: make(chan derpReadResult, 1), // must be buffered, see issue 3736 derpStarted: make(chan struct{}), peerLastDerp: make(map[key.NodePublic]int), peerMap: newPeerMap(), @@ -2647,6 +2648,7 @@ func (c *connBind) Close() error { } // Send an empty read result to unblock receiveDERP, // which will then check connBind.Closed. + // connBind.Closed takes c.mu, but c.derpRecvCh is buffered. c.derpRecvCh <- derpReadResult{} return nil }