Before we invent another stream API (which isn't a small patch at all,
and has a bunch of design decisions involved AFAICT), can we please
document the semantics of svn_stream_close()? How it interacts with
clearing the stream's pool, whether it's permitted not to call it at
all, and whether it's permitted to be an expensive operation or is it
supposed to be an O(1) operation.
It's pretty annoying to see a patch rejected because of stream semantics
that are documented not in svn_io.h but in Bert's head.
Julian Foad wrote on Mon, May 09, 2011 at 21:51:26 +0100:
> 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.
>
> I don't think that works as a part of the generic stream model. The
> "close" method on any stream is required to call the "close" method of
> its underlying stream, but that's not what we want in the case of an
> unexpected close (which we could call an "abort"). If we have a
> checksumming stream (which reads to the end on close and so doesn't want
> its close method to be called on abort), and this stream is wrapped by
> some kind of filtering stream (which allocates some resources and
> chooses to have its "close" method called on abort), then if we abort we
> would call filtering.close() which necessarily calls
> checksumming.close() even though we didn't want that.
>
> Instead I think we need the ability for a stream to provide an "abort"
> method, which will be called on pool clean-up and which is not required
> to call the underlying stream's close() but is merely required to free
> up any resources it may have allocated itself.
>
> Does that make sense?
>
> - Julian
>
>
> > This is particularly needed for the Python bindings. The PyObject
> > wrapped by the svn_stream_t created by svn_swig_py_make_stream() might
> > go scope in Python, but the stream maintains a reference to it so the
> > PyObject doesn't get destroyed/deleted.
>
>
> > +/** Set @a stream to be automatically closed when pool containing it is
> > cleared
> > + *
> > + * @since New in 1.7
> > + */
> > +void
> > +svn_stream_close_on_cleanup(svn_stream_t *stream);
>
>
Received on 2011-05-10 16:59:44 CEST