Skip to content

Commit

Permalink
identify: Fix memory leak of unused pending_opens (#273)
Browse files Browse the repository at this point in the history
The identify protocol implementation leaked SubstreamIds and PeerIds via
the `pending_opens` hashMap.
Objects were only inserted in the `pending_opens`, however they were
never removed.

The only possible purpose of `pending_opens` is to double-check the
events coming from the service layer: `TransportEvent::SubstreamOpened`.
However this is not needed, as illustrated by the current
implementation.

Part of endeavors to fix memory leaks:
paritytech/polkadot-sdk#6149

### Testing Done

- custom patched litep2p to dump the internal state of identify protocol
running in kusama

cc @paritytech/networking

Signed-off-by: Alexandru Vasile <[email protected]>
  • Loading branch information
lexnv authored Oct 24, 2024
1 parent 8888d07 commit 0084dc3
Showing 1 changed file with 1 addition and 6 deletions.
7 changes: 1 addition & 6 deletions src/protocol/libp2p/identify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,9 +180,6 @@ pub(crate) struct Identify {
/// Protocols supported by the local node, filled by `Litep2p`.
protocols: Vec<String>,

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

/// Pending outbound substreams.
pending_outbound: FuturesUnordered<BoxFuture<'static, crate::Result<IdentifyResponse>>>,

Expand All @@ -200,7 +197,6 @@ impl Identify {
public: config.public.expect("public key to be supplied"),
protocol_version: config.protocol_version,
user_agent: config.user_agent.unwrap_or(DEFAULT_AGENT.to_string()),
pending_opens: HashMap::new(),
pending_inbound: FuturesUnordered::new(),
pending_outbound: FuturesUnordered::new(),
protocols: config.protocols.iter().map(|protocol| protocol.to_string()).collect(),
Expand All @@ -211,8 +207,7 @@ impl Identify {
fn on_connection_established(&mut self, peer: PeerId, endpoint: Endpoint) -> crate::Result<()> {
tracing::trace!(target: LOG_TARGET, ?peer, ?endpoint, "connection established");

let substream_id = self.service.open_substream(peer)?;
self.pending_opens.insert(substream_id, peer);
self.service.open_substream(peer)?;
self.peers.insert(peer, endpoint);

Ok(())
Expand Down

0 comments on commit 0084dc3

Please sign in to comment.