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

Try to fix channel close issue #118

Merged
merged 1 commit into from
Feb 1, 2023
Merged

Try to fix channel close issue #118

merged 1 commit into from
Feb 1, 2023

Conversation

yorkz1994
Copy link

This related to my comment on #109 (comment).

I managed to find the real problem.
In russh/src/client/encrypted.rs:
image
When received channel close message, it removes the channel first from channel map, then send back channel_close message to server by Encrypted::byte function.
But in Encrypted::byte function, it checks the channel map to actually send the message, since it has been removed, the close message never got send to the server, this keeps the server channel leak.

image

My solution is to move remove channel in map code into Encrypted::close function.

  1. If client initiate close, Encrypted::close will be invoked, close message will be sent to server and removed in local channel map, if later received server close message, as channel has been removed, no close message will be send again.

  2. If server initiate close, Encrypted::close will be invoked too, close message will be sent to server and removed in local channel map.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@Eugeny Eugeny merged commit f35fbff into Eugeny:master Feb 1, 2023
@Eugeny
Copy link
Owner

Eugeny commented Feb 1, 2023

Wow - nice find, thank you for the contribution! @all-contributors please add @yorkz1994 for code

@allcontributors
Copy link
Contributor

@Eugeny

I couldn't determine any contributions to add, did you specify any contributions?
Please make sure to use valid contribution names.

I've put up a pull request to add @yorkz1994! 🎉

@yorkz1994
Copy link
Author

@Eugeny,

Do you plan for a new release to crates.io for this fix?

@Eugeny
Copy link
Owner

Eugeny commented Feb 7, 2023

Just released v0.36.1 ✌️

@yorkz1994
Copy link
Author

Excellent!
Thanks!

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.

None yet

2 participants