httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Fritsch>
Subject Re: Change FILE/... buckets to close file descriptor on destruction?
Date Tue, 15 Jun 2010 17:23:10 GMT
On Monday 14 June 2010, William A. Rowe Jr. wrote:
> On 6/14/2010 4:20 PM, Stefan Fritsch wrote:
> > 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?
> We could create a new apr_bucket_file_closing flavor, but no.  You
> can't alter the public structures until 2.0.  I really don't
> recall why these internals were exposed, but this does demonstrate
> that might have been a bad idea ;-)

Yes, that's unfortunate.
> As far as changing the structure itself, I wouldn't start creating
> two ints when it's simple to change can_mmap to a bit flag,
> likewise 'should be closed'.  But you are thinking of marking
> every bucket for this file with this flag?
> I'm wondering if this isn't actually a more general problem that
> would be better solved with a broader hook-on-refcount==0 entity?

You think in the form of an apr_bucket_destroy_ex() function? Maybe 
that would be possible without changing the ABI (or maybe not, 
apr_bucket_type_t with all the function pointers is public, too). But 
this it no longer looking like a simple solution to me.

And I just remembered mod_file_cache, which caches open fds. It just 
calls apr_brigade_insert_file which in turn calls 
apr_bucket_file_create. This looks to me like it would be broken by 
APR simply changing the semantics of FILE buckets to close the fd.

Therefore I have changed my mind and now think that a solution inside 
httpd may be best. For example the core_output_filter could setaside 
the buckets not into the conn pool but into a temp pool that is 
cleared after the EOR bucket is seen. Does that sound reasonable?

View raw message