apr-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jwool...@apache.org
Subject cvs commit: apr-util/buckets apr_buckets_mmap.c
Date Sat, 23 Nov 2002 21:17:27 GMT
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;
   }
  
  
  

Mime
View raw message