[Sent from the wrong e-mail address the first time]

Found this while trying to use pool debugging to track down an issue in Serf. Apparently testing pool debugging hasn't been much of a priority, heh.

Related to this, with lifetime and owner checking enabled, some of our tests break; I haven't tracked these down yet, but the symptoms are that the test program aborts when it tries to destroy a pool, due to ownership checks; apparently the pool is destroyed by a different thread than the one that created it. This happens in testlockperf, testmutexscope and testall.

Given that this is a likely scenario when multiple threads create pools, and then the main thread calls apr_terminate(), I wonder if these ownership checks are necessary. Or, conversely, if they should be disabled during [global] pool destruction. They're actually triggered by apr_pool_clear(), which /should/ pass a strict ownership check under normal circumstances.

-- Brane

--
On 2025/08/07 06:19:52 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

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)
@@ -600,6 +600,7 @@ 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;
@@ -1619,7 +1620,8 @@ static void apr_pool_check_lifetime(apr_
      * people have searched for the top level parent and
      * started to use that...
      */
-    if (pool == global_pool || global_pool == NULL)
+    if (pool == global_pool || global_pool == NULL
+        || (pool->parent == NULL && pool->unmanaged))
         return;
/* Lifetime
@@ -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;
@@ -2086,22 +2104,6 @@ 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) */
@@ -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;
@@ -2154,24 +2175,6 @@ APR_DECLARE(apr_status_t) apr_pool_creat
     }
 #endif /* APR_HAS_THREADS */
-#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_POOL_DEBUG & APR_POOL_DEBUG_VERBOSE)
     apr_pool_log_event(pool, "CREATEU", file_line, 1);
 #endif /* (APR_POOL_DEBUG & APR_POOL_DEBUG_VERBOSE) */


Reply via email to