This change does introduce an API change, and a somewhat possibly dangerous structure size alteration, and seems to be out of bounds for apr 1.7.x branch.
Anyone using a build crossing runtimes between apr 1.7.0 and 1.7.1 may experience unintended side-effects. The eventual change still is present for apr 1.8 adopters. While it's true that include/arch and include/private are not considered generally supported, folks are known to use them, and macros may cause the library to bleed internals. On Fri, Dec 4, 2020 at 12:15 PM <yla...@apache.org> wrote: > > Author: ylavic > Date: Fri Dec 4 18:15:55 2020 > New Revision: 1884103 > > URL: http://svn.apache.org/viewvc?rev=1884103&view=rev > Log: > Merge r936323, r1460182, r1884077, r1884078 from trunk: > > OS/2: Clean up a thread's pool when it terminates. > > > <not backported in 1.7.x> > Add apr_pool_owner_set function to allow use of pool debugging with threads > > Actually this function has been mentioned in the docs for over 10 years > but has never been implemented. > </not backported in 1.7.x> > > Also consistently destroy the thread's pool when it exits normally, not only > on apr_thread_exit(). This was already done on OS2. > > Other platforms than unix are untested. > > > apr_thread: destroy the thread's pool at _join() time, unless _detach()ed. > > Destroying a joinable thread pool from apr_thread_exit() or when the thread > function returns, i.e. from inside the thread itself, is racy or deadlocky > with APR_POOL_DEBUG, with the parent pool being destroyed. > > This commit adds a ->detached flag in each arch's apr_thread_t struct to track > whether a thread is detached (either at _create() or _detach() time). If > detached, the pool is destroyed when the thread exits, otherwise when the > thread is joined with apr_thread_join(). > > > apr_thread: use unmanaged pools for detached threads. > > A detached thread is by definition out of control, unjoinable, unmanaged, > and it can terminate/exit after its parent pool is detroyed. > > To avoid use-after-free in this case, let's use an unmanaged pool for detached > threads, either by creating an unmanaged pool from the start if the thread > is created detached, or by "unmanaging" the pool if the thread is detached > later with apr_thread_detach(). > > To "umanage" the pool, provide a new internal helper, apr__pool_unmanage() > which takes care of removing the pool from its parent's list. > > > Submitted by: bjh, sf, ylavic, ylavic > > Modified: > apr/apr/branches/1.7.x/ (props changed) > apr/apr/branches/1.7.x/include/arch/beos/apr_arch_threadproc.h > apr/apr/branches/1.7.x/include/arch/netware/apr_arch_threadproc.h > apr/apr/branches/1.7.x/include/arch/unix/apr_arch_threadproc.h > apr/apr/branches/1.7.x/include/arch/win32/apr_arch_threadproc.h > apr/apr/branches/1.7.x/memory/unix/apr_pools.c > apr/apr/branches/1.7.x/threadproc/beos/thread.c > apr/apr/branches/1.7.x/threadproc/netware/thread.c > apr/apr/branches/1.7.x/threadproc/os2/thread.c > apr/apr/branches/1.7.x/threadproc/unix/thread.c > apr/apr/branches/1.7.x/threadproc/win32/thread.c > > Propchange: apr/apr/branches/1.7.x/ > ------------------------------------------------------------------------------ > Merged /apr/apr/trunk:r936323,1460182,1884077-1884078 > > Modified: apr/apr/branches/1.7.x/include/arch/beos/apr_arch_threadproc.h > URL: > http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/include/arch/beos/apr_arch_threadproc.h?rev=1884103&r1=1884102&r2=1884103&view=diff > ============================================================================== > --- apr/apr/branches/1.7.x/include/arch/beos/apr_arch_threadproc.h (original) > +++ apr/apr/branches/1.7.x/include/arch/beos/apr_arch_threadproc.h Fri Dec 4 > 18:15:55 2020 > @@ -45,6 +45,7 @@ struct apr_thread_t { > void *data; > apr_thread_start_t func; > apr_status_t exitval; > + int detached; > }; > > struct apr_threadattr_t { > > Modified: apr/apr/branches/1.7.x/include/arch/netware/apr_arch_threadproc.h > URL: > http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/include/arch/netware/apr_arch_threadproc.h?rev=1884103&r1=1884102&r2=1884103&view=diff > ============================================================================== > --- apr/apr/branches/1.7.x/include/arch/netware/apr_arch_threadproc.h > (original) > +++ apr/apr/branches/1.7.x/include/arch/netware/apr_arch_threadproc.h Fri Dec > 4 18:15:55 2020 > @@ -36,6 +36,7 @@ struct apr_thread_t { > void *data; > apr_thread_start_t func; > apr_status_t exitval; > + int detached; > }; > > struct apr_threadattr_t { > > Modified: apr/apr/branches/1.7.x/include/arch/unix/apr_arch_threadproc.h > URL: > http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/include/arch/unix/apr_arch_threadproc.h?rev=1884103&r1=1884102&r2=1884103&view=diff > ============================================================================== > --- apr/apr/branches/1.7.x/include/arch/unix/apr_arch_threadproc.h (original) > +++ apr/apr/branches/1.7.x/include/arch/unix/apr_arch_threadproc.h Fri Dec 4 > 18:15:55 2020 > @@ -59,6 +59,7 @@ struct apr_thread_t { > void *data; > apr_thread_start_t func; > apr_status_t exitval; > + int detached; > }; > > struct apr_threadattr_t { > > Modified: apr/apr/branches/1.7.x/include/arch/win32/apr_arch_threadproc.h > URL: > http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/include/arch/win32/apr_arch_threadproc.h?rev=1884103&r1=1884102&r2=1884103&view=diff > ============================================================================== > --- apr/apr/branches/1.7.x/include/arch/win32/apr_arch_threadproc.h (original) > +++ apr/apr/branches/1.7.x/include/arch/win32/apr_arch_threadproc.h Fri Dec > 4 18:15:55 2020 > @@ -31,6 +31,7 @@ struct apr_thread_t { > void *data; > apr_thread_start_t func; > apr_status_t exitval; > + int exited; > }; > > struct apr_threadattr_t { > > Modified: apr/apr/branches/1.7.x/memory/unix/apr_pools.c > URL: > http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/memory/unix/apr_pools.c?rev=1884103&r1=1884102&r2=1884103&view=diff > ============================================================================== > --- apr/apr/branches/1.7.x/memory/unix/apr_pools.c (original) > +++ apr/apr/branches/1.7.x/memory/unix/apr_pools.c Fri Dec 4 18:15:55 2020 > @@ -2324,6 +2324,45 @@ APR_DECLARE(void) apr_pool_lock(apr_pool > > #endif /* !APR_POOL_DEBUG */ > > +/* For APR internal use only (for now). > + * Detach the pool from its/any parent (i.e. un-manage). > + */ > +apr_status_t apr__pool_unmanage(apr_pool_t *pool); > +apr_status_t apr__pool_unmanage(apr_pool_t *pool) > +{ > + apr_pool_t *parent = pool->parent; > + > + if (!parent) { > + return APR_NOTFOUND; > + } > + > +#if APR_POOL_DEBUG > + if (pool->allocator && pool->allocator == parent->allocator) { > + return APR_EINVAL; > + } > + apr_thread_mutex_lock(parent->mutex); > +#else > + if (pool->allocator == parent->allocator) { > + return APR_EINVAL; > + } > + allocator_lock(parent->allocator); > +#endif > + > + /* Remove the pool from the parent's children */ > + if ((*pool->ref = pool->sibling) != NULL) { > + pool->sibling->ref = pool->ref; > + } > + pool->parent = NULL; > + > +#if APR_POOL_DEBUG > + apr_thread_mutex_unlock(parent->mutex); > +#else > + allocator_unlock(parent->allocator); > +#endif > + > + return APR_SUCCESS; > +} > + > #ifdef NETWARE > void netware_pool_proc_cleanup () > { > > Modified: apr/apr/branches/1.7.x/threadproc/beos/thread.c > URL: > http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/threadproc/beos/thread.c?rev=1884103&r1=1884102&r2=1884103&view=diff > ============================================================================== > --- apr/apr/branches/1.7.x/threadproc/beos/thread.c (original) > +++ apr/apr/branches/1.7.x/threadproc/beos/thread.c Fri Dec 4 18:15:55 2020 > @@ -17,6 +17,9 @@ > #include "apr_arch_threadproc.h" > #include "apr_portable.h" > > +/* Internal (from apr_pools.c) */ > +extern apr_status_t apr__pool_unmanage(apr_pool_t *pool); > + > APR_DECLARE(apr_status_t) apr_threadattr_create(apr_threadattr_t **new, > apr_pool_t *pool) > { > (*new) = (apr_threadattr_t *)apr_palloc(pool, > @@ -65,7 +68,13 @@ APR_DECLARE(apr_status_t) apr_threadattr > static void *dummy_worker(void *opaque) > { > apr_thread_t *thd = (apr_thread_t*)opaque; > - return thd->func(thd, thd->data); > + void *ret; > + > + ret = thd->func(thd, thd->data); > + if (thd->detached) { > + apr_pool_destroy(thd->pool); > + } > + return ret; > } > > APR_DECLARE(apr_status_t) apr_thread_create(apr_thread_t **new, > apr_threadattr_t *attr, > @@ -75,7 +84,7 @@ APR_DECLARE(apr_status_t) apr_thread_cre > int32 temp; > apr_status_t stat; > > - (*new) = (apr_thread_t *)apr_palloc(pool, sizeof(apr_thread_t)); > + (*new) = (apr_thread_t *)apr_pcalloc(pool, sizeof(apr_thread_t)); > if ((*new) == NULL) { > return APR_ENOMEM; > } > @@ -84,17 +93,41 @@ APR_DECLARE(apr_status_t) apr_thread_cre > (*new)->func = func; > (*new)->exitval = -1; > > + (*new)->detached = (attr && apr_threadattr_detach_get(attr) == > APR_DETACH); > + if ((*new)->detached) { > + stat = apr_pool_create_unmanaged_ex(&(*new)->pool, > + apr_pool_abort_get(pool), > + NULL); > + } > + else { > + /* The thread can be apr_thread_detach()ed later, so the pool needs > + * its own allocator to not depend on the parent pool which could be > + * destroyed before the thread exits. The allocator needs no mutex > + * obviously since the pool should not be used nor create children > + * pools outside the thread. > + */ > + apr_allocator_t *allocator; > + if (apr_allocator_create(&allocator) != APR_SUCCESS) { > + return APR_ENOMEM; > + } > + stat = apr_pool_create_ex(&(*new)->pool, pool, NULL, allocator); > + if (stat == APR_SUCCESS) { > + apr_allocator_owner_set(allocator, (*new)->pool); > + } > + else { > + apr_allocator_destroy(allocator); > + } > + } > + if (stat != APR_SUCCESS) { > + return stat; > + } > + > /* First we create the new thread...*/ > if (attr) > temp = attr->attr; > else > temp = B_NORMAL_PRIORITY; > > - stat = apr_pool_create(&(*new)->pool, pool); > - if (stat != APR_SUCCESS) { > - return stat; > - } > - > (*new)->td = spawn_thread((thread_func)dummy_worker, > "apr thread", > temp, > @@ -121,8 +154,10 @@ int apr_os_thread_equal(apr_os_thread_t > > APR_DECLARE(apr_status_t) apr_thread_exit(apr_thread_t *thd, apr_status_t > retval) > { > - apr_pool_destroy(thd->pool); > thd->exitval = retval; > + if (thd->detached) { > + apr_pool_destroy(thd->pool); > + } > exit_thread ((status_t)(retval)); > /* This will never be reached... */ > return APR_SUCCESS; > @@ -131,6 +166,11 @@ APR_DECLARE(apr_status_t) apr_thread_exi > APR_DECLARE(apr_status_t) apr_thread_join(apr_status_t *retval, apr_thread_t > *thd) > { > status_t rv = 0, ret; > + > + if (thd->detached) { > + return APR_EINVAL; > + } > + > ret = wait_for_thread(thd->td, &rv); > if (ret == B_NO_ERROR) { > *retval = rv; > @@ -142,6 +182,7 @@ APR_DECLARE(apr_status_t) apr_thread_joi > */ > if (thd->exitval != -1) { > *retval = thd->exitval; > + apr_pool_destroy(thd->pool); > return APR_SUCCESS; > } else > return ret; > @@ -150,7 +191,14 @@ APR_DECLARE(apr_status_t) apr_thread_joi > > APR_DECLARE(apr_status_t) apr_thread_detach(apr_thread_t *thd) > { > - if (suspend_thread(thd->td) == B_NO_ERROR){ > + if (thd->detached) { > + return APR_EINVAL; > + } > + > + if (suspend_thread(thd->td) == B_NO_ERROR) { > + /* Detach from the parent pool too */ > + apr__pool_unmanage(thd->pool); > + thd->detached = 1; > return APR_SUCCESS; > } > else { > > Modified: apr/apr/branches/1.7.x/threadproc/netware/thread.c > URL: > http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/threadproc/netware/thread.c?rev=1884103&r1=1884102&r2=1884103&view=diff > ============================================================================== > --- apr/apr/branches/1.7.x/threadproc/netware/thread.c (original) > +++ apr/apr/branches/1.7.x/threadproc/netware/thread.c Fri Dec 4 18:15:55 > 2020 > @@ -21,6 +21,9 @@ > > static int thread_count = 0; > > +/* Internal (from apr_pools.c) */ > +extern apr_status_t apr__pool_unmanage(apr_pool_t *pool); > + > apr_status_t apr_threadattr_create(apr_threadattr_t **new, > apr_pool_t *pool) > { > @@ -67,7 +70,13 @@ APR_DECLARE(apr_status_t) apr_threadattr > static void *dummy_worker(void *opaque) > { > apr_thread_t *thd = (apr_thread_t *)opaque; > - return thd->func(thd, thd->data); > + void *ret; > + > + ret = thd->func(thd, thd->data); > + if (thd->detached) { > + apr_pool_destroy(thd->pool); > + } > + return ret; > } > > apr_status_t apr_thread_create(apr_thread_t **new, > @@ -97,7 +106,7 @@ apr_status_t apr_thread_create(apr_threa > stack_size = attr->stack_size; > } > > - (*new) = (apr_thread_t *)apr_palloc(pool, sizeof(apr_thread_t)); > + (*new) = (apr_thread_t *)apr_pcalloc(pool, sizeof(apr_thread_t)); > > if ((*new) == NULL) { > return APR_ENOMEM; > @@ -106,8 +115,32 @@ apr_status_t apr_thread_create(apr_threa > (*new)->data = data; > (*new)->func = func; > (*new)->thread_name = (char*)apr_pstrdup(pool, threadName); > - > - stat = apr_pool_create(&(*new)->pool, pool); > + > + (*new)->detached = (attr && apr_threadattr_detach_get(attr) == > APR_DETACH); > + if ((*new)->detached) { > + stat = apr_pool_create_unmanaged_ex(&(*new)->pool, > + apr_pool_abort_get(pool), > + NULL); > + } > + else { > + /* The thread can be apr_thread_detach()ed later, so the pool needs > + * its own allocator to not depend on the parent pool which could be > + * destroyed before the thread exits. The allocator needs no mutex > + * obviously since the pool should not be used nor create children > + * pools outside the thread. > + */ > + apr_allocator_t *allocator; > + if (apr_allocator_create(&allocator) != APR_SUCCESS) { > + return APR_ENOMEM; > + } > + stat = apr_pool_create_ex(&(*new)->pool, pool, NULL, allocator); > + if (stat == APR_SUCCESS) { > + apr_allocator_owner_set(allocator, (*new)->pool); > + } > + else { > + apr_allocator_destroy(allocator); > + } > + } > if (stat != APR_SUCCESS) { > return stat; > } > @@ -158,7 +191,9 @@ apr_status_t apr_thread_exit(apr_thread_ > apr_status_t retval) > { > thd->exitval = retval; > - apr_pool_destroy(thd->pool); > + if (thd->detached) { > + apr_pool_destroy(thd->pool); > + } > NXThreadExit(NULL); > return APR_SUCCESS; > } > @@ -169,8 +204,13 @@ apr_status_t apr_thread_join(apr_status_ > apr_status_t stat; > NXThreadId_t dthr; > > + if (thd->detached) { > + return APR_EINVAL; > + } > + > if ((stat = NXThreadJoin(thd->td, &dthr, NULL)) == 0) { > *retval = thd->exitval; > + apr_pool_destroy(thd->pool); > return APR_SUCCESS; > } > else { > @@ -180,6 +220,14 @@ apr_status_t apr_thread_join(apr_status_ > > apr_status_t apr_thread_detach(apr_thread_t *thd) > { > + if (thd->detached) { > + return APR_EINVAL; > + } > + > + /* Detach from the parent pool too */ > + apr__pool_unmanage(thd->pool); > + thd->detached = 1; > + > return APR_SUCCESS; > } > > > Modified: apr/apr/branches/1.7.x/threadproc/os2/thread.c > URL: > http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/threadproc/os2/thread.c?rev=1884103&r1=1884102&r2=1884103&view=diff > ============================================================================== > --- apr/apr/branches/1.7.x/threadproc/os2/thread.c (original) > +++ apr/apr/branches/1.7.x/threadproc/os2/thread.c Fri Dec 4 18:15:55 2020 > @@ -24,6 +24,10 @@ > #include "apr_arch_file_io.h" > #include <stdlib.h> > > +/* Internal (from apr_pools.c) */ > +extern apr_status_t apr__pool_unmanage(apr_pool_t *pool); > + > + > APR_DECLARE(apr_status_t) apr_threadattr_create(apr_threadattr_t **new, > apr_pool_t *pool) > { > (*new) = (apr_threadattr_t *)apr_palloc(pool, sizeof(apr_threadattr_t)); > @@ -70,6 +74,9 @@ static void apr_thread_begin(void *arg) > { > apr_thread_t *thread = (apr_thread_t *)arg; > thread->exitval = thread->func(thread, thread->data); > + if (thd->attr->attr & APR_THREADATTR_DETACHED) { > + apr_pool_destroy(thread->pool); > + } > } > > > @@ -81,7 +88,7 @@ APR_DECLARE(apr_status_t) apr_thread_cre > apr_status_t stat; > apr_thread_t *thread; > > - thread = (apr_thread_t *)apr_palloc(pool, sizeof(apr_thread_t)); > + thread = (apr_thread_t *)apr_pcalloc(pool, sizeof(apr_thread_t)); > *new = thread; > > if (thread == NULL) { > @@ -91,8 +98,40 @@ APR_DECLARE(apr_status_t) apr_thread_cre > thread->attr = attr; > thread->func = func; > thread->data = data; > - stat = apr_pool_create(&thread->pool, pool); > - > + > + if (attr && attr->attr & APR_THREADATTR_DETACHED) { > + stat = apr_pool_create_unmanaged_ex(&thread->pool, > + apr_pool_abort_get(pool), > + NULL); > + } > + else { > + /* The thread can be apr_thread_detach()ed later, so the pool needs > + * its own allocator to not depend on the parent pool which could be > + * destroyed before the thread exits. The allocator needs no mutex > + * obviously since the pool should not be used nor create children > + * pools outside the thread. > + */ > + apr_allocator_t *allocator; > + if (apr_allocator_create(&allocator) != APR_SUCCESS) { > + return APR_ENOMEM; > + } > + stat = apr_pool_create_ex(&thread->pool, pool, NULL, allocator); > + if (stat == APR_SUCCESS) { > + apr_thread_mutex_t *mutex; > + stat = apr_thread_mutex_create(&mutex, APR_THREAD_MUTEX_DEFAULT, > + thread->pool); > + if (stat == APR_SUCCESS) { > + apr_allocator_mutex_set(allocator, mutex); > + apr_allocator_owner_set(allocator, thread->pool); > + } > + else { > + apr_pool_destroy(thread->pool); > + } > + } > + if (stat != APR_SUCCESS) { > + apr_allocator_destroy(allocator); > + } > + } > if (stat != APR_SUCCESS) { > return stat; > } > @@ -132,6 +171,9 @@ APR_DECLARE(apr_os_thread_t) apr_os_thre > APR_DECLARE(apr_status_t) apr_thread_exit(apr_thread_t *thd, apr_status_t > retval) > { > thd->exitval = retval; > + if (thd->attr->attr & APR_THREADATTR_DETACHED) { > + apr_pool_destroy(thd->pool); > + } > _endthread(); > return -1; /* If we get here something's wrong */ > } > @@ -152,14 +194,24 @@ APR_DECLARE(apr_status_t) apr_thread_joi > rc = 0; /* Thread had already terminated */ > > *retval = thd->exitval; > - return APR_OS2_STATUS(rc); > + if (rc == 0) { > + apr_pool_destroy(thd->pool); > + } > + return APR_FROM_OS_ERROR(rc); > } > > > > APR_DECLARE(apr_status_t) apr_thread_detach(apr_thread_t *thd) > { > + if (thd->attr->attr & APR_THREADATTR_DETACHED) { > + return APR_EINVAL; > + } > + > + /* Detach from the parent pool too */ > + apr__pool_unmanage(thd->pool); > thd->attr->attr |= APR_THREADATTR_DETACHED; > + > return APR_SUCCESS; > } > > > Modified: apr/apr/branches/1.7.x/threadproc/unix/thread.c > URL: > http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/threadproc/unix/thread.c?rev=1884103&r1=1884102&r2=1884103&view=diff > ============================================================================== > --- apr/apr/branches/1.7.x/threadproc/unix/thread.c (original) > +++ apr/apr/branches/1.7.x/threadproc/unix/thread.c Fri Dec 4 18:15:55 2020 > @@ -22,6 +22,9 @@ > > #if APR_HAVE_PTHREAD_H > > +/* Internal (from apr_pools.c) */ > +extern apr_status_t apr__pool_unmanage(apr_pool_t *pool); > + > /* Destroy the threadattr object */ > static apr_status_t threadattr_cleanup(void *data) > { > @@ -139,7 +142,13 @@ APR_DECLARE(apr_status_t) apr_threadattr > static void *dummy_worker(void *opaque) > { > apr_thread_t *thread = (apr_thread_t*)opaque; > - return thread->func(thread, thread->data); > + void *ret; > + > + ret = thread->func(thread, thread->data); > + if (thread->detached) { > + apr_pool_destroy(thread->pool); > + } > + return ret; > } > > APR_DECLARE(apr_status_t) apr_thread_create(apr_thread_t **new, > @@ -166,16 +175,40 @@ APR_DECLARE(apr_status_t) apr_thread_cre > (*new)->data = data; > (*new)->func = func; > > + (*new)->detached = (attr && apr_threadattr_detach_get(attr) == > APR_DETACH); > + if ((*new)->detached) { > + stat = apr_pool_create_unmanaged_ex(&(*new)->pool, > + apr_pool_abort_get(pool), > + NULL); > + } > + else { > + /* The thread can be apr_thread_detach()ed later, so the pool needs > + * its own allocator to not depend on the parent pool which could be > + * destroyed before the thread exits. The allocator needs no mutex > + * obviously since the pool should not be used nor create children > + * pools outside the thread. > + */ > + apr_allocator_t *allocator; > + if (apr_allocator_create(&allocator) != APR_SUCCESS) { > + return APR_ENOMEM; > + } > + stat = apr_pool_create_ex(&(*new)->pool, pool, NULL, allocator); > + if (stat == APR_SUCCESS) { > + apr_allocator_owner_set(allocator, (*new)->pool); > + } > + else { > + apr_allocator_destroy(allocator); > + } > + } > + if (stat != APR_SUCCESS) { > + return stat; > + } > + > if (attr) > temp = &attr->attr; > else > temp = NULL; > > - stat = apr_pool_create(&(*new)->pool, pool); > - if (stat != APR_SUCCESS) { > - return stat; > - } > - > if ((stat = pthread_create((*new)->td, temp, dummy_worker, (*new))) == > 0) { > return APR_SUCCESS; > } > @@ -203,7 +236,9 @@ APR_DECLARE(apr_status_t) apr_thread_exi > apr_status_t retval) > { > thd->exitval = retval; > - apr_pool_destroy(thd->pool); > + if (thd->detached) { > + apr_pool_destroy(thd->pool); > + } > pthread_exit(NULL); > return APR_SUCCESS; > } > @@ -214,8 +249,13 @@ APR_DECLARE(apr_status_t) apr_thread_joi > apr_status_t stat; > apr_status_t *thread_stat; > > + if (thd->detached) { > + return APR_EINVAL; > + } > + > if ((stat = pthread_join(*thd->td,(void *)&thread_stat)) == 0) { > *retval = thd->exitval; > + apr_pool_destroy(thd->pool); > return APR_SUCCESS; > } > else { > @@ -231,11 +271,18 @@ APR_DECLARE(apr_status_t) apr_thread_det > { > apr_status_t stat; > > + if (thd->detached) { > + return APR_EINVAL; > + } > + > #ifdef HAVE_ZOS_PTHREADS > if ((stat = pthread_detach(thd->td)) == 0) { > #else > if ((stat = pthread_detach(*thd->td)) == 0) { > #endif > + /* Detach from the parent pool too */ > + apr__pool_unmanage(thd->pool); > + thd->detached = 1; > > return APR_SUCCESS; > } > > Modified: apr/apr/branches/1.7.x/threadproc/win32/thread.c > URL: > http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/threadproc/win32/thread.c?rev=1884103&r1=1884102&r2=1884103&view=diff > ============================================================================== > --- apr/apr/branches/1.7.x/threadproc/win32/thread.c (original) > +++ apr/apr/branches/1.7.x/threadproc/win32/thread.c Fri Dec 4 18:15:55 2020 > @@ -28,6 +28,9 @@ > /* Chosen for us by apr_initialize */ > DWORD tls_apr_thread = 0; > > +/* Internal (from apr_pools.c) */ > +extern apr_status_t apr__pool_unmanage(apr_pool_t *pool); > + > APR_DECLARE(apr_status_t) apr_threadattr_create(apr_threadattr_t **new, > apr_pool_t *pool) > { > @@ -75,8 +78,14 @@ APR_DECLARE(apr_status_t) apr_threadattr > static void *dummy_worker(void *opaque) > { > apr_thread_t *thd = (apr_thread_t *)opaque; > + void *ret; > + > TlsSetValue(tls_apr_thread, thd->td); > - return thd->func(thd, thd->data); > + ret = thd->func(thd, thd->data); > + if (!thd->td) { /* detached? */ > + apr_pool_destroy(thd->pool); > + } > + return ret; > } > > APR_DECLARE(apr_status_t) apr_thread_create(apr_thread_t **new, > @@ -88,7 +97,7 @@ APR_DECLARE(apr_status_t) apr_thread_cre > unsigned temp; > HANDLE handle; > > - (*new) = (apr_thread_t *)apr_palloc(pool, sizeof(apr_thread_t)); > + (*new) = (apr_thread_t *)apr_pcalloc(pool, sizeof(apr_thread_t)); > > if ((*new) == NULL) { > return APR_ENOMEM; > @@ -96,8 +105,31 @@ APR_DECLARE(apr_status_t) apr_thread_cre > > (*new)->data = data; > (*new)->func = func; > - (*new)->td = NULL; > - stat = apr_pool_create(&(*new)->pool, pool); > + > + if (attr && attr->detach) { > + stat = apr_pool_create_unmanaged_ex(&(*new)->pool, > + apr_pool_abort_get(pool), > + NULL); > + } > + else { > + /* The thread can be apr_thread_detach()ed later, so the pool needs > + * its own allocator to not depend on the parent pool which could be > + * destroyed before the thread exits. The allocator needs no mutex > + * obviously since the pool should not be used nor create children > + * pools outside the thread. > + */ > + apr_allocator_t *allocator; > + if (apr_allocator_create(&allocator) != APR_SUCCESS) { > + return APR_ENOMEM; > + } > + stat = apr_pool_create_ex(&(*new)->pool, pool, NULL, allocator); > + if (stat == APR_SUCCESS) { > + apr_allocator_owner_set(allocator, (*new)->pool); > + } > + else { > + apr_allocator_destroy(allocator); > + } > + } > if (stat != APR_SUCCESS) { > return stat; > } > @@ -132,9 +164,11 @@ APR_DECLARE(apr_status_t) apr_thread_cre > APR_DECLARE(apr_status_t) apr_thread_exit(apr_thread_t *thd, > apr_status_t retval) > { > + thd->exited = 1; > thd->exitval = retval; > - apr_pool_destroy(thd->pool); > - thd->pool = NULL; > + if (!thd->td) { /* detached? */ > + apr_pool_destroy(thd->pool); > + } > #ifndef _WIN32_WCE > _endthreadex(0); > #else > @@ -147,30 +181,42 @@ APR_DECLARE(apr_status_t) apr_thread_joi > apr_thread_t *thd) > { > apr_status_t rv = APR_SUCCESS; > + DWORD ret; > > if (!thd->td) { > /* Can not join on detached threads */ > return APR_DETACH; > } > - rv = WaitForSingleObject(thd->td, INFINITE); > - if ( rv == WAIT_OBJECT_0 || rv == WAIT_ABANDONED) { > + > + ret = WaitForSingleObject(thd->td, INFINITE); > + if (ret == WAIT_OBJECT_0 || ret == WAIT_ABANDONED) { > /* If the thread_exit has been called */ > - if (!thd->pool) > + if (thd->exited) > *retval = thd->exitval; > else > rv = APR_INCOMPLETE; > } > else > rv = apr_get_os_error(); > - CloseHandle(thd->td); > - thd->td = NULL; > + > + if (rv == APR_SUCCESS) { > + CloseHandle(thd->td); > + apr_pool_destroy(thd->pool); > + thd->td = NULL; > + } > > return rv; > } > > APR_DECLARE(apr_status_t) apr_thread_detach(apr_thread_t *thd) > { > - if (thd->td && CloseHandle(thd->td)) { > + if (!thd->td) { > + return APR_EINVAL; > + } > + > + if (CloseHandle(thd->td)) { > + /* Detach from the parent pool too */ > + apr__pool_unmanage(thd->pool); > thd->td = NULL; > return APR_SUCCESS; > } > >
