From 8a5d6298bc49f9e680dd5a395b873b62f6c173e7 Mon Sep 17 00:00:00 2001
From: Luca Moser <moser.luca@gmail.com>
Date: Fri, 7 Feb 2020 09:47:21 +0100
Subject: [PATCH] Fixes deadlock in neighbor manager (#236)

* fixes locks not getting unlocked

* only hold the lock in DropNeighbor while deleted the neighbor entry
---
 packages/gossip/manager.go | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/packages/gossip/manager.go b/packages/gossip/manager.go
index 2a44ddd4..bc3d7222 100644
--- a/packages/gossip/manager.go
+++ b/packages/gossip/manager.go
@@ -78,6 +78,7 @@ func (m *Manager) AddOutbound(p *peer.Peer) error {
 	var srv *server.TCP
 	m.mu.RLock()
 	if m.srv == nil {
+		m.mu.RUnlock()
 		return ErrNotStarted
 	}
 	srv = m.srv
@@ -94,6 +95,7 @@ func (m *Manager) AddInbound(p *peer.Peer) error {
 	var srv *server.TCP
 	m.mu.RLock()
 	if m.srv == nil {
+		m.mu.RUnlock()
 		return ErrNotStarted
 	}
 	srv = m.srv
@@ -104,15 +106,11 @@ func (m *Manager) AddInbound(p *peer.Peer) error {
 
 // NeighborRemoved disconnects the neighbor with the given ID.
 func (m *Manager) DropNeighbor(id peer.ID) error {
-	m.mu.Lock()
-	defer m.mu.Unlock()
-	if _, ok := m.neighbors[id]; !ok {
-		return ErrNotANeighbor
+	n, err := m.removeNeighbor(id)
+	if err != nil {
+		return err
 	}
-	n := m.neighbors[id]
-	delete(m.neighbors, id)
-
-	err := n.Close()
+	err = n.Close()
 	Events.NeighborRemoved.Trigger(n.Peer)
 	return err
 }
@@ -139,12 +137,11 @@ func (m *Manager) SendTransaction(txData []byte, to ...peer.ID) {
 
 func (m *Manager) GetAllNeighbors() []*Neighbor {
 	m.mu.RLock()
+	defer m.mu.RUnlock()
 	result := make([]*Neighbor, 0, len(m.neighbors))
 	for _, n := range m.neighbors {
 		result = append(result, n)
 	}
-	m.mu.RUnlock()
-
 	return result
 }
 
@@ -159,13 +156,12 @@ func (m *Manager) getNeighborsById(ids []peer.ID) []*Neighbor {
 	result := make([]*Neighbor, 0, len(ids))
 
 	m.mu.RLock()
+	defer m.mu.RUnlock()
 	for _, id := range ids {
 		if n, ok := m.neighbors[id]; ok {
 			result = append(result, n)
 		}
 	}
-	m.mu.RUnlock()
-
 	return result
 }
 
@@ -215,6 +211,17 @@ func (m *Manager) addNeighbor(peer *peer.Peer, connectorFunc func(*peer.Peer) (n
 	return nil
 }
 
+func (m *Manager) removeNeighbor(id peer.ID) (*Neighbor, error) {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	if _, ok := m.neighbors[id]; !ok {
+		return nil, ErrNotANeighbor
+	}
+	n := m.neighbors[id]
+	delete(m.neighbors, id)
+	return n, nil
+}
+
 func (m *Manager) handlePacket(data []byte, p *peer.Peer) error {
 	// ignore empty packages
 	if len(data) == 0 {
-- 
GitLab