Fix possible fd leak with threaded runtime #28
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR introduces a fix where creating many runtimes in a loop will cause an fd leak. This is because storing a strong arc within each `on_thread_start` fn will cause the runtime handle to not be deallocated when the `compat::Runtime` is dropped. To avoid this we only store a weak pointer within the `on_thread_start` fn and attempt to upgrade it to get the handle. Since, `on_thread_start` is generally always called while the tokio runtime is not dropped the upgrade should not fail. This then allows us to drop the runtime and deallocate the handle by storing the only long lived strong arc within `compat::Runtime`. I've verified this fixes our repro and doesn't leak fds on our benchmarks where we orignally discovered this issue. Closes #27 Signed-off-by: Lucio Franco <luciofranco14@gmail.com>
7 tasks
hawkw
approved these changes
Feb 28, 2020
Co-Authored-By: Eliza Weisman <eliza@buoyant.io>
Co-Authored-By: Eliza Weisman <eliza@buoyant.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR introduces a fix where creating many runtimes in a loop will
cause an fd leak. This is because storing a strong arc within each
on_thread_startfn will cause the runtime handle to not be deallocatedwhen the
compat::Runtimeis dropped. To avoid this we only store aweak pointer within the
on_thread_startfn and attempt to upgrade itto get the handle. Since,
on_thread_startis generally always calledwhile the tokio runtime is not dropped the upgrade should not fail. This
then allows us to drop the runtime and deallocate the handle by storing
the only long lived strong arc within
compat::Runtime.I've verified this fixes our repro and doesn't leak fds on our
benchmarks where we orignally discovered this issue.
Closes #27
Signed-off-by: Lucio Franco luciofranco14@gmail.com