subversion-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Willmer <al.will...@logica.com>
Subject Re: Avoinding file handle leak using the Python bindings & core.Stream
Date Mon, 16 Apr 2012 13:43:03 GMT
Ulrich Eckhardt <ulrich.eckhardt <at> dominolaser.com> writes:
> Am 16.04.2012 11:29, schrieb Willmer, Alex (PTS):
> > To work around this I have monkey patched a
> > Stream.close() method that calls svn_stream_close, which is used
> > in a try/finally block.
> 
> Disclaimer: I'm not really familiar with the SVN/Python bindings.
> However, concerning Python in general, explicitly calling close() is the
> wrong way. Instead you should make sure files are closed by opening them
> in a with clause:
> 
>   with open(...) as f:
>       ... # use f

Unfortunately I'm targeting Python 2.4, but always worth mentioned and I intend 
to include context manager support in any patch I submit.

> That said, when the core.Stream instance is garbage-collected (normally
> when it goes out of scope), it also releases its "self._stream" which
> can subsequently be garbage-collected. If this object in turn requires
> explicit cleanup of its internal resources, it should provide a __del__
> method doing that. If it leaks this a file handle in the (unusual?) case
> that the content wasn't read, that is the place where the bug actually is.

As I'm sure you're aware using __del__() for object clean-up is tricky and 
discouraged. However assume-cleanup-on-destruction seems to be how the bindings 
are used currently (at least by Trac).

> > 1. In the Python bindings core.Stream doesn't have a .close()
> > method [a]. Is there any reason this might be intentional?
> 
> I guess that yes. The point is that the file interface only has read()
> and write(), but not close(). In other words, functions that are
> supposed to work with files and file-like types should only expect those
> two functions, not e.g. a close().

I see that as the loose definition of "file like" biting us. I was just 
surprised to learn that the formalised io.IOBase abstract base class in 2.7/3.x 
has  14 methods/properties http://docs.python.org/library/io.html#io.IOBase.


Mime
View raw message