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

Add back-reference from Transport to the SSHClient that created it #891

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@omeranson
Copy link
Contributor

omeranson commented Feb 9, 2017

In some cases, the SSH client is created, the command is executed, the
streams are extracted, and the explicit reference to SSHClient is then
discarded (since it was e.g. created in a function that only returns the
streams). In this case, the SHSClient may be garbage collected, and the
connection's state is undefined.

This fix adds a reference from Transport to the SSHClient that created
it. The streams have a reference to the Channel, which references the
Transport. Now that the Transport references the SSHClient, it won't be
garbage collected until it is closed.

Closes-Bug: #44
Related-Bug: #344

Add back-reference from Transport to the SSHClient that created it
In some cases, the SSH client is created, the command is executed, the
streams are extracted, and the explicit reference to SSHClient is then
discarded (since it was e.g. created in a function that only returns the
streams). In this case, the SHSClient may be garbage collected, and the
connection's state is undefined.

This fix adds a reference from Transport to the SSHClient that created
it. The streams have a reference to the Channel, which references the
Transport. Now that the Transport references the SSHClient, it won't be
garbage collected until it is closed.

Closes-Bug: #44
Related-Bug: #344
@coveralls

This comment has been minimized.

Copy link

coveralls commented Feb 9, 2017

Coverage Status

Coverage increased (+0.1%) to 74.281% when pulling a3b9c6c on omeranson:sshclient_reference into d35b67a on paramiko:master.

@omeranson

This comment has been minimized.

Copy link
Contributor Author

omeranson commented Feb 16, 2017

Hi, @bitprophet , I'd be happy if you could take a look.

Thanks.

@bitprophet

This comment has been minimized.

Copy link
Member

bitprophet commented Feb 19, 2017

Can't help but think this is a code smell implying that Client and Transport need some shaking up, but I'm not about to demand that for such an old codebase 😆 thanks for the patch!

@bitprophet bitprophet added this to the 1.17.4 etc milestone Feb 19, 2017

bitprophet added a commit that referenced this pull request Feb 20, 2017

@bitprophet

This comment has been minimized.

Copy link
Member

bitprophet commented Feb 20, 2017

Cherry-picked back to 1.17.x on up. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.