Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Possible ssh2Stream._channels[chan] leak (v0.8.9) #1012

Closed
Luluno01 opened this issue May 22, 2021 · 7 comments
Closed

Possible ssh2Stream._channels[chan] leak (v0.8.9) #1012

Luluno01 opened this issue May 22, 2021 · 7 comments

Comments

@Luluno01
Copy link

Luluno01 commented May 22, 2021

I am forwarding out TCP connections from remote system to local system. A few hundreds of connections were established and ended (the end event emitted). However, the ._channels also had hundreds of properties with numerical keys and true as their values. It looks something like

Client {
  ...
  _channels: {
    '0': [Function (anonymous)],
    '1': [Function (anonymous)],
    '2': true,
    '3': true,
    '4': true,
    '5': true,
    '6': true,
    '7': true,
    '8': true,
    '9': true,
    '10': true,
    ...
    '478': true
  },
  ...
}

After searching for all accesses to _channels in the code and some inspection, I found it could be a leak. Here are the related code snippets for your easy reference.

Assignment to _channels[*]:

https://github.com/mscdex/ssh2/blob/v0.8.x/lib/client.js#L1508

Deletion of _channels[*]:

https://github.com/mscdex/ssh2/blob/v0.8.x/lib/client.js#L1199

Call chain of deletion:

https://github.com/mscdex/ssh2/blob/v0.8.x/lib/client.js#L338

https://github.com/mscdex/ssh2/blob/v0.8.x/lib/client.js#L1156

It seems these properties will be removed only when the underlying SSH connection is closed:

https://github.com/mscdex/ssh2/blob/v0.8.x/lib/client.js#L313-L338

This will eventually exhaust the memory because in my case the port forwarding is intended to be persistent.

@mscdex
Copy link
Owner

mscdex commented May 22, 2021

The reserved channel numbers are deleted when the channels themselves are closed.

Judging by the (large number of) true _channels values you're seeing, it appears either accept() or reject() are not being called for those incoming forward requests.

@mscdex
Copy link
Owner

mscdex commented May 22, 2021

Actually, upon further review, I think I may see what's happening here.....

@mscdex mscdex closed this as completed in aac13eb May 22, 2021
@mscdex
Copy link
Owner

mscdex commented May 22, 2021

Thanks for the report!

@Luluno01
Copy link
Author

Thank you for your attention! So the fix is only available in the latest unreleased version? Is it recommended to update to the latest version and how to migrate to the latest version? If not, when can I safely delete those leaked channels (as a workaround at the moment)?

@mscdex
Copy link
Owner

mscdex commented May 23, 2021

It will be included in the upcoming v1.0.0 release. I will probably do that release in the next couple of days or so.

@mscdex
Copy link
Owner

mscdex commented May 23, 2021

Regarding changes between v0.8.x and what will be v1.0.0, you can find most of them here: #935

@Luluno01
Copy link
Author

Sounds great! Thanks!

xiaobaidadada pushed a commit to xiaobaidadada/ssh2-prebuilt that referenced this issue Feb 25, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants