Skip to content

Commit

Permalink
notification: Fix memory leak of pending substreams (#296)
Browse files Browse the repository at this point in the history
This PR fixes a subtle memory leak that can happen in the following
edge-case situation:
- connection is established and substream outbound is initiated with
remote peer
- the substream ID is tracked until the substream either completes
successfully or fails
- the connection is closed soon after, leading to no substream events
ever being generated

For this edge-cases, we need to remove the tracking of the substream ID
when the connection is reported as closed.

This has been detected after running a node for more than 2 days with
the following generic metrics PR:
- #294

Closes: #295

cc @paritytech/networking

---------

Signed-off-by: Alexandru Vasile <[email protected]>
  • Loading branch information
lexnv authored Dec 3, 2024
1 parent 7612ec9 commit af19055
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/protocol/notification/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ pub(crate) struct NotificationProtocol {
/// Connected peers.
peers: HashMap<PeerId, PeerContext>,

/// Pending outboudn substreams.
/// Pending outbound substreams.
pending_outbound: HashMap<SubstreamId, PeerId>,

/// Handshaking service which reads and writes the handshakes to inbound
Expand Down Expand Up @@ -384,6 +384,8 @@ impl NotificationProtocol {
async fn on_connection_closed(&mut self, peer: PeerId) -> crate::Result<()> {
tracing::trace!(target: LOG_TARGET, ?peer, protocol = %self.protocol, "connection closed");

self.pending_outbound.retain(|_, p| p != &peer);

let Some(context) = self.peers.remove(&peer) else {
tracing::error!(
target: LOG_TARGET,
Expand Down

0 comments on commit af19055

Please sign in to comment.