Skip to content

Commit 55364ad

Browse files
committed
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. git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1927948 13f79535-47bb-0310-9956-ffa450edef68
1 parent 471de34 commit 55364ad

File tree

1 file changed

+26
-20
lines changed

1 file changed

+26
-20
lines changed

memory/unix/apr_pools.c

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -600,7 +600,6 @@ struct apr_pool_t {
600600
apr_os_thread_t owner;
601601
apr_thread_mutex_t *mutex;
602602
#endif /* APR_HAS_THREADS */
603-
int unmanaged;
604603
#endif /* APR_POOL_DEBUG */
605604
#ifdef NETWARE
606605
apr_os_proc_t owner_proc;
@@ -1619,9 +1618,14 @@ static void apr_pool_check_lifetime(apr_pool_t *pool)
16191618
* ok, since the only user is apr_pools.c. Unless
16201619
* people have searched for the top level parent and
16211620
* started to use that...
1621+
* Like the global pool, unmanaged pools have their
1622+
* own lifetime and no ->parent, ignore both here.
1623+
* Last (internal) case is from apr_pool_create_ex_debug()
1624+
* where pool->mutex is created before attaching to the
1625+
* parent, hence an allocation happens with no ->parent
1626+
* nor lifetime to be checked here.
16221627
*/
1623-
if (pool == global_pool || global_pool == NULL
1624-
|| (pool->parent == NULL && pool->unmanaged))
1628+
if (pool->parent == NULL)
16251629
return;
16261630

16271631
/* Lifetime
@@ -2065,22 +2069,6 @@ APR_DECLARE(apr_status_t) apr_pool_create_ex_debug(apr_pool_t **newpool,
20652069
pool->owner_proc = (apr_os_proc_t)getnlmhandle();
20662070
#endif /* defined(NETWARE) */
20672071

2068-
if ((pool->parent = parent) != NULL) {
2069-
pool_lock(parent);
2070-
2071-
if ((pool->sibling = parent->child) != NULL)
2072-
pool->sibling->ref = &pool->sibling;
2073-
2074-
parent->child = pool;
2075-
pool->ref = &parent->child;
2076-
2077-
pool_unlock(parent);
2078-
}
2079-
else {
2080-
pool->sibling = NULL;
2081-
pool->ref = NULL;
2082-
}
2083-
20842072
#if APR_HAS_THREADS
20852073
if (parent == NULL || parent->allocator != allocator) {
20862074
apr_status_t rv;
@@ -2104,6 +2092,22 @@ APR_DECLARE(apr_status_t) apr_pool_create_ex_debug(apr_pool_t **newpool,
21042092
}
21052093
#endif /* APR_HAS_THREADS */
21062094

2095+
if ((pool->parent = parent) != NULL) {
2096+
pool_lock(parent);
2097+
2098+
if ((pool->sibling = parent->child) != NULL)
2099+
pool->sibling->ref = &pool->sibling;
2100+
2101+
parent->child = pool;
2102+
pool->ref = &parent->child;
2103+
2104+
pool_unlock(parent);
2105+
}
2106+
else {
2107+
pool->sibling = NULL;
2108+
pool->ref = NULL;
2109+
}
2110+
21072111
#if (APR_POOL_DEBUG & APR_POOL_DEBUG_VERBOSE)
21082112
apr_pool_log_event(pool, "CREATE", file_line, 1);
21092113
#endif /* (APR_POOL_DEBUG & APR_POOL_DEBUG_VERBOSE) */
@@ -2132,7 +2136,6 @@ APR_DECLARE(apr_status_t) apr_pool_create_unmanaged_ex_debug(apr_pool_t **newpoo
21322136

21332137
memset(pool, 0, SIZEOF_POOL_T);
21342138

2135-
pool->unmanaged = 1;
21362139
pool->abort_fn = abort_fn;
21372140
pool->tag = file_line;
21382141
pool->file_line = file_line;
@@ -2169,6 +2172,9 @@ APR_DECLARE(apr_status_t) apr_pool_create_unmanaged_ex_debug(apr_pool_t **newpoo
21692172
*/
21702173
if ((rv = apr_thread_mutex_create(&pool->mutex,
21712174
APR_THREAD_MUTEX_NESTED, pool)) != APR_SUCCESS) {
2175+
/* Free the allocator created/owned above eventually */
2176+
if (pool_allocator->owner == pool)
2177+
apr_allocator_destroy(pool_allocator);
21722178
free(pool);
21732179
return rv;
21742180
}

0 commit comments

Comments
 (0)