From eb453bde265c8fdaa470cb3e5a248937dcd58bd4 Mon Sep 17 00:00:00 2001
From: Hans Moog <hm@mkjc.net>
Date: Tue, 23 Apr 2019 19:49:52 +0200
Subject: [PATCH] Refactor: refactored the code to remove race conditions

---
 packages/database/logger.go                   |  4 ++
 plugins/analysis/client/plugin.go             |  6 +-
 .../instances/acceptedneighbors/instance.go   |  5 ++
 .../instances/chosenneighbors/instance.go     |  5 ++
 .../instances/knownpeers/instance.go          |  6 +-
 .../instances/neighborhood/instance.go        | 12 ++--
 plugins/autopeering/plugin.go                 |  7 ++-
 .../protocol/incoming_request_processor.go    | 63 ++++++++++++++-----
 .../protocol/outgoing_request_processor.go    |  7 ++-
 .../types/peerregister/peer_register.go       | 30 ++++++---
 .../neighbormanager/accepted_neighbors.go     |  7 ---
 .../neighbormanager/chosen_neighbors.go       |  7 ---
 .../gossip/neighbormanager/neighbormanager.go |  3 -
 plugins/statusscreen/ui_header_bar.go         |  7 ++-
 14 files changed, 104 insertions(+), 65 deletions(-)
 create mode 100644 plugins/autopeering/instances/acceptedneighbors/instance.go
 create mode 100644 plugins/autopeering/instances/chosenneighbors/instance.go
 delete mode 100644 plugins/gossip/neighbormanager/accepted_neighbors.go
 delete mode 100644 plugins/gossip/neighbormanager/chosen_neighbors.go
 delete mode 100644 plugins/gossip/neighbormanager/neighbormanager.go

diff --git a/packages/database/logger.go b/packages/database/logger.go
index e8a4c6fd..72b6a72e 100644
--- a/packages/database/logger.go
+++ b/packages/database/logger.go
@@ -13,3 +13,7 @@ func (this *logger) Infof(string, ...interface{}) {
 func (this *logger) Warningf(string, ...interface{}) {
     // disable logging
 }
+
+func (this *logger) Debugf(string, ...interface{}) {
+	// disable logging
+}
diff --git a/plugins/analysis/client/plugin.go b/plugins/analysis/client/plugin.go
index 3e589f47..4463dce8 100644
--- a/plugins/analysis/client/plugin.go
+++ b/plugins/analysis/client/plugin.go
@@ -8,11 +8,11 @@ import (
     "github.com/iotaledger/goshimmer/plugins/analysis/types/addnode"
     "github.com/iotaledger/goshimmer/plugins/analysis/types/connectnodes"
     "github.com/iotaledger/goshimmer/plugins/analysis/types/ping"
+    "github.com/iotaledger/goshimmer/plugins/autopeering/instances/chosenneighbors"
     "github.com/iotaledger/goshimmer/plugins/autopeering/protocol"
     "github.com/iotaledger/goshimmer/plugins/autopeering/types/peer"
     "github.com/iotaledger/goshimmer/plugins/autopeering/types/request"
     "github.com/iotaledger/goshimmer/plugins/autopeering/types/response"
-    "github.com/iotaledger/goshimmer/plugins/gossip/neighbormanager"
     "net"
     "time"
 )
@@ -88,10 +88,10 @@ func setupHooks(conn *network.ManagedConnection, eventDispatchers *EventDispatch
 }
 
 func reportChosenNeighbors(dispatchers *EventDispatchers) {
-    for _, chosenNeighbor := range neighbormanager.CHOSEN_NEIGHBORS {
+    for _, chosenNeighbor := range chosenneighbors.INSTANCE.Peers {
         dispatchers.AddNode(chosenNeighbor.Identity.Identifier)
     }
-    for _, chosenNeighbor := range neighbormanager.CHOSEN_NEIGHBORS {
+    for _, chosenNeighbor := range chosenneighbors.INSTANCE.Peers {
         dispatchers.ConnectNodes(accountability.OWN_ID.Identifier, chosenNeighbor.Identity.Identifier)
     }
 }
diff --git a/plugins/autopeering/instances/acceptedneighbors/instance.go b/plugins/autopeering/instances/acceptedneighbors/instance.go
new file mode 100644
index 00000000..1990c8a3
--- /dev/null
+++ b/plugins/autopeering/instances/acceptedneighbors/instance.go
@@ -0,0 +1,5 @@
+package acceptedneighbors
+
+import "github.com/iotaledger/goshimmer/plugins/autopeering/types/peerregister"
+
+var INSTANCE = peerregister.New()
diff --git a/plugins/autopeering/instances/chosenneighbors/instance.go b/plugins/autopeering/instances/chosenneighbors/instance.go
new file mode 100644
index 00000000..c7deb649
--- /dev/null
+++ b/plugins/autopeering/instances/chosenneighbors/instance.go
@@ -0,0 +1,5 @@
+package chosenneighbors
+
+import "github.com/iotaledger/goshimmer/plugins/autopeering/types/peerregister"
+
+var INSTANCE = peerregister.New()
diff --git a/plugins/autopeering/instances/knownpeers/instance.go b/plugins/autopeering/instances/knownpeers/instance.go
index baf11fef..43637ca1 100644
--- a/plugins/autopeering/instances/knownpeers/instance.go
+++ b/plugins/autopeering/instances/knownpeers/instance.go
@@ -6,14 +6,14 @@ import (
     "github.com/iotaledger/goshimmer/plugins/autopeering/types/peerregister"
 )
 
-var INSTANCE = initKnownPeers()
+var INSTANCE *peerregister.PeerRegister
 
 func Configure(plugin *node.Plugin) {
     INSTANCE = initKnownPeers()
 }
 
-func initKnownPeers() peerregister.PeerRegister {
-    knownPeers := make(peerregister.PeerRegister)
+func initKnownPeers() *peerregister.PeerRegister {
+    knownPeers := peerregister.New()
     for _, entryNode := range entrynodes.INSTANCE {
         knownPeers.AddOrUpdate(entryNode)
     }
diff --git a/plugins/autopeering/instances/neighborhood/instance.go b/plugins/autopeering/instances/neighborhood/instance.go
index ea3da933..8bc69db0 100644
--- a/plugins/autopeering/instances/neighborhood/instance.go
+++ b/plugins/autopeering/instances/neighborhood/instance.go
@@ -11,16 +11,16 @@ import (
     "time"
 )
 
-var INSTANCE peerregister.PeerRegister
+var INSTANCE *peerregister.PeerRegister
 
 var LIST_INSTANCE peerlist.PeerList
 
 // Selects a fixed neighborhood from all known peers - this allows nodes to "stay in the same circles" that share their
 // view on the ledger an is a preparation for economic clustering
-var NEIGHBORHOOD_SELECTOR = func(this peerregister.PeerRegister, req *request.Request) peerregister.PeerRegister {
-    filteredPeers := make(peerregister.PeerRegister)
-    for id, peer := range this {
-        filteredPeers[id] = peer
+var NEIGHBORHOOD_SELECTOR = func(this *peerregister.PeerRegister, req *request.Request) *peerregister.PeerRegister {
+    filteredPeers := peerregister.New()
+    for id, peer := range this.Peers {
+        filteredPeers.Peers[id] = peer
     }
 
     return filteredPeers
@@ -35,7 +35,7 @@ func Configure(plugin *node.Plugin) {
 }
 
 func updateNeighborHood() {
-    if float64(len(INSTANCE)) * 1.2 <= float64(len(knownpeers.INSTANCE)) || lastUpdate.Before(time.Now().Add(-300 * time.Second)) {
+    if INSTANCE == nil || float64(len(INSTANCE.Peers)) * 1.2 <= float64(len(knownpeers.INSTANCE.Peers)) || lastUpdate.Before(time.Now().Add(-300 * time.Second)) {
         INSTANCE = knownpeers.INSTANCE.Filter(NEIGHBORHOOD_SELECTOR, outgoingrequest.INSTANCE)
         LIST_INSTANCE = INSTANCE.List()
 
diff --git a/plugins/autopeering/plugin.go b/plugins/autopeering/plugin.go
index a4f5f6ae..b7cb7fca 100644
--- a/plugins/autopeering/plugin.go
+++ b/plugins/autopeering/plugin.go
@@ -4,13 +4,14 @@ import (
     "github.com/iotaledger/goshimmer/packages/daemon"
     "github.com/iotaledger/goshimmer/packages/node"
     "github.com/iotaledger/goshimmer/plugins/autopeering/instances"
+    "github.com/iotaledger/goshimmer/plugins/autopeering/instances/acceptedneighbors"
+    "github.com/iotaledger/goshimmer/plugins/autopeering/instances/chosenneighbors"
     "github.com/iotaledger/goshimmer/plugins/autopeering/instances/knownpeers"
     "github.com/iotaledger/goshimmer/plugins/autopeering/protocol"
     "github.com/iotaledger/goshimmer/plugins/autopeering/types/peer"
     "github.com/iotaledger/goshimmer/plugins/autopeering/types/request"
     "github.com/iotaledger/goshimmer/plugins/autopeering/types/response"
     "github.com/iotaledger/goshimmer/plugins/autopeering/server"
-    "github.com/iotaledger/goshimmer/plugins/gossip/neighbormanager"
 )
 
 func configure(plugin *node.Plugin) {
@@ -23,13 +24,13 @@ func configure(plugin *node.Plugin) {
     })
 
     protocol.Events.IncomingRequestAccepted.Attach(func(req *request.Request) {
-        if neighbormanager.ACCEPTED_NEIGHBORS.AddOrUpdate(req.Issuer) {
+        if acceptedneighbors.INSTANCE.AddOrUpdate(req.Issuer) {
             plugin.LogSuccess("new neighbor accepted: " + req.Issuer.Address.String() + " / " + req.Issuer.Identity.StringIdentifier)
         }
     })
 
     protocol.Events.OutgoingRequestAccepted.Attach(func(res *response.Response) {
-        if neighbormanager.CHOSEN_NEIGHBORS.AddOrUpdate(res.Issuer) {
+        if chosenneighbors.INSTANCE.AddOrUpdate(res.Issuer) {
             plugin.LogSuccess("new neighbor chosen: " + res.Issuer.Address.String() + " / " + res.Issuer.Identity.StringIdentifier)
         }
     })
diff --git a/plugins/autopeering/protocol/incoming_request_processor.go b/plugins/autopeering/protocol/incoming_request_processor.go
index ffc2c283..5d5f2bbe 100644
--- a/plugins/autopeering/protocol/incoming_request_processor.go
+++ b/plugins/autopeering/protocol/incoming_request_processor.go
@@ -2,44 +2,73 @@ package protocol
 
 import (
     "github.com/iotaledger/goshimmer/packages/node"
+    "github.com/iotaledger/goshimmer/plugins/autopeering/instances/acceptedneighbors"
     "github.com/iotaledger/goshimmer/plugins/autopeering/instances/chosenneighborcandidates"
     "github.com/iotaledger/goshimmer/plugins/autopeering/instances/neighborhood"
     "github.com/iotaledger/goshimmer/plugins/autopeering/protocol/constants"
     "github.com/iotaledger/goshimmer/plugins/autopeering/types/peer"
     "github.com/iotaledger/goshimmer/plugins/autopeering/types/peerlist"
     "github.com/iotaledger/goshimmer/plugins/autopeering/types/request"
-    "github.com/iotaledger/goshimmer/plugins/gossip/neighbormanager"
 )
 
 func createIncomingRequestProcessor(plugin *node.Plugin) func(req *request.Request) {
     return func(req *request.Request) {
-        plugin.LogDebug("received peering request from " + req.Issuer.String())
+        go processIncomingRequest(plugin, req)
+    }
+}
 
-        Events.DiscoverPeer.Trigger(req.Issuer)
+func processIncomingRequest(plugin *node.Plugin, req *request.Request) {
+    plugin.LogDebug("received peering request from " + req.Issuer.String())
 
-        //distanceFn := chosenneighborcandidates.DISTANCE(ownpeer.INSTANCE)
+    Events.DiscoverPeer.Trigger(req.Issuer)
 
-        if len(neighbormanager.ACCEPTED_NEIGHBORS) <= constants.NEIGHBOR_COUNT / 2 {
-            if err := req.Accept(proposedPeeringCandidates(req)); err != nil {
-                plugin.LogDebug("error when sending response to" + req.Issuer.String())
-            }
+    if requestingNodeIsCloser(req) {
+        acceptedneighbors.INSTANCE.Lock.Lock()
+        defer acceptedneighbors.INSTANCE.Lock.Unlock()
 
-            plugin.LogDebug("sent positive peering response to " + req.Issuer.String())
-
-            Events.IncomingRequestAccepted.Trigger(req)
-        } else {
-            if err := req.Reject(proposedPeeringCandidates(req)); err != nil {
-                plugin.LogDebug("error when sending response to" + req.Issuer.String())
+        if requestingNodeIsCloser(req) {
+            acceptedneighbors.INSTANCE.AddOrUpdate(req.Issuer)
+            if len(acceptedneighbors.INSTANCE.Peers) > constants.NEIGHBOR_COUNT / 2 {
+                // drop further away node
             }
 
-            plugin.LogDebug("sent negative peering response to " + req.Issuer.String())
+            acceptRequest(plugin, req)
 
-            Events.IncomingRequestRejected.Trigger(req)
+            return
         }
     }
+
+    rejectRequest(plugin, req)
+}
+
+func requestingNodeIsCloser(req *request.Request) bool {
+    //distanceFn := chosenneighborcandidates.DISTANCE(ownpeer.INSTANCE)
+
+    return len(acceptedneighbors.INSTANCE.Peers) <= constants.NEIGHBOR_COUNT / 2 ||
+        acceptedneighbors.INSTANCE.Contains(req.Issuer.Identity.StringIdentifier)
+}
+
+func acceptRequest(plugin *node.Plugin, req *request.Request) {
+    if err := req.Accept(generateProposedPeeringCandidates(req)); err != nil {
+        plugin.LogDebug("error when sending response to" + req.Issuer.String())
+    }
+
+    plugin.LogDebug("sent positive peering response to " + req.Issuer.String())
+
+    Events.IncomingRequestAccepted.Trigger(req)
+}
+
+func rejectRequest(plugin *node.Plugin, req *request.Request) {
+    if err := req.Reject(generateProposedPeeringCandidates(req)); err != nil {
+        plugin.LogDebug("error when sending response to" + req.Issuer.String())
+    }
+
+    plugin.LogDebug("sent negative peering response to " + req.Issuer.String())
+
+    Events.IncomingRequestRejected.Trigger(req)
 }
 
-func proposedPeeringCandidates(req *request.Request) peerlist.PeerList {
+func generateProposedPeeringCandidates(req *request.Request) peerlist.PeerList {
     return neighborhood.LIST_INSTANCE.Filter(func(p *peer.Peer) bool {
         return p.Identity.PublicKey != nil
     }).Sort(chosenneighborcandidates.DISTANCE(req.Issuer))
diff --git a/plugins/autopeering/protocol/outgoing_request_processor.go b/plugins/autopeering/protocol/outgoing_request_processor.go
index 4edccbdd..7844125b 100644
--- a/plugins/autopeering/protocol/outgoing_request_processor.go
+++ b/plugins/autopeering/protocol/outgoing_request_processor.go
@@ -4,13 +4,14 @@ import (
     "github.com/iotaledger/goshimmer/packages/accountability"
     "github.com/iotaledger/goshimmer/packages/daemon"
     "github.com/iotaledger/goshimmer/packages/node"
+    "github.com/iotaledger/goshimmer/plugins/autopeering/instances/acceptedneighbors"
     "github.com/iotaledger/goshimmer/plugins/autopeering/instances/chosenneighborcandidates"
+    "github.com/iotaledger/goshimmer/plugins/autopeering/instances/chosenneighbors"
     "github.com/iotaledger/goshimmer/plugins/autopeering/instances/outgoingrequest"
     "github.com/iotaledger/goshimmer/plugins/autopeering/protocol/constants"
     "github.com/iotaledger/goshimmer/plugins/autopeering/types/peer"
     "github.com/iotaledger/goshimmer/plugins/autopeering/protocol/types"
     "github.com/iotaledger/goshimmer/plugins/autopeering/server/tcp"
-    "github.com/iotaledger/goshimmer/plugins/gossip/neighbormanager"
     "time"
 )
 
@@ -43,8 +44,8 @@ func sendOutgoingRequests(plugin *node.Plugin) {
         go func(peer *peer.Peer) {
             nodeId := peer.Identity.StringIdentifier
 
-            if !neighbormanager.ACCEPTED_NEIGHBORS.Contains(nodeId) &&
-                !neighbormanager.CHOSEN_NEIGHBORS.Contains(nodeId) &&
+            if !acceptedneighbors.INSTANCE.Contains(nodeId) &&
+                !chosenneighbors.INSTANCE.Contains(nodeId) &&
                 accountability.OWN_ID.StringIdentifier != nodeId {
 
                 if dialed, err := peer.Send(outgoingrequest.INSTANCE.Marshal(), types.PROTOCOL_TYPE_TCP, true); err != nil {
diff --git a/plugins/autopeering/types/peerregister/peer_register.go b/plugins/autopeering/types/peerregister/peer_register.go
index 96c27f73..68fec0d3 100644
--- a/plugins/autopeering/types/peerregister/peer_register.go
+++ b/plugins/autopeering/types/peerregister/peer_register.go
@@ -6,17 +6,27 @@ import (
     "github.com/iotaledger/goshimmer/plugins/autopeering/types/peer"
     "github.com/iotaledger/goshimmer/plugins/autopeering/types/request"
     "github.com/iotaledger/goshimmer/plugins/autopeering/types/peerlist"
+    "sync"
 )
 
-type PeerRegister map[string]*peer.Peer
+type PeerRegister struct {
+    Peers map[string]*peer.Peer
+    Lock sync.RWMutex
+}
+
+func New() *PeerRegister {
+    return &PeerRegister{
+        Peers: make(map[string]*peer.Peer),
+    }
+}
 
 // returns true if a new entry was added
-func (this PeerRegister) AddOrUpdate(peer *peer.Peer) bool {
+func (this *PeerRegister) AddOrUpdate(peer *peer.Peer) bool {
     if peer.Identity == nil || bytes.Equal(peer.Identity.Identifier, accountability.OWN_ID.Identifier) {
         return false
     }
 
-    if existingPeer, exists := this[peer.Identity.StringIdentifier]; exists {
+    if existingPeer, exists := this.Peers[peer.Identity.StringIdentifier]; exists {
         existingPeer.Address = peer.Address
         existingPeer.GossipPort = peer.GossipPort
         existingPeer.PeeringPort = peer.PeeringPort
@@ -25,7 +35,7 @@ func (this PeerRegister) AddOrUpdate(peer *peer.Peer) bool {
 
         return false
     } else {
-        this[peer.Identity.StringIdentifier] = peer
+        this.Peers[peer.Identity.StringIdentifier] = peer
 
         // trigger add peer
 
@@ -33,23 +43,23 @@ func (this PeerRegister) AddOrUpdate(peer *peer.Peer) bool {
     }
 }
 
-func (this PeerRegister) Contains(key string) bool {
-    if _, exists := this[key]; exists {
+func (this *PeerRegister) Contains(key string) bool {
+    if _, exists := this.Peers[key]; exists {
         return true
     } else {
         return false
     }
 }
 
-func (this PeerRegister) Filter(filterFn func(this PeerRegister, req *request.Request) PeerRegister, req *request.Request) PeerRegister {
+func (this *PeerRegister) Filter(filterFn func(this *PeerRegister, req *request.Request) *PeerRegister, req *request.Request) *PeerRegister {
     return filterFn(this, req)
 }
 
-func (this PeerRegister) List() peerlist.PeerList {
-    peerList := make(peerlist.PeerList, len(this))
+func (this *PeerRegister) List() peerlist.PeerList {
+    peerList := make(peerlist.PeerList, len(this.Peers))
 
     counter := 0
-    for _, currentPeer := range this {
+    for _, currentPeer := range this.Peers {
         peerList[counter] = currentPeer
         counter++
     }
diff --git a/plugins/gossip/neighbormanager/accepted_neighbors.go b/plugins/gossip/neighbormanager/accepted_neighbors.go
deleted file mode 100644
index 46c34007..00000000
--- a/plugins/gossip/neighbormanager/accepted_neighbors.go
+++ /dev/null
@@ -1,7 +0,0 @@
-package neighbormanager
-
-import (
-    "github.com/iotaledger/goshimmer/plugins/autopeering/types/peerregister"
-)
-
-var ACCEPTED_NEIGHBORS = make(peerregister.PeerRegister)
diff --git a/plugins/gossip/neighbormanager/chosen_neighbors.go b/plugins/gossip/neighbormanager/chosen_neighbors.go
deleted file mode 100644
index 974693a5..00000000
--- a/plugins/gossip/neighbormanager/chosen_neighbors.go
+++ /dev/null
@@ -1,7 +0,0 @@
-package neighbormanager
-
-import (
-    "github.com/iotaledger/goshimmer/plugins/autopeering/types/peerregister"
-)
-
-var CHOSEN_NEIGHBORS = make(peerregister.PeerRegister)
diff --git a/plugins/gossip/neighbormanager/neighbormanager.go b/plugins/gossip/neighbormanager/neighbormanager.go
deleted file mode 100644
index a664339e..00000000
--- a/plugins/gossip/neighbormanager/neighbormanager.go
+++ /dev/null
@@ -1,3 +0,0 @@
-package neighbormanager
-
-
diff --git a/plugins/statusscreen/ui_header_bar.go b/plugins/statusscreen/ui_header_bar.go
index 2b4ac69c..60394449 100644
--- a/plugins/statusscreen/ui_header_bar.go
+++ b/plugins/statusscreen/ui_header_bar.go
@@ -4,9 +4,10 @@ import (
     "fmt"
     "github.com/gdamore/tcell"
     "github.com/iotaledger/goshimmer/packages/accountability"
+    "github.com/iotaledger/goshimmer/plugins/autopeering/instances/acceptedneighbors"
+    "github.com/iotaledger/goshimmer/plugins/autopeering/instances/chosenneighbors"
     "github.com/iotaledger/goshimmer/plugins/autopeering/instances/knownpeers"
     "github.com/iotaledger/goshimmer/plugins/autopeering/instances/neighborhood"
-    "github.com/iotaledger/goshimmer/plugins/gossip/neighbormanager"
     "github.com/rivo/tview"
     "math"
     "strconv"
@@ -66,9 +67,9 @@ func (headerBar *UIHeaderBar) Update() {
     fmt.Fprintln(headerBar.InfoContainer)
     fmt.Fprintf(headerBar.InfoContainer, "[::b]Node ID: [::d]%40v  ", accountability.OWN_ID.StringIdentifier)
     fmt.Fprintln(headerBar.InfoContainer)
-    fmt.Fprintf(headerBar.InfoContainer, "[::b]Neighbors: [::d]%40v  ", strconv.Itoa(len(neighbormanager.CHOSEN_NEIGHBORS)) + " chosen / " + strconv.Itoa(len(neighbormanager.ACCEPTED_NEIGHBORS)) + " accepted")
+    fmt.Fprintf(headerBar.InfoContainer, "[::b]Neighbors: [::d]%40v  ", strconv.Itoa(len(chosenneighbors.INSTANCE.Peers)) + " chosen / " + strconv.Itoa(len(acceptedneighbors.INSTANCE.Peers)) + " accepted")
     fmt.Fprintln(headerBar.InfoContainer)
-    fmt.Fprintf(headerBar.InfoContainer, "[::b]Known Peers: [::d]%40v  ", strconv.Itoa(len(knownpeers.INSTANCE)) + " total / " + strconv.Itoa(len(neighborhood.INSTANCE)) + " neighborhood")
+    fmt.Fprintf(headerBar.InfoContainer, "[::b]Known Peers: [::d]%40v  ", strconv.Itoa(len(knownpeers.INSTANCE.Peers)) + " total / " + strconv.Itoa(len(neighborhood.INSTANCE.Peers)) + " neighborhood")
     fmt.Fprintln(headerBar.InfoContainer)
     fmt.Fprintf(headerBar.InfoContainer, "[::b]Uptime: [::d]");
 
-- 
GitLab