Re: svn commit: r1927658 - apr/apr/trunk/memory/unix

By the way, this patch applies cleanly to 1.7.x and 1.8.x, but the test 
failure modes are a bit different. Still, I think it's worthwhile to 
backport.

I'm not sure how the backporting process works here, though. My memory 
is shot. Help?

-- Brane


On 12. 8. 25 19:37, Branko Čibej wrote:
>  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 something 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) */
> > 
> >