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

Fix resources cleanup in eth/* protocols #7311

Merged
merged 17 commits into from
Aug 24, 2024

Conversation

alexb5dh
Copy link
Contributor

@alexb5dh alexb5dh commented Aug 6, 2024

Changes

Fixes some of the missing ArrayPoolList cleanups in client code and tests.

Fixed:

  • eth/66 and SNAP protocols not disposing some messages in case of a SubprotocolException.
  • eth/65 not disposing NewPooledTransactionHashesMessage.
  • PooledTxsRequestor not disposing some of the allocated ArrayPoolLists
  • Tests under Nethermind.Network.Test.P2P.Subprotocols.Eth failing randomly due to some ArrayPoolLists not being disposed. Should be reproducible by either running or debugging tests for multiple protocol versions in a single run.

Also, optimized ArrayPool copying in PooledTxsRequestor.RequestTransactionsEth66, when multiple batches are sent.

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)

Testing

Requires testing

  • No

Documentation

Requires documentation update

  • No

Requires explanation in Release Notes

  • No

@alexb5dh alexb5dh self-assigned this Aug 6, 2024
@alexb5dh alexb5dh changed the title Fix randomly failing protocol tests Fix message disposing in networking tests and eth/66 protocol Aug 6, 2024
@alexb5dh alexb5dh changed the title Fix message disposing in networking tests and eth/66 protocol Fix resources cleanup in eth/* protocols Aug 8, 2024
@alexb5dh alexb5dh force-pushed the fix/randomly-failing-protocol-tests branch from 55cd6b6 to 5742b09 Compare August 8, 2024 21:24
@alexb5dh alexb5dh marked this pull request as ready for review August 8, 2024 21:38
@LukaszRozmej
Copy link
Member

@alexb5dh what about #7186 ?

…g-protocol-tests

# Conflicts:
#	src/Nethermind/Nethermind.Network.Test/P2P/Subprotocols/Eth/V66/Eth66ProtocolHandlerTests.cs
@alexb5dh alexb5dh force-pushed the fix/randomly-failing-protocol-tests branch from 72f5058 to 455253f Compare August 12, 2024 17:28
@LukaszRozmej LukaszRozmej merged commit 5b91189 into master Aug 24, 2024
66 checks passed
@LukaszRozmej LukaszRozmej deleted the fix/randomly-failing-protocol-tests branch August 24, 2024 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants