Fix socket and memory leak on shutdown #12
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
When the client is shutting down, its likely that the async_connection_process_loop is polling the stream(socket) and input channel futures, and never wakes so that the stop/shutdown channel can be checked. As a result the async_connection_process_loop task never exits and while not harmful to subsequent connections, the memory associated with the future is never released, and, since the task owns the stream, the socket itself is leaked as well. The fix provided by this patch is straightforward, simply poll the stop channel future as well. An even better fix than the one provided here would be to save the task handle future returned by spawn, poll it somewhere (in case it fails), and join the main task to it on shutdown, after the stop channel has been written to.
|
Thanks for the contribution. I'd have another release after the clippy fix. |
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.
When the client is shutting down, its likely that the async_connection_process_loop is polling the stream(socket) and input channel futures, and never wakes so that the stop/shutdown channel can be checked.
As a result the async_connection_process_loop task never exits and while not harmful to subsequent connections, the memory associated with the future is never released, and, since the task owns the stream, the socket itself is leaked as well.
The fix provided by this patch is straightforward, simply poll the stop channel future as well.
An even better fix than the one provided here would be to save the task handle future returned by spawn, poll it somewhere (in case it fails), and join the main task to it on shutdown, after the stop channel has been written to.