From b0cea10d1a51ec211f5beeda875a6436422732ed Mon Sep 17 00:00:00 2001
From: Steven Allen <steven@stebalien.com>
Date: Tue, 9 Jun 2020 18:51:35 -0700
Subject: [PATCH] fix: avoid taking accessing the peerQueues without taking the
 lock

Or, really, just avoid accessing it. We don't need it.

This caused a concurrent map access panic under load.
---
 internal/peermanager/peermanager.go            | 5 +----
 internal/session/session.go                    | 2 +-
 internal/session/session_test.go               | 4 +---
 internal/session/sessionwantsender_test.go     | 3 +--
 internal/sessionmanager/sessionmanager_test.go | 2 +-
 5 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/internal/peermanager/peermanager.go b/internal/peermanager/peermanager.go
index 4c489dd..0085762 100644
--- a/internal/peermanager/peermanager.go
+++ b/internal/peermanager/peermanager.go
@@ -198,7 +198,7 @@ func (pm *PeerManager) getOrCreate(p peer.ID) PeerQueue {
 
 // RegisterSession tells the PeerManager that the given session is interested
 // in events about the given peer.
-func (pm *PeerManager) RegisterSession(p peer.ID, s Session) bool {
+func (pm *PeerManager) RegisterSession(p peer.ID, s Session) {
 	pm.psLk.Lock()
 	defer pm.psLk.Unlock()
 
@@ -210,9 +210,6 @@ func (pm *PeerManager) RegisterSession(p peer.ID, s Session) bool {
 		pm.peerSessions[p] = make(map[uint64]struct{})
 	}
 	pm.peerSessions[p][s.ID()] = struct{}{}
-
-	_, ok := pm.peerQueues[p]
-	return ok
 }
 
 // UnregisterSession tells the PeerManager that the given session is no longer
diff --git a/internal/session/session.go b/internal/session/session.go
index 7b2953f..f2a4d2e 100644
--- a/internal/session/session.go
+++ b/internal/session/session.go
@@ -30,7 +30,7 @@ const (
 type PeerManager interface {
 	// RegisterSession tells the PeerManager that the session is interested
 	// in a peer's connection state
-	RegisterSession(peer.ID, bspm.Session) bool
+	RegisterSession(peer.ID, bspm.Session)
 	// UnregisterSession tells the PeerManager that the session is no longer
 	// interested in a peer's connection state
 	UnregisterSession(uint64)
diff --git a/internal/session/session_test.go b/internal/session/session_test.go
index 08bc9f8..b63a20d 100644
--- a/internal/session/session_test.go
+++ b/internal/session/session_test.go
@@ -136,9 +136,7 @@ func newFakePeerManager() *fakePeerManager {
 	}
 }
 
-func (pm *fakePeerManager) RegisterSession(peer.ID, bspm.Session) bool {
-	return true
-}
+func (pm *fakePeerManager) RegisterSession(peer.ID, bspm.Session)                    {}
 func (pm *fakePeerManager) UnregisterSession(uint64)                                 {}
 func (pm *fakePeerManager) SendWants(context.Context, peer.ID, []cid.Cid, []cid.Cid) {}
 func (pm *fakePeerManager) BroadcastWantHaves(ctx context.Context, cids []cid.Cid) {
diff --git a/internal/session/sessionwantsender_test.go b/internal/session/sessionwantsender_test.go
index 806112f..4b39a89 100644
--- a/internal/session/sessionwantsender_test.go
+++ b/internal/session/sessionwantsender_test.go
@@ -59,12 +59,11 @@ func newMockPeerManager() *mockPeerManager {
 	}
 }
 
-func (pm *mockPeerManager) RegisterSession(p peer.ID, sess bspm.Session) bool {
+func (pm *mockPeerManager) RegisterSession(p peer.ID, sess bspm.Session) {
 	pm.lk.Lock()
 	defer pm.lk.Unlock()
 
 	pm.peerSessions[p] = sess
-	return true
 }
 
 func (pm *mockPeerManager) has(p peer.ID, sid uint64) bool {
diff --git a/internal/sessionmanager/sessionmanager_test.go b/internal/sessionmanager/sessionmanager_test.go
index fb8445f..db88855 100644
--- a/internal/sessionmanager/sessionmanager_test.go
+++ b/internal/sessionmanager/sessionmanager_test.go
@@ -64,7 +64,7 @@ type fakePeerManager struct {
 	cancels []cid.Cid
 }
 
-func (*fakePeerManager) RegisterSession(peer.ID, bspm.Session) bool               { return true }
+func (*fakePeerManager) RegisterSession(peer.ID, bspm.Session)                    {}
 func (*fakePeerManager) UnregisterSession(uint64)                                 {}
 func (*fakePeerManager) SendWants(context.Context, peer.ID, []cid.Cid, []cid.Cid) {}
 func (*fakePeerManager) BroadcastWantHaves(context.Context, []cid.Cid)            {}
-- 
GitLab