apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Fritsch ...@sfritsch.de>
Subject Re: File descriptor leak with mpm-event / apr file bucket cleanup
Date Tue, 18 May 2010 20:50:03 GMT
On Tuesday 18 May 2010, Joe Orton wrote:
> On Tue, May 18, 2010 at 09:18:23AM +0200, Stefan Fritsch wrote:
> > On Tue, 18 May 2010, Ruediger Pluem wrote:
> > >So if you want to close this fd you IMHO would need to do some
> > >refcounting and only close it if no other filebucket still
> > >references it.
> > 
> > The filebuckets already do refcounting.
> > apr_bucket_shared_destroy(f) in the patch above is only true if
> > the refcount has reached zero.
> 
> They refcount the number of times the FILE bucket has been
> split/copied, they don't refcount the number of times the
> underlying apr_file_t object is used.
> 
> APR does not seem to be consistent about of whether "ownership" of
> the object is passed on when you create a bucket wrapper for that
> object type; there are no API guarantees or constraints here. 
> From a quick review, PIPE, MMAP, HEAP (kind of) do take ownership,
> FILE and SOCKET do not.
> 
> Have you run the perl-framework test suite to see whether this
> breaks anything?  This change does look like it'll break the APR
> test suite but only because of the way I happened to write one of
> the buckets tests.

It does not cause any breakage in the perl-framework. As you 
suspected, it does break apr-util's testbuckets test. I will look if I 
can run the subversion test suite, too.

If changing the current behaviour is considered too disruptive, one 
could also introduce a flag for the cleanup behaviour of FILE buckets, 
like there is a flag for mmap. IMHO doing the closing in the bucket 
cleanup is far preferable to doing it anywhere else in the core output 
filter.

Cheers,
Stefan

Mime
View raw message