-
Notifications
You must be signed in to change notification settings - Fork 11
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
Use poll implementation to fix file descriptor leak #72
base: master
Are you sure you want to change the base?
Conversation
Using select() here causes a file descriptor leak. We've replaced this with a poll() implementation to avoid this. The polling object needs to be opened with an eventmask that stops it from blocking paramiko. Fixes ploxiln#51 Co-authored-by: Dan Porter <[email protected]>
Mapping has been added to define the sender and receiver for each event. Additionally we continue with other events to combat the case where a sender has sent data and also hung up unexpectedly.
ebd9d53
to
ccbabd3
Compare
Correct me if I'm wrong, but I think file descriptors (pipes) are leaking due to the issue in this PR: And the issue is manifesting in issues with With that said, here's the docs for the
|
@jealouscloud there may be multiple sources of file descriptor leaks - in this instance the 1024 limit is definitely hit under The above code for |
One of the ways I troubleshot this issue was taking a Before noticing the leak cause of the leak for #74 I recall noticing there was a big difference in fd allocation when I used --eagerly-disconnect. That may or may not have helped your case but if it did, that might have been part of intended functionality instead of an unintended leak of unused resources. That said, fd count absolutely scales with your -z value too, depending on what that might have been. I'm not saying there aren't potentially multiple leaks but I do want to add what i've seen to the conversation. All that said, I still think this is a good/necessary change, leak or no. |
Using
select()
here causes a file descriptor leak. We've replaced this with apoll()
implementation to avoid this.The polling object needs to be opened with an eventmask that stops it from blocking paramiko.
Fixes #51.