From 5a382e7e0bf2c86195a689419c7a7ab186c53f6e Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Thu, 19 Sep 2019 08:55:55 +0100 Subject: [PATCH 1/2] Cherrypick fixes for _addPeerLoop memory leak for now --- src/yggdrasil/api.go | 23 ++++++++++++++++++++++- src/yggdrasil/core.go | 28 +++++++++++++++------------- 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/src/yggdrasil/api.go b/src/yggdrasil/api.go index d1753b69..b54d7a11 100644 --- a/src/yggdrasil/api.go +++ b/src/yggdrasil/api.go @@ -368,13 +368,34 @@ func (c *Core) SetLogger(log *log.Logger) { // connection drops. func (c *Core) AddPeer(addr string, sintf string) error { if err := c.CallPeer(addr, sintf); err != nil { + // TODO: We maybe want this to write the peer to the persistent + // configuration even if a connection attempt fails, but first we'll need to + // move the code to check the peer URI so that we don't deliberately save a + // peer with a known bad URI. Loading peers from config should really do the + // same thing too but I don't think that happens today return err } c.config.Mutex.Lock() if sintf == "" { + for _, peer := range c.config.Current.Peers { + if peer == addr { + return errors.New("peer already added") + } + } c.config.Current.Peers = append(c.config.Current.Peers, addr) } else { - c.config.Current.InterfacePeers[sintf] = append(c.config.Current.InterfacePeers[sintf], addr) + if _, ok := c.config.Current.InterfacePeers[sintf]; ok { + for _, peer := range c.config.Current.InterfacePeers[sintf] { + if peer == addr { + return errors.New("peer already added") + } + } + } + if _, ok := c.config.Current.InterfacePeers[sintf]; !ok { + c.config.Current.InterfacePeers[sintf] = []string{addr} + } else { + c.config.Current.InterfacePeers[sintf] = append(c.config.Current.InterfacePeers[sintf], addr) + } } c.config.Mutex.Unlock() return nil diff --git a/src/yggdrasil/core.go b/src/yggdrasil/core.go index 884ef9e0..907db699 100644 --- a/src/yggdrasil/core.go +++ b/src/yggdrasil/core.go @@ -21,16 +21,17 @@ type Core struct { // We're going to keep our own copy of the provided config - that way we can // guarantee that it will be covered by the mutex phony.Inbox - config config.NodeState // Config - boxPub crypto.BoxPubKey - boxPriv crypto.BoxPrivKey - sigPub crypto.SigPubKey - sigPriv crypto.SigPrivKey - switchTable switchTable - peers peers - router router - link link - log *log.Logger + config config.NodeState // Config + boxPub crypto.BoxPubKey + boxPriv crypto.BoxPrivKey + sigPub crypto.SigPubKey + sigPriv crypto.SigPrivKey + switchTable switchTable + peers peers + router router + link link + log *log.Logger + addPeerTimer *time.Timer } func (c *Core) _init() error { @@ -91,7 +92,7 @@ func (c *Core) _addPeerLoop() { // Add peers from the Peers section for _, peer := range current.Peers { - if err := c.AddPeer(peer, ""); err != nil { + if err := c.CallPeer(peer, ""); err != nil { c.log.Errorln("Failed to add peer:", err) } } @@ -99,14 +100,14 @@ func (c *Core) _addPeerLoop() { // Add peers from the InterfacePeers section for intf, intfpeers := range current.InterfacePeers { for _, peer := range intfpeers { - if err := c.AddPeer(peer, intf); err != nil { + if err := c.CallPeer(peer, intf); err != nil { c.log.Errorln("Failed to add peer:", err) } } } // Sit for a while - time.AfterFunc(time.Minute, func() { + c.addPeerTimer = time.AfterFunc(time.Minute, func() { c.Act(c, c._addPeerLoop) }) } @@ -187,4 +188,5 @@ func (c *Core) Stop() { // This function is unsafe and should only be ran by the core actor. func (c *Core) _stop() { c.log.Infoln("Stopping...") + c.addPeerTimer.Stop() } From 7b1678a11d6261e4adc9c7c62489bf644f75a3fa Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Thu, 19 Sep 2019 09:04:25 +0100 Subject: [PATCH 2/2] Goroutines in _addPeerLoop from bugfixes --- src/yggdrasil/core.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/yggdrasil/core.go b/src/yggdrasil/core.go index 907db699..42910aa7 100644 --- a/src/yggdrasil/core.go +++ b/src/yggdrasil/core.go @@ -92,17 +92,21 @@ func (c *Core) _addPeerLoop() { // Add peers from the Peers section for _, peer := range current.Peers { - if err := c.CallPeer(peer, ""); err != nil { - c.log.Errorln("Failed to add peer:", err) - } + go func(peer, intf string) { + if err := c.CallPeer(peer, intf); err != nil { + c.log.Errorln("Failed to add peer:", err) + } + }(peer, "") } // Add peers from the InterfacePeers section for intf, intfpeers := range current.InterfacePeers { for _, peer := range intfpeers { - if err := c.CallPeer(peer, intf); err != nil { - c.log.Errorln("Failed to add peer:", err) - } + go func(peer, intf string) { + if err := c.CallPeer(peer, intf); err != nil { + c.log.Errorln("Failed to add peer:", err) + } + }(peer, intf) } }