Fix a problem with the is_owner handling in the Unix and MMAP code that

would cause a cleanup to be killed that didn't exist in a certain set of
circumstances (create, dup but don't transfer ownership, dup again, delete
the second dup, boom).  Also fix a potential problem in the Unix code where
the ->mm pointer would not be set to NULL if munmap failed.  Logic like
that caused us headaches a while back in the directory cleanups.

Reviewed by:	Greg Ames, William Rowe


git-svn-id: http://svn.apache.org/repos/asf/apr/apr/trunk@63241 13f79535-47bb-0310-9956-ffa450edef68

2 files changed, 16 insertions, 22 deletions

diff --git a/mmap/unix/mmap.c b/mmap/unix/mmap.c
index 449aa78d3..21996add8 100644
--- a/mmap/unix/mmap.c
+++ b/mmap/unix/mmap.c
@@ -85,25 +85,21 @@ static apr_status_t mmap_cleanup(void *themmap)
     apr_mmap_t *mm = themmap;
     int rv;
 
-    if (!mm->is_owner) {
-        return APR_SUCCESS;
+    if ((!mm->is_owner) || (mm->mm == (void *)-1)) {
+        /* XXX: we shouldn't ever get here */
+        return APR_ENOENT;
     }
 
 #ifdef BEOS
     rv = delete_area(mm->area);
-
-    if (rv == 0) {
-        mm->mm = (void *)-1;
-        return APR_SUCCESS;
-    }
 #else
     rv = munmap(mm->mm, mm->size);
+#endif
+    mm->mm = (void *)-1;
 
     if (rv == 0) {
-        mm->mm = (void *)-1;
         return APR_SUCCESS;
     }
-#endif
     return errno;
 }
 
@@ -189,24 +185,21 @@ APR_DECLARE(apr_status_t) apr_mmap_dup(apr_mmap_t **new_mmap,
             (*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_pool_cleanup_register(p, *new_mmap, mmap_cleanup,
-                                  apr_pool_cleanup_null);
     }
     return APR_SUCCESS;
 }
 
 APR_DECLARE(apr_status_t) apr_mmap_delete(apr_mmap_t *mm)
 {
-    apr_status_t rv;
+    apr_status_t rv = APR_SUCCESS;
 
-    if (mm->mm == (void *)-1)
-        return APR_ENOENT;
-      
-    if ((rv = mmap_cleanup(mm)) == APR_SUCCESS) {
+    if (mm->is_owner && ((rv = mmap_cleanup(mm)) == APR_SUCCESS)) {
         apr_pool_cleanup_kill(mm->cntxt, mm, mmap_cleanup);
         return APR_SUCCESS;
     }
diff --git a/mmap/win32/mmap.c b/mmap/win32/mmap.c
index 4a65cff7e..836c6d92d 100644
--- a/mmap/win32/mmap.c
+++ b/mmap/win32/mmap.c
@@ -68,8 +68,9 @@ static apr_status_t mmap_cleanup(void *themmap)
     apr_mmap_t *mm = themmap;
     apr_status_t rv = 0;
 
-    if (!mm->is_owner) {
-        return APR_SUCCESS;
+    if (!mm->is_owner || !mm->mhandle) {
+        /* XXX: we shouldn't ever get here */
+        return APR_ENOENT;
     }
 
     if (mm->mv) {
@@ -184,21 +185,21 @@ APR_DECLARE(apr_status_t) apr_mmap_dup(apr_mmap_t **new_mmap,
             (*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_pool_cleanup_register(p, *new_mmap, mmap_cleanup,
-                                  apr_pool_cleanup_null);
     }
     return APR_SUCCESS;
 }
 
 APR_DECLARE(apr_status_t) apr_mmap_delete(apr_mmap_t *mm)
 {
-    apr_status_t rv;
+    apr_status_t rv = APR_SUCCESS;
 
-    if ((rv = mmap_cleanup(mm)) == APR_SUCCESS) {
+    if (mm->is_owner && ((rv = mmap_cleanup(mm)) == APR_SUCCESS)) {
         apr_pool_cleanup_kill(mm->cntxt, mm, mmap_cleanup);
         return APR_SUCCESS;
     }