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;
}
?
Regards;
Yann.
