apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Cliff Woolley <cliffwool...@yahoo.com>
Subject Re: apr_buckets_file.c:file_read + XTHREAD
Date Wed, 28 Nov 2001 01:14:14 GMT
On Tue, 27 Nov 2001, Doug MacEachern wrote:

> main issue for me is, if the file cannot be mmapped, i want to avoid
> apr_bucket_read()/file_read() and just call apr_file_read() directly for
> better performance.  i imagine other modules might want to do this as
> well.  wouldn't want to see copy-n-paste of the XTHREAD logic in these
> cases.

The point is that you can't thread-safely read an XTHREAD file because of
the file pointer.  Unless you serialize access to the file, that is.  If
you have an XTHREAD file, multiple threads are mucking about with the file
pointer, which is jumping all over the place.  You have no choice but to
seek.  But there's a race condition between the time you seek and the time
you call file_read.  We don't want to serialize obviously, so the only
choice is to reopen the file so that the new apr_file_t and corresponding
os file handle is private to the thread.

> as for mod_file_cache taking care of this, it too would only reopen
> the file on every request if we have to, no?

No.  How would it know?  The handler just sticks a file in a file bucket
and passes it down the filter chain.  It's what happens in the filter
chain that determines whether we have to reopen it or not.  If we never
try to read from the file, we can skip the reopen and use the cached
XTHREAD file descriptor with sendfile().  If we do try to read from it, we
need a thread-private descriptor.  The handler has no information about
whether that will happen or not, and the handler doesn't even have control
of the execution at that point... it's all up the filters/buckets code.

> after all, it is mod_file_cache that sets the XTHREAD flag to begin
> with.

Because mod_file_cache has a single file descriptor that it wishes to
share across threads.  It's up to APR/APR-Util to deal with that reality
from then on.

> just doesn't seem like bucket_read() is the right place for this
> logic.

It is.  I promise.  :)


   Cliff Woolley
   Charlottesville, VA

View raw message