From 2f15894a10a4e4a1161560f63b7e64a97bf30767 Mon Sep 17 00:00:00 2001 From: Dmytro Shynkevych Date: Tue, 14 Jul 2020 13:07:46 -0400 Subject: [PATCH] wgengine/magicsock: wait for derphttp client goroutine to exit Signed-off-by: Dmytro Shynkevych --- wgengine/magicsock/magicsock.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index f7678c163..3c473581a 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -1887,6 +1887,17 @@ func (c *Conn) Close() error { c.pconn6.Close() } err := c.pconn4.Close() + // The goroutine running dc.Connect in derpWriteChanOfAddr may linger + // and appear to leak, as observed in https://github.com/tailscale/tailscale/issues/554. + // This is despite the underlying context being cancelled by connCtxCancel above. + // To avoid this condition, we must wait on derpStarted here + // to ensure that this goroutine has exited by the time Close returns. + // We only do this if derpWriteChanOfAddr has executed at least once: + // on the first run, it sets firstDerp := true and spawns the aforementioned goroutine. + // To detect this, we check activeDerp, which is initialized to non-nil on the first run. + if c.activeDerp != nil { + <-c.derpStarted + } // Wait on endpoints updating right at the end, once everything is // already closed. We want everything else in the Conn to be // consistently in the closed state before we release mu to wait