diff --git a/router/connection_maker.go b/router/connection_maker.go index c771bc20..d8e34e55 100644 --- a/router/connection_maker.go +++ b/router/connection_maker.go @@ -206,8 +206,13 @@ func (cm *ConnectionMaker) ourConnections() (PeerNameSet, map[string]struct{}, m func (cm *ConnectionMaker) addPeerTargets(ourConnectedPeers PeerNameSet, addTarget func(string)) { cm.peers.ForEach(func(peer *Peer) { - for conn := range peer.Connections() { - otherPeer := conn.Remote().Name + if peer == cm.ourself.Peer { + return + } + // 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. + for otherPeer, conn := range peer.connections { if otherPeer == cm.ourself.Name { continue } diff --git a/router/json.go b/router/json.go index 2d01e8f6..9461c014 100644 --- a/router/json.go +++ b/router/json.go @@ -35,8 +35,30 @@ func (cache *MacCache) MarshalJSON() ([]byte, error) { } func (peers *Peers) MarshalJSON() ([]byte, error) { - var ps []*Peer - peers.ForEach(func(peer *Peer) { ps = append(ps, peer) }) + type p struct { + Name string + NickName string + UID uint64 + Version uint64 + Connections []Connection + } + var ps []*p + peers.ForEach(func(peer *Peer) { + var connections []Connection + if peer == peers.ourself.Peer { + for conn := range peers.ourself.Connections() { + connections = append(connections, conn) + } + } else { + // 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. + for _, conn := range peer.connections { + connections = append(connections, conn) + } + } + ps = append(ps, &p{peer.Name.String(), peer.NickName, peer.UID, peer.version, connections}) + }) return json.Marshal(ps) } @@ -63,21 +85,6 @@ func (routes *Routes) MarshalJSON() ([]byte, error) { return json.Marshal(r) } -func (peer *Peer) MarshalJSON() ([]byte, error) { - conns := peer.Connections() - connections := make([]Connection, 0, len(conns)) - for conn := range conns { - connections = append(connections, conn) - } - return json.Marshal(struct { - Name string - NickName string - UID uint64 - Version uint64 - Connections []Connection - }{peer.Name.String(), peer.NickName, peer.UID, peer.version, connections}) -} - func (conn *RemoteConnection) MarshalJSON() ([]byte, error) { return json.Marshal(struct { Name string diff --git a/router/local_peer.go b/router/local_peer.go index bfc21823..239fce4c 100644 --- a/router/local_peer.go +++ b/router/local_peer.go @@ -75,6 +75,16 @@ func (peer *LocalPeer) RelayBroadcast(srcPeer *Peer, df bool, frame []byte, dec } } +func (peer *LocalPeer) Connections() ConnectionSet { + connections := make(ConnectionSet) + peer.RLock() + defer peer.RUnlock() + for _, conn := range peer.connections { + connections[conn] = void + } + return connections +} + func (peer *LocalPeer) ConnectionTo(name PeerName) (Connection, bool) { peer.RLock() defer peer.RUnlock() diff --git a/router/peer.go b/router/peer.go index ffe56580..0ec4172a 100644 --- a/router/peer.go +++ b/router/peer.go @@ -42,16 +42,6 @@ func (peer *Peer) Info() string { return fmt.Sprint(peer.String(), " (v", peer.version, ") (UID ", peer.UID, ")") } -func (peer *Peer) Connections() ConnectionSet { - connections := make(ConnectionSet) - peer.RLock() - defer peer.RUnlock() - for _, conn := range peer.connections { - connections[conn] = void - } - return connections -} - func (peer *Peer) SetVersionAndConnections(version uint64, connections map[PeerName]Connection) { peer.Lock() defer peer.Unlock() diff --git a/router/peers.go b/router/peers.go index 8362e4bd..45e5a75c 100644 --- a/router/peers.go +++ b/router/peers.go @@ -152,14 +152,26 @@ func (peers *Peers) GarbageCollect() []*Peer { func (peers *Peers) String() string { var buf bytes.Buffer + printConnection := func(conn Connection) { + established := "" + if !conn.Established() { + established = " (unestablished)" + } + fmt.Fprintf(&buf, " -> %s [%v%s]\n", conn.Remote(), conn.RemoteTCPAddr(), established) + } peers.ForEach(func(peer *Peer) { fmt.Fprintln(&buf, peer.Info()) - for conn := range peer.Connections() { - established := "" - if !conn.Established() { - established = " (unestablished)" + if peer == peers.ourself.Peer { + for conn := range peers.ourself.Connections() { + printConnection(conn) + } + } else { + // 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. + for _, conn := range peer.connections { + printConnection(conn) } - fmt.Fprintf(&buf, " -> %s [%v%s]\n", conn.Remote(), conn.RemoteTCPAddr(), established) } }) return buf.String() diff --git a/router/router.go b/router/router.go index dcd33f44..753853fc 100644 --- a/router/router.go +++ b/router/router.go @@ -66,7 +66,7 @@ func NewRouter(config RouterConfig, name PeerName, nickName string) *Router { router.Macs = NewMacCache(macMaxAge, onMacExpiry) router.Peers = NewPeers(router.Ourself, onPeerGC) router.Peers.FetchWithDefault(router.Ourself.Peer) - router.Routes = NewRoutes(router.Ourself.Peer, router.Peers) + router.Routes = NewRoutes(router.Ourself, router.Peers) router.ConnectionMaker = NewConnectionMaker(router.Ourself, router.Peers, router.Port) router.TopologyGossip = router.NewGossip("topology", router) return router diff --git a/router/routes.go b/router/routes.go index 1f50f213..92197fb5 100644 --- a/router/routes.go +++ b/router/routes.go @@ -9,7 +9,7 @@ import ( type Routes struct { sync.RWMutex - ourself *Peer + ourself *LocalPeer peers *Peers unicast map[PeerName]PeerName unicastAll map[PeerName]PeerName // [1] @@ -21,7 +21,7 @@ type Routes struct { // symmetric ones } -func NewRoutes(ourself *Peer, peers *Peers) *Routes { +func NewRoutes(ourself *LocalPeer, peers *Peers) *Routes { routes := &Routes{ ourself: ourself, peers: peers, @@ -219,7 +219,7 @@ func (routes *Routes) calculateBroadcast(establishedAndSymmetric bool) map[PeerN routes.peers.ForEach(func(peer *Peer) { hops := []PeerName{} - if found, reached := peer.Routes(ourself, establishedAndSymmetric); found { + if found, reached := peer.Routes(ourself.Peer, establishedAndSymmetric); found { // This is rather similar to the inner loop on // peer.Routes(...); the main difference is in the // locking.