Skip to content

dht: drop cached peer sessions when the peer's last connection closes #279

Description

@adust09

Current state

dhtStreams caches one PeerSession (MVar (Maybe StreamIO)) per peer, keyed by PeerId. Nothing ever removes an entry:

  • newDHTNode creates the map,
  • sendRequestViaSwitch / peerSession insert into it,
  • no code deletes from it.

A session survives the connection its stream was opened on. When the peer reconnects, the stale Just stream is tried first, fails, and is replaced — so this is not a correctness bug, just one wasted exchange. The real cost is that the map grows monotonically with the number of distinct peers a node has ever talked to, and each entry pins a dead StreamIO closure until that peer is contacted again. On a long-running node with high churn (a DHT server sees a lot of one-off peers) neither the map nor the retained streams are ever reclaimed.

go-libp2p handles this explicitly: messageSenderImpl removes the peerMessageSender from strmap and calls invalidate() on it, which sets ms.invalid = true and ms.s.Reset() — the comment there notes this "prevents the peerMessageSender from being reused/reinitialized and then forgotten (leaving the stream open)".

This was deliberately left out of scope by #253, which only serialized the exchange.

Scope

  • Register a swDisconnectNotifiers callback (as NAT.hs and NAT/Relay/Transport.hs already do) so the DHT learns when a peer's connection is torn down.
  • Remove the peer's session from dhtStreams and close the stream it holds.
  • Only drop the session when the peer has no remaining open connection, not on any single connection teardown.
  • Do not tear a session out from under a caller currently holding it — an in-flight exchange must finish or fail on its own terms, and must not resurrect the removed entry afterwards.
  • Deregister the notifier when the DHT node or Switch closes, so a stopped node does not keep a callback alive.

Notes

The "must not resurrect" constraint is the interesting part: peerSession is get-or-create, so a caller holding a session that has just been removed will happily put its stream back into an MVar no longer reachable from the map. That is harmless for correctness (the next caller creates a fresh session) but means the close has to be driven from the session itself rather than only from the map.

Acceptance tests

  • A peer's session is removed and its stream closed after the last connection to that peer closes.
  • A peer with a second open connection keeps its session when the first closes.
  • An exchange in flight when the disconnect fires completes or fails without leaving a live stream behind.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions