- get rid of individual Peer locks
- hold Peers read (or write) lock and LocalPeer read lock while
performing route calculations
This eliminates the deadlock resulting from the inconsistent lock
acquisition order in route calculcation.
Fixes#643.
This makes for a more symmetric API - Peers.FetchWithDefault does the
increment and Peers.Dereference does the decrement, whereas previously
the latter was done by Peer.DecrementLocalRefCount.
This does increase lock granularity, i.e. modifying any ref count now
requires a write lock on Peers.
When we want a string representation of a Peer, we nearly always just
want the full name. So make String() return that and introduce Info()
to return expanded info.
Topology gossip can be received on a connection before that connection
has been marked as 'established'. If we do not include unestablished
connections in topology gossip, then parts of the topology can be
removed by gc, since they are deemed unreachable, which can cause
subsequent incremental topology gossips to be rejected due to them
containing references to peers that have been removed by the gc.
Including unestablished connections in topology gossip requires:
- removing the check for 'established' connections in the encoding of
peers for topology gossip,
- adding the 'established' flag to the internal representation of
remote connections,
- adding the 'established' flag to to encoding of connections in
topology gossip,
- treating the *addition* of a connection as an update to the local
peer.
The latter entails
- bumping the peer version on connection addition (as well as on
marking 'established' as previously),
- bumping the peer version *unconditionally* on connection deletion
(rather than only when the connection was 'established', as
previously),
- moving the initial sending of complete topology on the connection,
and the broadcasting of a topology update containing the two peers
at the ends of the connection, from the connection 'established'
logic to the connection addition logic,
- broadcasting just an topology update for the local peer when a
connection gets marked as 'established',
- broadcasting a topology update for the local peer *unconditionally*
when a connection is removed (rather than only doing this when the
connection was 'established', as previously).
This was rather impenetrable and had some significant code
duplication. So add some words of explanation, and refactor to remove
the duplication.
Some fly-by fixing of minor bug too: buildUnicastRoutes failed to
exclude asymmetric connections from the first hop.
Golang locks aren't re-entrant. As result one can get deadlocks on
RWMutexes like this:
goroutine #1: attempt to acquire read lock -> success
goroutine #2: attempt to acquire write lock -> wait
goroutine #1: attempt to acquire read lock again -> wait
(a pending write lock prevents subsequent acquisitions of read locks)
Peer.HasPathTo could run into this since it could attempt to acquire
the read lock on a peer multiple times if previously it had
encountered that peer via an asymmetric connection.
Matthias Radestock