Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

req-resp: Fix memory leak of pending substreams #297

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

lexnv
Copy link
Collaborator

@lexnv lexnv commented Dec 3, 2024

Similar to #296, there is a possibility of leaking memory in the following edge-case:

  • T0: Connection is established and outbound substream is initiated with peer
    • This maps the substream ID to the request bytes information
  • T1: Connection is closed before the service has a chance to report TransportEvent::SubstreamOpened or TransportEvent::SubstreamOpenFailure

In this case, if we connect and immediately disconnect with a request in flight, we are effectively leaking the request bytes.

Detected by:

Dashboard

  • We are leaking ~111 requests over 3 days timespan:
Screenshot 2024-12-03 at 10 41 01

cc @paritytech/networking

@lexnv lexnv added the bug Something isn't working label Dec 3, 2024
@lexnv lexnv self-assigned this Dec 3, 2024
@lexnv lexnv merged commit 7612ec9 into master Dec 3, 2024
8 checks passed
@lexnv lexnv deleted the lexnv/pending-outbound-req-resp branch December 3, 2024 13:16
lexnv added a commit that referenced this pull request Dec 3, 2024
## [0.8.3] - 2024-12-03

This release includes two fixes for small memory leaks on edge-cases in
the notification and request-response protocols.

### Fixed

- req-resp: Fix memory leak of pending substreams
([#297](#297))
- notification: Fix memory leak of pending substreams
([#296](#296))

cc @paritytech/networking

---------

Signed-off-by: Alexandru Vasile <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
github-merge-queue bot pushed a commit to paritytech/polkadot-sdk that referenced this pull request Dec 4, 2024
## [0.8.3] - 2024-12-03

This release includes two fixes for small memory leaks on edge-cases in
the notification and request-response protocols.

### Fixed

- req-resp: Fix memory leak of pending substreams
([#297](paritytech/litep2p#297))
- notification: Fix memory leak of pending substreams
([#296](paritytech/litep2p#296))

cc @paritytech/networking

---------

Signed-off-by: Alexandru Vasile <[email protected]>
lexnv added a commit to paritytech/polkadot-sdk that referenced this pull request Dec 4, 2024
## [0.8.3] - 2024-12-03

This release includes two fixes for small memory leaks on edge-cases in
the notification and request-response protocols.

### Fixed

- req-resp: Fix memory leak of pending substreams
([#297](paritytech/litep2p#297))
- notification: Fix memory leak of pending substreams
([#296](paritytech/litep2p#296))

cc @paritytech/networking

---------

Signed-off-by: Alexandru Vasile <[email protected]>
Krayt78 pushed a commit to Krayt78/polkadot-sdk that referenced this pull request Dec 18, 2024
## [0.8.3] - 2024-12-03

This release includes two fixes for small memory leaks on edge-cases in
the notification and request-response protocols.

### Fixed

- req-resp: Fix memory leak of pending substreams
([paritytech#297](paritytech/litep2p#297))
- notification: Fix memory leak of pending substreams
([paritytech#296](paritytech/litep2p#296))

cc @paritytech/networking

---------

Signed-off-by: Alexandru Vasile <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants