fix: per-IP connection counter leak blocking reconnection#70
Merged
Conversation
…ailure Two bugs caused the per-IP counter to never decrement, permanently blocking reconnection after ~3 disconnect/reconnect cycles: 1. UpdatePeerId: on duplicate detection, _peerIpMap[realId] was removed, destroying the kept connection's IP mapping. When the kept connection later disconnected, RemoveConnection could not find the IP to decrement. Fix: don't touch _peerIpMap[realId] in the duplicate path. 2. AcceptLoopAsync: if TryAdd failed for a duplicate tempId, the per-IP counter (already incremented) was never decremented. Fix: decrement before disposing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a bug where validators permanently lose the ability to reconnect to peers after ~3 disconnect/reconnect cycles, due to the per-IP connection counter never being decremented.
Two leaks in
TcpTransport:UpdatePeerId(main cause): On duplicate connection detection (simultaneous inbound+outbound),_peerIpMap.TryRemove(realId, ...)destroyed the kept connection's IP mapping. When that connection later disconnected,RemoveConnectioncouldn't find the IP to decrement the counter. Each cycle leaked +1, hitting the limit of 3 after ~3 cycles.AcceptLoopAsync(secondary): IfTryAddfailed for a duplicate temp ID, the already-incremented per-IP counter was never decremented.Fix
UpdatePeerId: Transfer IP mapping torealIdonly after successfulTryAdd. On duplicate path, only decrement the counter for the disposed connection — don't touch_peerIpMap[realId].AcceptLoopAsync: Decrement per-IP counter whenTryAddfails before disposing the connection.Test plan
dotnet test— 110 network tests passing