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.
Received on 2012-04-16 15:45:44 CEST