apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brian Pane <bp...@pacbell.net>
Subject [PATCH 2] Re: [PATCH] mmap_setaside (with apr_mmap_dup)
Date Fri, 16 Nov 2001 04:35:47 GMT
Greg Stein wrote:

>>I gave up trying to do full reference counting semantics for
>>duplicates of apr_mmap_t, because I couldn't find a suitable
>>pool to own the locks that would be required in a threaded
>Not sure what you mean here. I don't quite understand how threading comes
>into play. apr_file_t and apr_mmap_t can't be used by multiple threads,
>AFAIK. And I don't think they should either...

When I tried to implement a traditional reference-counting solution
(all apr_mmap_t's duplicated from the same original share a reference
count, and the mmap'ed segment is destroyed only when the refcount
reaches zero), I ran into threading problems with mod_file_cache.
It's possible for two threads to be doing a dup of the same
cached apr_mmap_t at the same time--if they're both delivering the
same statically cached file, and they both end up having to call
mmap_setaside.  To avoid the race condition here, we'd need a lock
around the reference count. The lock API requires that we create the
lock from a pool, but there isn't a really suitable pool from which
to allocate the lock, given that its lifetime isn't necessarily tied
to that of a request or connection.

Fortunately, this whole problem went away when I switched from the
reference-counted model to the transfer-of-ownership model.  The latter
doesn't require any locking.

>>--- srclib/apr/mmap/win32/mmap.c	2001/06/27 22:07:24	1.6
>>+++ srclib/apr/mmap/win32/mmap.c	2001/11/15 05:45:58
>>@@ -62,10 +62,15 @@
>>-static apr_status_t mmap_cleanup(void *themmap)
>>+APR_DECLARE(apr_status_t) apr_mmap_cleanup(void *themmap)
>> {
>Why is this APR_DECLARE'd now? And why the function rename?
This is an error left over from an earlier design approach that I tried
and abandoned.  Thanks for catching that.  I've attached a new version
of the patch that fixes it.


View raw message