device: wait for routines to stop before removing peers

Peers are currently removed after Device's goroutines are signaled to stop,
but without waiting for them to actually do so, which is racy.

For example, RoutineHandshake may be in Peer.SendKeepalive
when the corresponding peer is removed, which closes its nonce channel.
This causes a send on a closed channel, as observed in tailscale/tailscale#487.

This patch seems to be the correct synchronizing action:
Peer's goroutines are receivers and handle channel closure gracefully,
so Device's goroutines are the ones that should be fully stopped first.

Signed-Off-By: Dmytro Shynkevych <dmytro@tailscale.com>

diff --git a/device/device.go b/device/device.go
index c64432ed307200f8ef3a0a2d95803c13d7b9dce5..c440679c0c84f9e41d04caa92910d1366734e311 100644 (file)
--- a/device/device.go
+++ b/device/device.go
@@ -383,10 +383,10 @@ func (device *Device) Close() {
        device.isUp.Set(false)
 
        close(device.signals.stop)
+       device.state.stopping.Wait()
 
        device.RemoveAllPeers()
 
-       device.state.stopping.Wait()
        device.FlushPacketQueues()
 
        device.rate.limiter.Close()