Skip to content

Commit 067df4a

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. apr_pools: abort() if creating the pool mutex failed (APR_POOL_DEBUG mode). * memory/unix/apr_pools.c (apr_pool_create_ex_debug): Call abort_fn if any. (apr_pool_create_unmanaged_ex_debug): Call abort_fn if any. Merges r1927948, r1927949 from trunk git-svn-id: https://svn.apache.org/repos/asf/apr/apr/branches/1.8.x@1927950 13f79535-47bb-0310-9956-ffa450edef68
1 parent 939a165 commit 067df4a

File tree

1 file changed

+30
-20
lines changed

1 file changed

+30
-20
lines changed

memory/unix/apr_pools.c

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -590,7 +590,6 @@ struct apr_pool_t {
590590
apr_os_thread_t owner;
591591
apr_thread_mutex_t *mutex;
592592
#endif /* APR_HAS_THREADS */
593-
int unmanaged;
594593
#endif /* APR_POOL_DEBUG */
595594
#ifdef NETWARE
596595
apr_os_proc_t owner_proc;
@@ -1605,9 +1604,14 @@ static void apr_pool_check_lifetime(apr_pool_t *pool)
16051604
* ok, since the only user is apr_pools.c. Unless
16061605
* people have searched for the top level parent and
16071606
* started to use that...
1607+
* Like the global pool, unmanaged pools have their
1608+
* own lifetime and no ->parent, ignore both here.
1609+
* Last (internal) case is from apr_pool_create_ex_debug()
1610+
* where pool->mutex is created before attaching to the
1611+
* parent, hence an allocation happens with no ->parent
1612+
* nor lifetime to be checked here.
16081613
*/
1609-
if (pool == global_pool || global_pool == NULL
1610-
|| (pool->parent == NULL && pool->unmanaged))
1614+
if (pool->parent == NULL)
16111615
return;
16121616

16131617
/* Lifetime
@@ -2037,22 +2041,6 @@ APR_DECLARE(apr_status_t) apr_pool_create_ex_debug(apr_pool_t **newpool,
20372041
pool->owner_proc = (apr_os_proc_t)getnlmhandle();
20382042
#endif /* defined(NETWARE) */
20392043

2040-
if ((pool->parent = parent) != NULL) {
2041-
pool_lock(parent);
2042-
2043-
if ((pool->sibling = parent->child) != NULL)
2044-
pool->sibling->ref = &pool->sibling;
2045-
2046-
parent->child = pool;
2047-
pool->ref = &parent->child;
2048-
2049-
pool_unlock(parent);
2050-
}
2051-
else {
2052-
pool->sibling = NULL;
2053-
pool->ref = NULL;
2054-
}
2055-
20562044
#if APR_HAS_THREADS
20572045
if (parent == NULL || parent->allocator != allocator) {
20582046
apr_status_t rv;
@@ -2067,6 +2055,8 @@ APR_DECLARE(apr_status_t) apr_pool_create_ex_debug(apr_pool_t **newpool,
20672055
*/
20682056
if ((rv = apr_thread_mutex_create(&pool->mutex,
20692057
APR_THREAD_MUTEX_NESTED, pool)) != APR_SUCCESS) {
2058+
if (abort_fn)
2059+
abort_fn(rv);
20702060
free(pool);
20712061
return rv;
20722062
}
@@ -2076,6 +2066,22 @@ APR_DECLARE(apr_status_t) apr_pool_create_ex_debug(apr_pool_t **newpool,
20762066
}
20772067
#endif /* APR_HAS_THREADS */
20782068

2069+
if ((pool->parent = parent) != NULL) {
2070+
pool_lock(parent);
2071+
2072+
if ((pool->sibling = parent->child) != NULL)
2073+
pool->sibling->ref = &pool->sibling;
2074+
2075+
parent->child = pool;
2076+
pool->ref = &parent->child;
2077+
2078+
pool_unlock(parent);
2079+
}
2080+
else {
2081+
pool->sibling = NULL;
2082+
pool->ref = NULL;
2083+
}
2084+
20792085
#if (APR_POOL_DEBUG & APR_POOL_DEBUG_VERBOSE)
20802086
apr_pool_log_event(pool, "CREATE", file_line, 1);
20812087
#endif /* (APR_POOL_DEBUG & APR_POOL_DEBUG_VERBOSE) */
@@ -2113,7 +2119,6 @@ APR_DECLARE(apr_status_t) apr_pool_create_unmanaged_ex_debug(apr_pool_t **newpoo
21132119

21142120
memset(pool, 0, SIZEOF_POOL_T);
21152121

2116-
pool->unmanaged = 1;
21172122
pool->abort_fn = abort_fn;
21182123
pool->tag = file_line;
21192124
pool->file_line = file_line;
@@ -2150,6 +2155,11 @@ APR_DECLARE(apr_status_t) apr_pool_create_unmanaged_ex_debug(apr_pool_t **newpoo
21502155
*/
21512156
if ((rv = apr_thread_mutex_create(&pool->mutex,
21522157
APR_THREAD_MUTEX_NESTED, pool)) != APR_SUCCESS) {
2158+
if (abort_fn)
2159+
abort_fn(rv);
2160+
/* Free the allocator created/owned above eventually */
2161+
if (pool_allocator->owner == pool)
2162+
apr_allocator_destroy(pool_allocator);
21532163
free(pool);
21542164
return rv;
21552165
}

0 commit comments

Comments
 (0)