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


git-svn-id: https://svn.apache.org/repos/asf/apr/apr/branches/1.7.x@1884103 13f79535-47bb-0310-9956-ffa450edef68

10 files changed, 321 insertions, 37 deletions

diff --git a/include/arch/beos/apr_arch_threadproc.h b/include/arch/beos/apr_arch_threadproc.h
index 13de05363..b7db0a300 100644
--- a/include/arch/beos/apr_arch_threadproc.h
+++ b/include/arch/beos/apr_arch_threadproc.h
@@ -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 {
diff --git a/include/arch/netware/apr_arch_threadproc.h b/include/arch/netware/apr_arch_threadproc.h
index 2fee2c00e..ce217aaba 100644
--- a/include/arch/netware/apr_arch_threadproc.h
+++ b/include/arch/netware/apr_arch_threadproc.h
@@ -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 {
diff --git a/include/arch/unix/apr_arch_threadproc.h b/include/arch/unix/apr_arch_threadproc.h
index 7a3b3c092..adeb51c8a 100644
--- a/include/arch/unix/apr_arch_threadproc.h
+++ b/include/arch/unix/apr_arch_threadproc.h
@@ -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 {
diff --git a/include/arch/win32/apr_arch_threadproc.h b/include/arch/win32/apr_arch_threadproc.h
index d3ce9c518..b39fdbbea 100644
--- a/include/arch/win32/apr_arch_threadproc.h
+++ b/include/arch/win32/apr_arch_threadproc.h
@@ -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 {
diff --git a/memory/unix/apr_pools.c b/memory/unix/apr_pools.c
index 6a5feba38..50badc4b0 100644
--- a/memory/unix/apr_pools.c
+++ b/memory/unix/apr_pools.c
@@ -2324,6 +2324,45 @@ APR_DECLARE(void) apr_pool_lock(apr_pool_t *pool, int flag)
 
 #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 ()
 {
diff --git a/threadproc/beos/thread.c b/threadproc/beos/thread.c
index 8d8383942..1d79c32bf 100644
--- a/threadproc/beos/thread.c
+++ b/threadproc/beos/thread.c
@@ -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_guardsize_set(apr_threadattr_t *attr,
 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_create(apr_thread_t **new, apr_threadattr_t
     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_create(apr_thread_t **new, apr_threadattr_t
     (*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 tid1, apr_os_thread_t tid2)
 
 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_exit(apr_thread_t *thd, apr_status_t retval
 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_join(apr_status_t *retval, apr_thread_t *th
          */
         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_join(apr_status_t *retval, apr_thread_t *th
 
 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 {
diff --git a/threadproc/netware/thread.c b/threadproc/netware/thread.c
index f98366855..ac9331db9 100644
--- a/threadproc/netware/thread.c
+++ b/threadproc/netware/thread.c
@@ -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_guardsize_set(apr_threadattr_t *attr,
 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_thread_t **new,
         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_thread_t **new,
     (*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_t *thd,
                              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_t *retval,
     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_t *retval,
 
 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;
 }
 
diff --git a/threadproc/os2/thread.c b/threadproc/os2/thread.c
index 00ec4eb5c..441de42bb 100644
--- a/threadproc/os2/thread.c
+++ b/threadproc/os2/thread.c
@@ -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_create(apr_thread_t **new, apr_threadattr_t
     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_create(apr_thread_t **new, apr_threadattr_t
     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_thread_current()
 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_join(apr_status_t *retval, apr_thread_t *th
         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;
 }
 
diff --git a/threadproc/unix/thread.c b/threadproc/unix/thread.c
index 6d060be55..b16c6f9e2 100644
--- a/threadproc/unix/thread.c
+++ b/threadproc/unix/thread.c
@@ -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_guardsize_set(apr_threadattr_t *attr,
 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_create(apr_thread_t **new,
     (*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_exit(apr_thread_t *thd,
                                           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_join(apr_status_t *retval,
     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_detach(apr_thread_t *thd)
 {
     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;
     }
diff --git a/threadproc/win32/thread.c b/threadproc/win32/thread.c
index 25034571e..7fa71784d 100644
--- a/threadproc/win32/thread.c
+++ b/threadproc/win32/thread.c
@@ -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_guardsize_set(apr_threadattr_t *attr,
 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_create(apr_thread_t **new,
 	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_create(apr_thread_t **new,
 
     (*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_create(apr_thread_t **new,
 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_join(apr_status_t *retval,
                                           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;
     }