On 20. 8. 25 14:56, Yann Ylavic wrote:
On Thu, Aug 7, 2025 at 8:19 AM<br...@apache.org> wrote:
Author: brane
Date: Thu Aug  7 06:19:52 2025
New Revision: 1927658

Log:
Fix pool debugging. With lifetime or owner checks enabled, pools couldn't
even be created, except for the global pool which is a special case.

* memory/unix/apr_pools.c
   (struct apr_pool_t): Add an 'unmanaged' member for pool-debug mode.
    The lifetime checks must be skipped for unmanaged pools, since they
    inevitably fail: apr_pool_is_child_of() expects all pools to have a
    parent, which obviously is not the case for unmanaged pools.

   (apr_pool_create_ex_debug): Create the pool's mutex after the parent
    has been assigned, because that involves an allocation which triggers
    a lifetime check which ... well, see above.

   (apr_pool_create_unmanaged): Set the pool->unmanaged flag and create the
    pool's mutex after the owner and allocator have been assigned. A pool
    without the owner set fails the ownershhip check, and without an
    allocator it's sort of hard to allocate space for the mutex.

Modified:
    apr/apr/trunk/memory/unix/apr_pools.c
[]
--- apr/apr/trunk/memory/unix/apr_pools.c       Thu Aug  7 05:08:51 2025        
(r1927657)
+++ apr/apr/trunk/memory/unix/apr_pools.c       Thu Aug  7 06:19:52 2025        
(r1927658)
[]
@@ -2063,6 +2065,22 @@ 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;
Here we might fail and need to rewind the above if parent != NULL,
maybe something like this:

Index: memory/unix/apr_pools.c
===================================================================
--- memory/unix/apr_pools.c    (revision 1927913)
+++ memory/unix/apr_pools.c    (working copy)
@@ -2095,7 +2095,15 @@ APR_DECLARE(apr_status_t) apr_pool_create_ex_debug
           */
          if ((rv = apr_thread_mutex_create(&pool->mutex,
                  APR_THREAD_MUTEX_NESTED, pool)) != APR_SUCCESS) {
-            free(pool);
+            /* Unwind parent binding from above eventually */
+            if (parent) {
+                pool_lock(parent);
+                pool_destroy_debug(pool, file_line);
+                pool_unlock(parent);
+            }
+            else {
+                free(pool);
+            }
              return rv;
          }
      }
?

[]
@@ -2130,11 +2132,30 @@ 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;

  #if APR_HAS_THREADS
+    pool->owner = apr_os_thread_current();
+#endif /* APR_HAS_THREADS */
+#ifdef NETWARE
+    pool->owner_proc = (apr_os_proc_t)getnlmhandle();
+#endif /* defined(NETWARE) */
+
+    if ((pool_allocator = allocator) == NULL) {
+        apr_status_t rv;
+        if ((rv = apr_allocator_create(&pool_allocator)) != APR_SUCCESS) {
+            if (abort_fn)
+                abort_fn(rv);
+            return rv;
+        }
+        pool_allocator->owner = pool;
+    }
+    pool->allocator = pool_allocator;
+
+#if APR_HAS_THREADS
      {
          apr_status_t rv;
Likewise here we need to destroy pool_allocator eventually on failure,
something like this:

Index: memory/unix/apr_pools.c
===================================================================
--- memory/unix/apr_pools.c    (revision 1927913)
+++ memory/unix/apr_pools.c    (working copy)
@@ -2169,6 +2177,10 @@ APR_DECLARE(apr_status_t) apr_pool_create_unmanage
           */
          if ((rv = apr_thread_mutex_create(&pool->mutex,
                  APR_THREAD_MUTEX_NESTED, pool)) != APR_SUCCESS) {
+            /* Don't leak own allocator created above eventually */
+            if (pool_allocator->owner == pool) {
+                apr_allocator_destroy(pool_allocator);
+            }
              free(pool);
              return rv;
          }
?



I'll check. I didn't dig that deep, I only moved the code around so that the data dependencies lined up.

-- Brane

Reply via email to