Update agent.py to prevent ResourceWarning #459

Closed
wants to merge 1 commit into
from

Projects

None yet

2 participants

@tkrapp
tkrapp commented Dec 16, 2014

When using Python 3 with warnings "enabled" you get a ResourceWarning each time the '_close'-method of AgentSSH is called.
Closing the socket stored in '_conn' explicitly before setting the attribute to None prevents this error.

@tkrapp tkrapp Update agent.py to prevent ResourceWarning
When using Python 3 with warnings "enabled" you get a ResourceWarning each time the '_close'-method of AgentSSH is called.
Closing the socket stored in '_conn' explicitly before setting the attribute to None prevents this error.
cee169e
@bitprophet
Member

Wish the git history was clearer, looks like that commented-out line was added with the original agent support itself - so it's unclear why the original author apparently had the idea to do this, then decided not to at the end.

My guess, looking at the module, is it was a (crappy) defense against closing prior to actual connectivity (i.e. creating the agent, then calling close without connect, would explode as None has no close method). Though as-used, this never happens as connect is called on __init__ in the main public Agent class.

Feels like we could amend your PR to say if self._conn is not None: and that should cover things. I'll test this out now.

@bitprophet bitprophet added this to the 1.15.2 et al milestone Dec 17, 2014
@bitprophet
Member

Also noticed we sometimes explicitly call ._conn.close from the related classes (boo!) but it looks like multiple calls to .close don't cause a stink, so meh.

@bitprophet bitprophet added a commit that referenced this pull request Dec 17, 2014
@bitprophet bitprophet Changelog re #459 8f2d8c0
@tkrapp tkrapp deleted the tkrapp:patch-1 branch Dec 18, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment