From 12486b055734f92c1622af9128f28f8376f75d02 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Sat, 6 Jul 2019 11:52:30 +0100 Subject: [PATCH 1/4] Try to more gracefully handle shutdowns on Windows --- cmd/yggdrasil/main.go | 16 +++++++++------- src/multicast/multicast.go | 7 +++++++ src/tuntap/iface.go | 6 ++++++ src/tuntap/tun.go | 10 ++++++++++ 4 files changed, 32 insertions(+), 7 deletions(-) diff --git a/cmd/yggdrasil/main.go b/cmd/yggdrasil/main.go index 6af27725..fd8828c8 100644 --- a/cmd/yggdrasil/main.go +++ b/cmd/yggdrasil/main.go @@ -231,11 +231,6 @@ func main() { } else { logger.Errorln("Unable to get Listener:", err) } - // The Stop function ensures that the TUN/TAP adapter is correctly shut down - // before the program exits. - defer func() { - n.core.Stop() - }() // Make some nice output that tells us what our IPv6 address and subnet are. // This is just logged to stdout for the user. address := n.core.Address() @@ -256,6 +251,8 @@ func main() { // deferred Stop function above will run which will shut down TUN/TAP. for { select { + case _ = <-c: + goto exit case _ = <-r: if *useconffile != "" { cfg = readConfig(useconf, useconffile, normaliseconf) @@ -265,11 +262,16 @@ func main() { } else { logger.Errorln("Reloading config at runtime is only possible with -useconffile") } - case _ = <-c: - goto exit } } exit: + // When gracefully shutting down we should try and clean up as much as + // possible, although not all of these functions are necessarily implemented + // yet + n.core.Stop() + n.admin.Stop() + n.multicast.Stop() + n.tuntap.Stop() } func (n *node) sessionFirewall(pubkey *crypto.BoxPubKey, initiator bool) bool { diff --git a/src/multicast/multicast.go b/src/multicast/multicast.go index 3c0d8c0f..ba1f18fb 100644 --- a/src/multicast/multicast.go +++ b/src/multicast/multicast.go @@ -26,6 +26,7 @@ type Multicast struct { groupAddr string listeners map[string]*yggdrasil.TcpListener listenPort uint16 + isOpen bool } // Init prepares the multicast interface for use. @@ -61,6 +62,7 @@ func (m *Multicast) Start() error { // Windows can't set this flag, so we need to handle it in other ways } + m.isOpen = true go m.multicastStarted() go m.listen() go m.announce() @@ -70,6 +72,8 @@ func (m *Multicast) Start() error { // Stop is not implemented for multicast yet. func (m *Multicast) Stop() error { + m.isOpen = false + m.sock.Close() return nil } @@ -246,6 +250,9 @@ func (m *Multicast) listen() { for { nBytes, rcm, fromAddr, err := m.sock.ReadFrom(bs) if err != nil { + if !m.isOpen { + return + } panic(err) } if rcm != nil { diff --git a/src/tuntap/iface.go b/src/tuntap/iface.go index 16a3b65d..60c814c2 100644 --- a/src/tuntap/iface.go +++ b/src/tuntap/iface.go @@ -98,6 +98,9 @@ func (tun *TunAdapter) writer() error { util.PutBytes(b) } if err != nil { + if !tun.isOpen { + return err + } tun.log.Errorln("TUN/TAP iface write error:", err) continue } @@ -114,6 +117,9 @@ func (tun *TunAdapter) reader() error { // Wait for a packet to be delivered to us through the TUN/TAP adapter n, err := tun.iface.Read(bs) if err != nil { + if !tun.isOpen { + return err + } panic(err) } if n == 0 { diff --git a/src/tuntap/tun.go b/src/tuntap/tun.go index ed5d2d45..b7b4cfa6 100644 --- a/src/tuntap/tun.go +++ b/src/tuntap/tun.go @@ -181,6 +181,16 @@ func (tun *TunAdapter) Start() error { return nil } +// Start the setup process for the TUN/TAP adapter. If successful, starts the +// read/write goroutines to handle packets on that interface. +func (tun *TunAdapter) Stop() error { + tun.isOpen = false + // TODO: we have nothing that cleanly stops all the various goroutines opened + // by TUN/TAP, e.g. readers/writers, sessions + tun.iface.Close() + return nil +} + // UpdateConfig updates the TUN/TAP module with the provided config.NodeConfig // and then signals the various module goroutines to reconfigure themselves if // needed. From 02c99d3e7d2ae9d66557533e365ecc5f79113ebe Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Sat, 6 Jul 2019 12:04:31 +0100 Subject: [PATCH 2/4] More directly define a minwinsvc exit handler --- cmd/yggdrasil/main.go | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/cmd/yggdrasil/main.go b/cmd/yggdrasil/main.go index fd8828c8..79446849 100644 --- a/cmd/yggdrasil/main.go +++ b/cmd/yggdrasil/main.go @@ -172,7 +172,7 @@ func main() { logger = log.New(syslogger, "", log.Flags()) } default: - if logfd, err := os.Create(*logto); err == nil { + if logfd, err := os.OpenFile(*logto, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644); err == nil { logger = log.New(logfd, "", log.Flags()) } } @@ -242,11 +242,16 @@ func main() { r := make(chan os.Signal, 1) signal.Notify(c, os.Interrupt, syscall.SIGTERM) signal.Notify(r, os.Interrupt, syscall.SIGHUP) - // Create a function to capture the service being stopped on Windows. - winTerminate := func() { - c <- os.Interrupt + // Define what happens when we want to stop Yggdrasil. + terminate := func() { + n.core.Stop() + n.admin.Stop() + n.multicast.Stop() + n.tuntap.Stop() + os.Exit(0) } - minwinsvc.SetOnExit(winTerminate) + // Capture the service being stopped on Windows. + minwinsvc.SetOnExit(terminate) // Wait for the terminate/interrupt signal. Once a signal is received, the // deferred Stop function above will run which will shut down TUN/TAP. for { @@ -265,13 +270,7 @@ func main() { } } exit: - // When gracefully shutting down we should try and clean up as much as - // possible, although not all of these functions are necessarily implemented - // yet - n.core.Stop() - n.admin.Stop() - n.multicast.Stop() - n.tuntap.Stop() + terminate() } func (n *node) sessionFirewall(pubkey *crypto.BoxPubKey, initiator bool) bool { From 618d46a7b30bb7e491591ebeb3f15ad40e41021d Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Sat, 6 Jul 2019 12:12:30 +0100 Subject: [PATCH 3/4] Don't block on adding peers in case one is unreachable and we are forced to wait for timeout --- src/yggdrasil/core.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/yggdrasil/core.go b/src/yggdrasil/core.go index 3a7f9f1b..62d89a8b 100644 --- a/src/yggdrasil/core.go +++ b/src/yggdrasil/core.go @@ -88,14 +88,14 @@ func (c *Core) addPeerLoop() { // Add peers from the Peers section for _, peer := range current.Peers { - c.AddPeer(peer, "") + go c.AddPeer(peer, "") time.Sleep(time.Second) } // Add peers from the InterfacePeers section for intf, intfpeers := range current.InterfacePeers { for _, peer := range intfpeers { - c.AddPeer(peer, intf) + go c.AddPeer(peer, intf) time.Sleep(time.Second) } } From 4804ce39afb250e2a5890182007e3b66e2a620a3 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Sat, 6 Jul 2019 12:17:40 +0100 Subject: [PATCH 4/4] Tidy up the terminate path a bit --- cmd/yggdrasil/main.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/cmd/yggdrasil/main.go b/cmd/yggdrasil/main.go index 79446849..129b01d5 100644 --- a/cmd/yggdrasil/main.go +++ b/cmd/yggdrasil/main.go @@ -242,16 +242,9 @@ func main() { r := make(chan os.Signal, 1) signal.Notify(c, os.Interrupt, syscall.SIGTERM) signal.Notify(r, os.Interrupt, syscall.SIGHUP) - // Define what happens when we want to stop Yggdrasil. - terminate := func() { - n.core.Stop() - n.admin.Stop() - n.multicast.Stop() - n.tuntap.Stop() - os.Exit(0) - } // Capture the service being stopped on Windows. - minwinsvc.SetOnExit(terminate) + minwinsvc.SetOnExit(n.shutdown) + defer n.shutdown() // Wait for the terminate/interrupt signal. Once a signal is received, the // deferred Stop function above will run which will shut down TUN/TAP. for { @@ -270,7 +263,14 @@ func main() { } } exit: - terminate() +} + +func (n *node) shutdown() { + n.core.Stop() + n.admin.Stop() + n.multicast.Stop() + n.tuntap.Stop() + os.Exit(0) } func (n *node) sessionFirewall(pubkey *crypto.BoxPubKey, initiator bool) bool {