apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Fritsch ...@sfritsch.de>
Subject Re: Change FILE/... buckets to close file descriptor on destruction?
Date Mon, 14 Jun 2010 21:20:36 GMT
On Tuesday 18 May 2010, Stefan Fritsch wrote:
> 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.

From other mail:
> I found no breakage in subversion 1.6.11's test suite.

To get this item from the httpd 2.4 blockers list, we could vote:

[  ] change FILE/PIPE/SOCKET bucket in APR to close file descriptor
     when the last referencing bucket is destroyed
[  ] same as above, but only if a new flag is set (like flag for mmap)
[  ] don't change APR, work around problem in httpd

But I am not sure what introducing a new element in struct 
apr_bucket_file would mean for ABI compatibility. On the one hand, 
apr_bucket_file is public, on the other hand, there are functions like 
apr_bucket_file_create and apr_bucket_file_enable_mmap. Would adding 
an element at the end be ok for apr-util 1.4, or would that only be 
possible in 2.0?


View raw message