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.
