> -----Original Message-----
> From: Julian Foad [mailto:julian.f...@wandisco.com]
> Sent: maandag 9 mei 2011 22:51
> To: Nick Piper
> Cc: dev@subversion.apache.org
> Subject: Re: AW: Python bindings: Use of svn_swig_py_make_stream() -
> avoiding leaking fds
>
> Nick Piper wrote:
> > Add svn_stream_close_on_cleanup(), which will register a stream to be
> > automatically closed when the pool is destroyed.
>
> First impression: the public name makes it sound like a user-level
> thing, but this is for use by a stream's implementation, right? Not by
> its users.
Well in some cases it is a user level thing.
Swig python wants to receive a close event to release resources, but our
streams don't close in this specific case.
(Because in all the other cases we do close it or we don't need a close).
Earlier I suggesting such a function, to allow the python bindings to request
this close on pool cleanup.
(And we had this discussion earlier... when we were discussing if we should use
the close event to install a new pristine file)
We kept the definition of streams very generic in the past (read: before 1.0),
to allow reusing the concept everywhere.
And now with 1.7 we start adding new requirements.
Personally I don't think we shouldn't add pool cleanups everywhere because some
code might need it.
For .Net Microsoft recommends using finalizers *only* when you have to directly
release unmanaged resources like file handles. So you use them in the low
layers where you are managing resources, not high up the chain where you are
just wrapping lower layer.
I think we should apply the same rule here: Use pool cleanups when necessary,
not by default.
We are just reinventing reference counting if we are adding pool cleanups
everywhere.
The python bindings need it, so I would say: Add it there.
(And maybe make it easier to use from other bindings when it proves useful.
That is why I suggested adding the function in svn_stream.h)
Bert
