svn commit: r1927951 - in apr/apr/branches/1.7.x: . memory/unix

[yl...@apache.org]

Author: ylavic
Date: Thu Aug 21 14:22:52 2025
New Revision: 1927951

Log:
apr_pools: Follow up to r1927658.

In apr_pool_create_ex_debug(), attaching the pool to its parent before creating
the mutex races with other threads running apr_pool_walk_tree() on any ancestor
and finding the pool and crashing on pool_lock() with ->mutex == NULL.
Fix this by moving back the attachment after the mutex is created.

To prevent apr_pool_check_lifetime() failing when the ->mutex is created in
apr_pool_create_ex_debug() with no ->parent (per r1927658), let's make it
ignore pools with ->parent == NULL. This covers both the global pool, all the
unmanaged pools and finally the internal case in apr_pool_create_ex_debug().
There is no way for any other pool to have ->parent == NULL since it's forced
to the global_pool when no parent is given.

* memory/unix/apr_pools.c
  (struct apr_pool_t): Remove the "unmanaged" flag since it's no longer used,
   unmanaged pools have their ->parent == NULL.

  (apr_pool_check_lifetime): Ignore pools with ->parent == NULL, which covers
   all the cases.

  (apr_pool_create_ex_debug): Move back setting the ->parent after the ->mutex
   is created.

  (apr_pool_create_unmanaged_ex_debug): Free the owned allocator if the ->mutex
   creation failed.


apr_pools: abort() if creating the pool mutex failed (APR_POOL_DEBUG mode).

* memory/unix/apr_pools.c
  (apr_pool_create_ex_debug): Call abort_fn if any.

  (apr_pool_create_unmanaged_ex_debug): Call abort_fn if any.


Merges r1927948, r1927949 from trunk
Merges r1927950 from ^/apr/apr/branches/1.8.x

Modified:
   apr/apr/branches/1.7.x/   (props changed)
   apr/apr/branches/1.7.x/memory/unix/apr_pools.c

Modified: apr/apr/branches/1.7.x/memory/unix/apr_pools.c
==============================================================================
--- apr/apr/branches/1.7.x/memory/unix/apr_pools.c	Thu Aug 21 14:21:18 2025	(r1927950)
+++ apr/apr/branches/1.7.x/memory/unix/apr_pools.c	Thu Aug 21 14:22:52 2025	(r1927951)
@@ -590,7 +590,6 @@ struct apr_pool_t {
     apr_os_thread_t       owner;
     apr_thread_mutex_t   *mutex;
 #endif /* APR_HAS_THREADS */
-    int                   unmanaged;
 #endif /* APR_POOL_DEBUG */
 #ifdef NETWARE
     apr_os_proc_t         owner_proc;
@@ -1605,9 +1604,14 @@ static void apr_pool_check_lifetime(apr_
      * ok, since the only user is apr_pools.c.  Unless
      * people have searched for the top level parent and
      * started to use that...
+     * Like the global pool, unmanaged pools have their
+     * own lifetime and no ->parent, ignore both here.
+     * Last (internal) case is from apr_pool_create_ex_debug()
+     * where pool->mutex is created before attaching to the
+     * parent, hence an allocation happens with no ->parent
+     * nor lifetime to be checked here.
      */
-    if (pool == global_pool || global_pool == NULL
-        || (pool->parent == NULL && pool->unmanaged))
+    if (pool->parent == NULL)
         return;
 
     /* Lifetime
@@ -2037,22 +2041,6 @@ APR_DECLARE(apr_status_t) apr_pool_creat
     pool->owner_proc = (apr_os_proc_t)getnlmhandle();
 #endif /* defined(NETWARE) */
 
-    if ((pool->parent = parent) != NULL) {
-        pool_lock(parent);
-
-        if ((pool->sibling = parent->child) != NULL)
-            pool->sibling->ref = &pool->sibling;
-
-        parent->child = pool;
-        pool->ref = &parent->child;
-
-        pool_unlock(parent);
-    }
-    else {
-        pool->sibling = NULL;
-        pool->ref = NULL;
-    }
-
 #if APR_HAS_THREADS
     if (parent == NULL || parent->allocator != allocator) {
         apr_status_t rv;
@@ -2067,6 +2055,8 @@ APR_DECLARE(apr_status_t) apr_pool_creat
          */
         if ((rv = apr_thread_mutex_create(&pool->mutex,
                 APR_THREAD_MUTEX_NESTED, pool)) != APR_SUCCESS) {
+            if (abort_fn)
+                abort_fn(rv);
             free(pool);
             return rv;
         }
@@ -2076,6 +2066,22 @@ APR_DECLARE(apr_status_t) apr_pool_creat
     }
 #endif /* APR_HAS_THREADS */
 
+    if ((pool->parent = parent) != NULL) {
+        pool_lock(parent);
+
+        if ((pool->sibling = parent->child) != NULL)
+            pool->sibling->ref = &pool->sibling;
+
+        parent->child = pool;
+        pool->ref = &parent->child;
+
+        pool_unlock(parent);
+    }
+    else {
+        pool->sibling = NULL;
+        pool->ref = NULL;
+    }
+
 #if (APR_POOL_DEBUG & APR_POOL_DEBUG_VERBOSE)
     apr_pool_log_event(pool, "CREATE", file_line, 1);
 #endif /* (APR_POOL_DEBUG & APR_POOL_DEBUG_VERBOSE) */
@@ -2113,7 +2119,6 @@ APR_DECLARE(apr_status_t) apr_pool_creat
 
     memset(pool, 0, SIZEOF_POOL_T);
 
-    pool->unmanaged = 1;
     pool->abort_fn = abort_fn;
     pool->tag = file_line;
     pool->file_line = file_line;
@@ -2150,6 +2155,11 @@ APR_DECLARE(apr_status_t) apr_pool_creat
          */
         if ((rv = apr_thread_mutex_create(&pool->mutex,
                 APR_THREAD_MUTEX_NESTED, pool)) != APR_SUCCESS) {
+            if (abort_fn)
+                abort_fn(rv);
+            /* Free the allocator created/owned above eventually */
+            if (pool_allocator->owner == pool)
+                apr_allocator_destroy(pool_allocator);
             free(pool);
             return rv;
         }