You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by jw...@apache.org on 2002/11/23 22:17:27 UTC
cvs commit: apr-util/buckets apr_buckets_mmap.c
jwoolley 2002/11/23 13:17:27
Modified: . CHANGES
include apr_mmap.h
mmap/unix mmap.c
mmap/win32 mmap.c
buckets apr_buckets_mmap.c
Log:
Fixed the apr_mmap_dup ownership problem for disjoint pools by getting
rid of the is_owner thing completely. Instead, we place all of the dup'ed
apr_mmap_t's in a ring with each other (essentially the same as refcounting
the mmaped region but without the where-do-you-store-it pitfall).
Revision Changes Path
1.361 +6 -0 apr/CHANGES
Index: CHANGES
===================================================================
RCS file: /home/cvs/apr/CHANGES,v
retrieving revision 1.360
retrieving revision 1.361
diff -u -d -u -r1.360 -r1.361
--- CHANGES 23 Nov 2002 04:32:58 -0000 1.360
+++ CHANGES 23 Nov 2002 21:17:27 -0000 1.361
@@ -1,5 +1,11 @@
Changes with APR 0.9.2
+ *) Changed apr_mmap_dup() and friends so that there's no longer any
+ is_owner concept on the mmaped region, but rather something more
+ along the lines of a reference count. This allows the old apr_mmap_t
+ to still be used safely when the new apr_mmap_t is in a disjoint pool.
+ [Cliff Woolley, Sander Striker]
+
*) Fix a bug in apr_hash_merge() which caused the last entry in the
overlay hash to be lost. PR 10522 [Jeff Trawick]
1.34 +12 -7 apr/include/apr_mmap.h
Index: apr_mmap.h
===================================================================
RCS file: /home/cvs/apr/include/apr_mmap.h,v
retrieving revision 1.33
retrieving revision 1.34
diff -u -d -u -r1.33 -r1.34
--- apr_mmap.h 10 Nov 2002 08:35:16 -0000 1.33
+++ apr_mmap.h 23 Nov 2002 21:17:27 -0000 1.34
@@ -58,6 +58,7 @@
#include "apr.h"
#include "apr_pools.h"
#include "apr_errno.h"
+#include "apr_ring.h"
#include "apr_file_io.h" /* for apr_file_t */
#ifdef BEOS
@@ -115,8 +116,12 @@
void *mm;
/** The amount of data in the mmap */
apr_size_t size;
- /** Whether this object is reponsible for doing the munmap */
- int is_owner;
+ /** @deprecated this field is no longer used and will be removed
+ * in APR 1.0 */
+ int unused;
+ /** ring of apr_mmap_t's that reference the same
+ * mmap'ed region; acts in place of a reference count */
+ APR_RING_ENTRY(apr_mmap_t) link;
};
#if APR_HAS_MMAP || defined(DOXYGEN)
@@ -174,8 +179,8 @@
* @param new_mmap The structure to duplicate into.
* @param old_mmap The mmap to duplicate.
* @param p The pool to use for new_mmap.
- * @param transfer_ownership Whether responsibility for destroying
- * the memory-mapped segment is transferred from old_mmap to new_mmap
+ * @param transfer_ownership DEPRECATED: this param is now ignored
+ * and should be removed prior to APR 1.0
*/
APR_DECLARE(apr_status_t) apr_mmap_dup(apr_mmap_t **new_mmap,
apr_mmap_t *old_mmap,
@@ -185,10 +190,11 @@
#if defined(DOXYGEN)
/**
* Transfer the specified MMAP to a different pool
- * @param new_mmap The structure to duplicate into.
+ * @param new_mmap The structure to duplicate into.
* @param old_mmap The file to transfer.
* @param p The pool to use for new_mmap.
- * @remark After this call, old_mmap cannot be used
+ * @deprecated Just use apr_mmap_dup(). The transfer_ownership flag will
+ * go away soon anyway.
*/
APR_DECLARE(apr_status_t) apr_mmap_setaside(apr_mmap_t **new_mmap,
apr_mmap_t *old_mmap,
@@ -196,7 +202,6 @@
#else
#define apr_mmap_setaside(new_mmap, old_mmap, p) apr_mmap_dup(new_mmap, old_mmap, p, 1)
#endif /* DOXYGEN */
-
/**
* Remove a mmap'ed.
1.44 +15 -22 apr/mmap/unix/mmap.c
Index: mmap.c
===================================================================
RCS file: /home/cvs/apr/mmap/unix/mmap.c,v
retrieving revision 1.43
retrieving revision 1.44
diff -u -d -u -r1.43 -r1.44
--- mmap.c 16 Apr 2002 20:25:57 -0000 1.43
+++ mmap.c 23 Nov 2002 21:17:27 -0000 1.44
@@ -83,13 +83,17 @@
static apr_status_t mmap_cleanup(void *themmap)
{
apr_mmap_t *mm = themmap;
- int rv;
+ apr_mmap_t *next = APR_RING_NEXT(mm,link);
+ int rv = 0;
- if (!mm->is_owner) {
- return APR_EINVAL;
- }
- else if (mm->mm == (void *)-1) {
- return APR_ENOENT;
+ /* we no longer refer to the mmaped region */
+ APR_RING_REMOVE(mm,link);
+ APR_RING_NEXT(mm,link) = NULL;
+ APR_RING_PREV(mm,link) = NULL;
+
+ if (next != mm) {
+ /* more references exist, so we're done */
+ return APR_SUCCESS;
}
#ifdef BEOS
@@ -163,7 +167,7 @@
(*new)->mm = mm;
(*new)->size = size;
(*new)->cntxt = cont;
- (*new)->is_owner = 1;
+ APR_RING_ELEM_INIT(*new, link);
/* register the cleanup... */
apr_pool_cleanup_register((*new)->cntxt, (void*)(*new), mmap_cleanup,
@@ -179,21 +183,10 @@
*new_mmap = (apr_mmap_t *)apr_pmemdup(p, old_mmap, sizeof(apr_mmap_t));
(*new_mmap)->cntxt = p;
- /* The old_mmap can transfer ownership only if the old_mmap itself
- * is an owner of the mmap'ed segment.
- */
- if (old_mmap->is_owner) {
- if (transfer_ownership) {
- (*new_mmap)->is_owner = 1;
- old_mmap->is_owner = 0;
- apr_pool_cleanup_kill(old_mmap->cntxt, old_mmap, mmap_cleanup);
- apr_pool_cleanup_register(p, *new_mmap, mmap_cleanup,
- apr_pool_cleanup_null);
- }
- else {
- (*new_mmap)->is_owner = 0;
- }
- }
+ APR_RING_INSERT_AFTER(old_mmap, *new_mmap, link);
+
+ apr_pool_cleanup_register(p, *new_mmap, mmap_cleanup,
+ apr_pool_cleanup_null);
return APR_SUCCESS;
}
1.15 +14 -21 apr/mmap/win32/mmap.c
Index: mmap.c
===================================================================
RCS file: /home/cvs/apr/mmap/win32/mmap.c,v
retrieving revision 1.14
retrieving revision 1.15
diff -u -d -u -r1.14 -r1.15
--- mmap.c 18 Apr 2002 20:26:40 -0000 1.14
+++ mmap.c 23 Nov 2002 21:17:27 -0000 1.15
@@ -66,13 +66,17 @@
static apr_status_t mmap_cleanup(void *themmap)
{
apr_mmap_t *mm = themmap;
+ apr_mmap_t *next = APR_RING_NEXT(mm,link);
apr_status_t rv = 0;
- if (!mm->is_owner) {
- return APR_EINVAL;
- }
- else if (!mm->mhandle) {
- return APR_ENOENT;
+ /* we no longer refer to the mmaped region */
+ APR_RING_REMOVE(mm,link);
+ APR_RING_NEXT(mm,link) = NULL;
+ APR_RING_PREV(mm,link) = NULL;
+
+ if (next != mm) {
+ /* more references exist, so we're done */
+ return APR_SUCCESS;
}
if (mm->mv) {
@@ -163,7 +167,7 @@
(*new)->mm = (char*)((*new)->mv) + (*new)->poffset;
(*new)->size = size;
(*new)->cntxt = cont;
- (*new)->is_owner = 1;
+ APR_RING_ELEM_INIT(*new, link);
/* register the cleanup... */
apr_pool_cleanup_register((*new)->cntxt, (void*)(*new), mmap_cleanup,
@@ -179,21 +183,10 @@
*new_mmap = (apr_mmap_t *)apr_pmemdup(p, old_mmap, sizeof(apr_mmap_t));
(*new_mmap)->cntxt = p;
- /* The old_mmap can transfer ownership only if the old_mmap itself
- * is an owner of the mmap'ed segment.
- */
- if (old_mmap->is_owner) {
- if (transfer_ownership) {
- (*new_mmap)->is_owner = 1;
- old_mmap->is_owner = 0;
- apr_pool_cleanup_kill(old_mmap->cntxt, old_mmap, mmap_cleanup);
- apr_pool_cleanup_register(p, *new_mmap, mmap_cleanup,
- apr_pool_cleanup_null);
- }
- else {
- (*new_mmap)->is_owner = 0;
- }
- }
+ APR_RING_INSERT_AFTER(old_mmap, *new_mmap, link);
+
+ apr_pool_cleanup_register(p, *new_mmap, mmap_cleanup,
+ apr_pool_cleanup_null);
return APR_SUCCESS;
}
1.53 +23 -21 apr-util/buckets/apr_buckets_mmap.c
Index: apr_buckets_mmap.c
===================================================================
RCS file: /home/cvs/apr-util/buckets/apr_buckets_mmap.c,v
retrieving revision 1.52
retrieving revision 1.53
diff -u -d -u -r1.52 -r1.53
--- apr_buckets_mmap.c 2 Jun 2002 19:54:49 -0000 1.52
+++ apr_buckets_mmap.c 23 Nov 2002 21:17:27 -0000 1.53
@@ -62,9 +62,9 @@
apr_bucket_mmap *m = b->data;
apr_status_t ok;
void *addr;
-
+
if (!m->mmap) {
- /* due to cleanup issues we have no choice but to check this */
+ /* the apr_mmap_t was already cleaned up out from under us */
return APR_EINVAL;
}
@@ -79,10 +79,11 @@
static apr_status_t mmap_bucket_cleanup(void *data)
{
- /* the mmap has disappeared out from under us. we have no choice
- * but to invalidate this bucket. from now on, all you can do to this
- * bucket is destroy it, not read from it.
- */
+ /* the apr_mmap_t is about to disappear out from under us, so we
+ * have no choice but to pretend it doesn't exist anymore. the
+ * refcount is now useless because there's nothing to refer to
+ * anymore. so the only valid action on any remaining referrer
+ * is to delete it. no more reads, no more anything. */
apr_bucket_mmap *m = data;
m->mmap = NULL;
@@ -94,7 +95,7 @@
apr_bucket_mmap *m = data;
if (apr_bucket_shared_destroy(m)) {
- if (m->mmap && m->mmap->is_owner) {
+ if (m->mmap) {
apr_pool_cleanup_kill(m->mmap->cntxt, m, mmap_bucket_cleanup);
apr_mmap_delete(m->mmap);
}
@@ -114,10 +115,8 @@
m = apr_bucket_alloc(sizeof(*m), b->list);
m->mmap = mm;
- if (mm->is_owner) {
- apr_pool_cleanup_register(mm->cntxt, m, mmap_bucket_cleanup,
- apr_pool_cleanup_null);
- }
+ apr_pool_cleanup_register(mm->cntxt, m, mmap_bucket_cleanup,
+ apr_pool_cleanup_null);
b = apr_bucket_shared_make(b, m, start, length);
b->type = &apr_bucket_type_mmap;
@@ -139,33 +138,36 @@
return apr_bucket_mmap_make(b, mm, start, length);
}
-static apr_status_t mmap_bucket_setaside(apr_bucket *data, apr_pool_t *p)
+static apr_status_t mmap_bucket_setaside(apr_bucket *b, apr_pool_t *p)
{
- apr_bucket_mmap *m = data->data;
+ apr_bucket_mmap *m = b->data;
apr_mmap_t *mm = m->mmap;
apr_mmap_t *new_mm;
apr_status_t ok;
if (!mm) {
- /* due to cleanup issues we have no choice but to check this */
+ /* the apr_mmap_t was already cleaned up out from under us */
return APR_EINVAL;
}
+ /* shortcut if possible */
if (apr_pool_is_ancestor(mm->cntxt, p)) {
return APR_SUCCESS;
}
+ /* duplicate apr_mmap_t into new pool */
+ /* XXX: the transfer_ownership flag on this call
+ * will go away soon.. it's ignored right now. */
ok = apr_mmap_dup(&new_mm, mm, p, 1);
if (ok != APR_SUCCESS) {
return ok;
}
-
- m->mmap = new_mm;
- if (new_mm->is_owner) {
- apr_pool_cleanup_kill(mm->cntxt, m, mmap_bucket_cleanup);
- apr_pool_cleanup_register(new_mm->cntxt, m, mmap_bucket_cleanup,
- apr_pool_cleanup_null);
- }
+
+ /* decrement refcount on old apr_bucket_mmap */
+ mmap_bucket_destroy(mm);
+
+ /* create new apr_bucket_mmap pointing to new apr_mmap_t */
+ apr_bucket_mmap_make(b, new_mm, b->start, b->length);
return APR_SUCCESS;
}