[PYTHON-CRYPTO] Yet another attempt to plug M2Crypto.SSL.Connection.Connection

[Guido van Rossum]

On Fri, 25 Feb 2005 16:47:45 +1300, Michael Dunstan <michael at elyt.com> wrote:
> Reading "M2Crypto woes"
> http://www.artima.com/weblogs/viewpost.jsp?thread=95863 reminded me
> that I should go back and see what happened with that patch that in
> some fluke just happened to work for me.
>
> Seems that this still exists even on the 0.15 branch. Okay - time to
> take a closer look then. Attached is a trivial example demonstrating
> the memory leak. When you run this it prints something like:
>
> [<__main__.Leaky instance at 0x6de50>]
>
> Meaning that we have an instance of the Leaky class that is now marked
> as garbage and not collected. But the other two instances of non leaky
> classes are not present. They have been garbage collected. A class with
> the combination of assignment of a bound method and use of __del__
> creates a cycle that can not be garbage collected. Well, that's how I'm
> interpreting the behaviour I'm seeing. (With python 2.3.4.)
>
> So what to do about this in the case of
> M2Crypto.SSL.Connection.Connection? (Without resorting to wrapping
> read/write with an if statement.) One way would be to provide a
> different class for the different behaviour rather than use
> setblocking(). For example:
>
> class Connection:
>      """A blocking SSL connection."""
>      sendall = send = write = _write_bio
>      recv = read = _read_bio
>
> class NonBlockingConnection(Connection):
>      """A non blocking SSL connection."""
>      send = write = _write_nbio
>      recv = read = _read_nbio
>
> (I don't pretend to understand why sendall is not affected by the
> different modes. Should it even exist for NonBlockingConnection?)
>
> Not sure how that fits into the existing framework though.

The best solution is usually not to use __del__. It is rarely
necessary. (About the only times where it's okay to have a __del__ is
when it's necessary is to delete an *external* resource such as an
integer file descriptor or a temporary file under all circumstances.)

In the case of a class whose instances are involved in cycles, __del__
will not be called by the collector, for very subtle reasons having to
do with not wanting to guess in which order the elements of the cycle
should be destroyed.

Assigning a bound method to an instance variable creates a cycle
because the bound method has a reference to the instance (its im_self
attribute).

Alas, it looks like the __del__ methods on Conection, Context and
Session are used to free the low-level BIO objects. So then the
solution may have to be to avoid cycles.

One tactic when dealing with something involved in a cycle that needs
to have a __del__ is to create a separate object that exists *just* to
have the __del__ method and is not involved in cycles. The collector
is smart enough to call the __del__ methods of objects that are not
involved in cycles but are kept alive by cycles. Example: A <-> B ->
C; when the cycle A <-> B is collected, C.__del__ is called, but not
A.__del__ or B.__del__.

BTW another leak in M2Crypto is the SSL Context created by
HTTPSConnection when none is passed in; SSL Context objects are only
deleted when their close() method is called, and nothing will ever
call close() on the ssl_ctx object contained in the HTTPSConnection
object.

There were some more leaks reported in the SWIG wrappers in the
followups to the above Blog url.

--
--Guido van Rossum (home page: http://www.python.org/~guido/)