You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by yl...@apache.org on 2020/12/04 18:15:56 UTC
svn commit: r1884103 - in /apr/apr/branches/1.7.x: ./ include/arch/beos/
include/arch/netware/ include/arch/unix/ include/arch/win32/ memory/unix/
threadproc/beos/ threadproc/netware/ threadproc/os2/ threadproc/unix/
threadproc/win32/
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;
}
Re: svn commit: r1884103 - in /apr/apr/branches/1.7.x: ./ include/arch/beos/ include/arch/netware/ include/arch/unix/ include/arch/win32/ memory/unix/ threadproc/beos/ threadproc/netware/ threadproc/os2/ threadproc/unix/ threadproc/win32/
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
Hopefully we aren't waiting any longer for 1.8.0 than 1.7.1, so that
shouldn't be a real risk
On Thu, Jun 30, 2022 at 7:02 AM Yann Ylavic <yl...@gmail.com> wrote:
>
> On Wed, Jun 29, 2022 at 6:09 PM Yann Ylavic <yl...@gmail.com> wrote:
> >
> > By making such impracticable policies for revision changes (semver
> > sense), we are going to simply not fix anything but trivial changes
> > without bumping minor, and no one would/should care about the old
> > (though still "maintained") minor versions. What's the point of
> > releasing 1.7.1 then? Let's go with 1.8.0 only, simpler (less
> > maintenance) and better for everybody..
>
> OK, by re-reading this I realize I was a bit harsh, a bit of
> frustration here (fixes that don't get in..), sorry about that.
>
> After a bit of diving into svn history, this commit was mainly to
> address UAF with apr_thread_pool code when it was using detached
> threads, which is not the case anymore (r1884110, r1889219).
> So the scope of the fix might be for user detached thread usage "only"
> (which may not be common), and APR_POOL_DEBUG (which is mainly never
> by devs only, though something happening in APR_POOL_DEBUG mode might
> happen without when the planets are no longer aligned).
> I'll try to revert and see how it goes for, e.g., httpd's ci (where
> mod_watchdog uses thread pools).
>
> Anyway, if we get bug/crash reports for 1.7.1 and some (time
> consuming) invertigations shows that it's fixed in 1.8, I'm going to
> be a bit frustated too ;)
>
> >
> > Regards;
> > Yann.
Re: svn commit: r1884103 - in /apr/apr/branches/1.7.x: ./ include/arch/beos/ include/arch/netware/ include/arch/unix/ include/arch/win32/ memory/unix/ threadproc/beos/ threadproc/netware/ threadproc/os2/ threadproc/unix/ threadproc/win32/
Posted by Yann Ylavic <yl...@gmail.com>.
On Wed, Jun 29, 2022 at 6:09 PM Yann Ylavic <yl...@gmail.com> wrote:
>
> By making such impracticable policies for revision changes (semver
> sense), we are going to simply not fix anything but trivial changes
> without bumping minor, and no one would/should care about the old
> (though still "maintained") minor versions. What's the point of
> releasing 1.7.1 then? Let's go with 1.8.0 only, simpler (less
> maintenance) and better for everybody..
OK, by re-reading this I realize I was a bit harsh, a bit of
frustration here (fixes that don't get in..), sorry about that.
After a bit of diving into svn history, this commit was mainly to
address UAF with apr_thread_pool code when it was using detached
threads, which is not the case anymore (r1884110, r1889219).
So the scope of the fix might be for user detached thread usage "only"
(which may not be common), and APR_POOL_DEBUG (which is mainly never
by devs only, though something happening in APR_POOL_DEBUG mode might
happen without when the planets are no longer aligned).
I'll try to revert and see how it goes for, e.g., httpd's ci (where
mod_watchdog uses thread pools).
Anyway, if we get bug/crash reports for 1.7.1 and some (time
consuming) invertigations shows that it's fixed in 1.8, I'm going to
be a bit frustated too ;)
>
> Regards;
> Yann.
Re: svn commit: r1884103 - in /apr/apr/branches/1.7.x: ./ include/arch/beos/ include/arch/netware/ include/arch/unix/ include/arch/win32/ memory/unix/ threadproc/beos/ threadproc/netware/ threadproc/os2/ threadproc/unix/ threadproc/win32/
Posted by Yann Ylavic <yl...@gmail.com>.
On Wed, Jun 29, 2022 at 4:47 PM William A Rowe Jr <wr...@rowe-clan.net> wrote:
>
> 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.
AFAICT, this change only touches internal/opaque structures..
Are you talking about apr__pool_unmanage()? If so it was later removed
in r1897210.
About this change particularly, it fixes serious bugs in APR threads,
just run the tests with r1884102 (before this commit), APR_POOL_DEBUG
(to ease reproduction) and under ASan (Address Sanitizer,
-fsanitize=address), or maybe it was httpd's tests.
I can't remember the details and travis does not seem to keep builds
history that far (Dec 2020) so I can't point you to the stack traces /
reports.
I do remember that period where we started to run ASan builds though
(in httpd and later APR), this led to a series of fixes in APR,
including this one..
>
> Anyone using a build crossing runtimes between apr 1.7.0 and 1.7.1 may
> experience
> unintended side-effects.
Anyone running apr-1.7.x without these fixes and using threads is kind
of in UB land.
>
> 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.
Really? We don't install these headers, and they are opaque precisely
so that we can change them without breaking users.
If users copy private headers for their use cases, shouldn't they take
the burden of maintaining their codebase (depending on that) when
those headers change? This can't be on us..
By making such impracticable policies for revision changes (semver
sense), we are going to simply not fix anything but trivial changes
without bumping minor, and no one would/should care about the old
(though still "maintained") minor versions. What's the point of
releasing 1.7.1 then? Let's go with 1.8.0 only, simpler (less
maintenance) and better for everybody..
Regards;
Yann.
Re: svn commit: r1884103 - in /apr/apr/branches/1.7.x: ./ include/arch/beos/ include/arch/netware/ include/arch/unix/ include/arch/win32/ memory/unix/ threadproc/beos/ threadproc/netware/ threadproc/os2/ threadproc/unix/ threadproc/win32/
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
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 <yl...@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;
> }
>
>