Skip to content

Commit

Permalink
comment on why we lock when handling (dis)connect notifs
Browse files Browse the repository at this point in the history
  • Loading branch information
Stebalien committed Jan 8, 2018
1 parent 64b46c1 commit 395fb26
Showing 1 changed file with 8 additions and 1 deletion.
9 changes: 8 additions & 1 deletion notif.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ func (nn *netNotifiee) Connected(n inet.Network, v inet.Conn) {
p := v.RemotePeer()
protos, err := dht.peerstore.SupportsProtocols(p, dhtProtocols...)
if err == nil && len(protos) != 0 {
// We lock here for consistency with the lock in testConnection.
// This probably isn't necessary because (dis)connect
// notifications are serialized but it's nice to be consistent.
dht.plk.Lock()
defer dht.plk.Unlock()
if dht.host.Network().Connectedness(p) == inet.Connected {
Expand Down Expand Up @@ -62,9 +65,11 @@ func (nn *netNotifiee) testConnection(v inet.Conn) {
// Remember this choice (makes subsequent negotiations faster)
dht.peerstore.AddProtocols(p, selected)

// We lock here as we race with disconnect. If we didn't lock, we could
// finish processing a connect after handling the associated disconnect
// event and add the peer to the routing table after removing it.
dht.plk.Lock()
defer dht.plk.Unlock()
// Make sure we're still connected under the lock (race with disconnect)
if dht.host.Network().Connectedness(p) == inet.Connected {
dht.Update(dht.Context(), p)
}
Expand All @@ -80,6 +85,8 @@ func (nn *netNotifiee) Disconnected(n inet.Network, v inet.Conn) {

p := v.RemotePeer()

// Lock and check to see if we're still connected. We lock to make sure
// we don't concurrently process a connect event.
dht.plk.Lock()
defer dht.plk.Unlock()
if dht.host.Network().Connectedness(p) == inet.Connected {
Expand Down

0 comments on commit 395fb26

Please sign in to comment.