[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: AW: Python bindings: Use of svn_swig_py_make_stream() - avoiding leaking fds

From: Philip Martin <philip.martin_at_wandisco.com>
Date: Mon, 09 May 2011 12:59:54 +0100

Daniel Shahaf <danielsh_at_elego.de> writes:

> Philip Martin wrote on Mon, May 09, 2011 at 12:33:14 +0100:
>> Daniel Shahaf <danielsh_at_elego.de> writes:
>>
>> > +static apr_status_t
>> > +close_stream_cleanup(void *stream)
>> > +{
>> > + apr_status_t apr_err = APR_SUCCESS;
>> > + svn_error_t *err;
>> > +
>> > + err = svn_stream_close(stream);
>> > + if (err)
>> > + {
>> > + apr_err = err->apr_err;
>> > + svn_error_clear(err);
>> > + }
>> > +
>> > + return apr_err;
>> > +}
>> > +
>> > svn_stream_t *
>> > svn_stream_create(void *baton, apr_pool_t *pool)
>> > {
>> > @@ -73,6 +89,9 @@ svn_stream_create(void *baton, apr_pool_t *pool)
>> > stream->mark_fn = NULL;
>> > stream->seek_fn = NULL;
>> > stream->buffered_fn = NULL;
>> > + apr_pool_cleanup_register(pool, stream,
>> > + close_stream_cleanup,
>> > + apr_pool_cleanup_null);
>> > return stream;
>> > }
>>
>> What happens to streams that are explicitly closed? When the pool
>> cleanup handler runs it will close the stream a second time.
>>
>
> Undefined behaviour?
>
> I don't think we need svn_atomic__init_once() here though, since we can
> assume that people don't call svn_stream_close() whilst clearing the
> pool that stream is allocated from. In other words, a "has_been_closed"
> boolean should suffice?

Yes, that would work. An alternative would be to deregister the pool
cleanup on an explicit close.

-- 
Philip
Received on 2011-05-09 14:00:41 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.