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
