diff --git a/router/handshake.go b/router/handshake.go index 4a003a19..3e42633a 100644 --- a/router/handshake.go +++ b/router/handshake.go @@ -75,7 +75,7 @@ func (conn *LocalConnection) handshake(enc *gob.Encoder, dec *gob.Decoder, accep return fmt.Errorf("Found unknown remote name: %s at %s", name, conn.remoteTCPAddr) } } - if existingConn, found := conn.local.ConnectionTo(name); found && existingConn.Established() { + if existingConn, found := conn.Router.Ourself.ConnectionTo(name); found && existingConn.Established() { return fmt.Errorf("Already have connection to %s at %s", existingConn.Remote(), existingConn.RemoteTCPAddr()) } uid, err := strconv.ParseUint(uidStr, 10, 64) diff --git a/router/local_peer.go b/router/local_peer.go index 2e4398fe..bfc21823 100644 --- a/router/local_peer.go +++ b/router/local_peer.go @@ -75,6 +75,13 @@ func (peer *LocalPeer) RelayBroadcast(srcPeer *Peer, df bool, frame []byte, dec } } +func (peer *LocalPeer) ConnectionTo(name PeerName) (Connection, bool) { + peer.RLock() + defer peer.RUnlock() + conn, found := peer.connections[name] + return conn, found // yes, you really can't inline that. FFS. +} + func (peer *LocalPeer) ConnectionsTo(names []PeerName) []Connection { conns := make([]Connection, 0, len(names)) peer.RLock() diff --git a/router/peer.go b/router/peer.go index 50b55e9a..1ba834b8 100644 --- a/router/peer.go +++ b/router/peer.go @@ -48,13 +48,6 @@ func (peer *Peer) Version() uint64 { return peer.version } -func (peer *Peer) ConnectionTo(name PeerName) (Connection, bool) { - peer.RLock() - defer peer.RUnlock() - conn, found := peer.connections[name] - return conn, found // yes, you really can't inline that. FFS. -} - func (peer *Peer) Connections() ConnectionSet { connections := make(ConnectionSet) peer.RLock() diff --git a/router/peers_test.go b/router/peers_test.go index c38e6588..daf1e944 100644 --- a/router/peers_test.go +++ b/router/peers_test.go @@ -52,7 +52,7 @@ func TestPeersEncoding(t *testing.T) { case 0: from, to := rand.Intn(numNodes), rand.Intn(numNodes) if from != to { - if _, found := peer[from].ConnectionTo(peer[to].Name); !found { + if _, found := peer[from].connections[peer[to].Name]; !found { ps[from].AddTestConnection(peer[to]) conns = append(conns, struct{ from, to int }{from, to}) checkApplyUpdate(t, ps[from]) diff --git a/router/routes.go b/router/routes.go index 04020954..1f50f213 100644 --- a/router/routes.go +++ b/router/routes.go @@ -231,7 +231,11 @@ func (routes *Routes) calculateBroadcast(establishedAndSymmetric bool) map[PeerN if _, found := reached[remoteName]; found { continue } - if remoteConn, found := conn.Remote().ConnectionTo(ourself.Name); !establishedAndSymmetric || (found && remoteConn.Established()) { + // conn.Remote() cannot by ourself. Modifying + // peer.connections requires a write lock on Peers, + // and since we are holding a read lock (due to the + // ForEach), access without locking the peer is safe. + if remoteConn, found := conn.Remote().connections[ourself.Name]; !establishedAndSymmetric || (found && remoteConn.Established()) { hops = append(hops, remoteName) } }