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: clean up pending dial targets #1059

Merged
merged 1 commit into from
Dec 10, 2021
Merged

Conversation

achingbrain
Copy link
Member

If the Promise.race throws, execution of the function is terminated so the pending dial target is never removed from the map and we leak memory.

This can happen when there are invalid multiaddrs or when a peer reports more dialable addresses than the threshold.

Instead wrap the Promise.race in a try/finally which will always remove the pending dial target in the event of success or failure.

If the `Promise.race` throws, execution of the function is terminated
so the pending dial target is never removed from the map and we leak
memory.

This can happen when there are invalid multiaddrs or when a peer reports
more dialable addresses than the threshold.

Instead wrap the `Promise.race` in a `try/finally` which will always
remove the pending dial target in the event of success or failure.
@achingbrain
Copy link
Member Author

achingbrain commented Dec 10, 2021

A graph, running IPFS using libp2p master vs this PR. Leave it running for an hour, just idling with the DHT enabled and it running it's query-self query every five minutes.

Blue and red lines are heap use in MB. Blue is master, red is this PR. Yellow is the number of network peers we have - low water mark 50, high 100.

Samples collected every 10 seconds, using --expose-gc and running global.gc() before every process.memoryUsage() invocation.

image

@achingbrain
Copy link
Member Author

Refs ipfs/js-ipfs#3469

@vasco-santos
Copy link
Member

wow, good job @achingbrain

@achingbrain achingbrain merged commit bdc9f16 into master Dec 10, 2021
@achingbrain achingbrain deleted the fix/clean-up-dial-targets branch December 10, 2021 12:42
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