gc.garbage from paramiko

[Robey Pointer]

I'm cc:ing the paramiko mailing list because I think this is relevant  
there too.


On 6 Feb 2006, at 0:09, Andrew Bennetts wrote:

> On Fri, Feb 03, 2006 at 10:41:11AM -0800, Robey Pointer wrote:
>>
>> On 2 Feb 2006, at 17:19, Andrew Bennetts wrote:
>>
>>> The root cause is that paramiko is using __del__.  This is almost
>>> always a bad
>>> idea, because objects with __del__ cannot be garbage collected if
>>> they're in a
>>> reference cycle.  It's an almost certain way to cause memory leaks,
>>> unless
>>> you're *extremely* careful to always break cycles manually.
>>>
>>> I think paramiko should not use __del__.
>>
>> Unfortunately __del__ is needed for some python-file emulation, which
>> bzr and other apps rely on heavily, often without knowing (which is
>> the point).  But it should be okay as long as no cycles are created.
>> But yes, I agree that use of __del__ is really a black art.
>
> What do you need to do on __del__?  socket.sockets already close  
> themselves just
> like files.

I guess that was kind of vague, sorry. :)  Python borrows a concept  
from c++, and causes files & sockets to auto-close themselves as soon  
as the last reference goes away.  Since paramiko supplies both socket- 
like things (Channels) and file-like things (SFTPFiles), I've found  
that python coders expect them to have the same del behavior as  
builtins, allowing things like this:

     open('email.txt', 'w').write('jdoe at example.com')

That said, I think the __del__ in Transport may be unnecessary.  (I  
don't think a Transport can be collected until all its channels are  
already gone.)  Though I'm still interested in cleaning up any  
remaining cycles that get created, because after reading threads on  
python-dev about this, I don't think the GC is going to be fixed  
anytime soon.


> Cycles are really, really hard to avoid.  If a traceback occurs,  
> that can create
> cycles.  There's lots of ways to unexpectedly make a cycle.

How could a traceback create a cycle?


> There's a pretty obvious cycle right here:
>
>         self.packetizer.set_keepalive(interval,
>             lambda x=self: x.global_request('keepalive at lag.net',  
> wait=False))

Oops, yeah.  Added to my to-do list. :)  Although I don't think  
that's the one you're seeing, because this code isn't called by bzr.

robey