Fix peer tagging #25
Fix peer tagging #25
Conversation
Another routine could observe event `Connected` earlier than ConnManager and tag peers, for now, those tags are lost.
| @@ -164,8 +164,12 @@ func (cm *BasicConnMgr) TagPeer(p peer.ID, tag string, val int) { | |||
|
|
|||
| pi, ok := cm.peers[p] | |||
| if !ok { | |||
| log.Info("tried to tag conn from untracked peer: ", p) | |||
| return | |||
| pi = &peerInfo{ | |||
There was a problem hiding this comment.
So, unfortunately, this will leak memory if we:
- Connect.
- Disconnect.
- Tag the peer.
A real fix will, unfortunately, require access to a Host and/or Network object (to check if we're currently connected. Unfortunately... that requires a much larger refactor (which is why I haven't bothered fixing this...).
There was a problem hiding this comment.
But as long as Connect and Disconnect are paired, the calls from a rational caller to TagPeer and UntagPeer are paired. So maybe the situation is improved a little by this patch.
I'm not self-bloating, but you should really consider refactor the whole system like I said here: libp2p/go-libp2p#467 (comment)
There was a problem hiding this comment.
But as long as Connect and Disconnect are paired, the calls from a rational caller to TagPeer and UntagPeer are paired. So maybe the situation is improved a little by this patch.
Not necessarily. Peers are sometimes tagged within Connect/Disconnect hooks but many services tag peers on an as-needed basis (and may never explicitly untag them). The expectation is that the connection manager will clean up any tags after a final disconnect.
|
I believe the effect this PR was trying to achieve was solved in #39 (buffering early tags). Please reopen if there’s still work to do. |
Another routine could observe event
Connectedearlier thanConnManagerand tag peers, for now, those tags are lost.Logs like
tried to tag conn from untracked peer: <peer.ID Qm*3JXWfP>could be found.