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 2022/01/18 17:43:10 UTC

svn commit: r1897179 - in /apr/apr/trunk: memory/unix/apr_pools.c threadproc/beos/thread.c threadproc/netware/thread.c threadproc/os2/thread.c threadproc/unix/thread.c threadproc/win32/thread.c

Author: ylavic
Date: Tue Jan 18 17:43:10 2022
New Revision: 1897179

URL: http://svn.apache.org/viewvc?rev=1897179&view=rev
Log:
apr_thread: Follow up to r1884078: Unmanaged pools for attached threads too.

r1884078 fixed lifetime issues with detached threads by using unmanaged pool
destroyed by the thread itself on exit, with no binding to the parent pool.

This commit makes use of unmanaged pools for attached threads too, they needed
their own allocator anyway due to apr_thread_detach() being callable anytime
later. apr__pool_unmanage() was a hack to detach a subpool from its parent, but
if a subpool needs its own allocator for this to work correctly there is no
point in creating a subpool for threads (no memory reuse on destroy for short
living threads for instance).

Since an attached thread has its own lifetime now, apr_thread_join() must be
called to free its resources/pool, though it's no different than before when
destroying the parent pool was UB if the thread was still running (i.e.  not
joined yet).

Let's acknoledge that threads want no binding with the pool passed to them at
creation time, besides the abort_fn which they can steal :)


Modified:
    apr/apr/trunk/memory/unix/apr_pools.c
    apr/apr/trunk/threadproc/beos/thread.c
    apr/apr/trunk/threadproc/netware/thread.c
    apr/apr/trunk/threadproc/os2/thread.c
    apr/apr/trunk/threadproc/unix/thread.c
    apr/apr/trunk/threadproc/win32/thread.c

Modified: apr/apr/trunk/memory/unix/apr_pools.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/memory/unix/apr_pools.c?rev=1897179&r1=1897178&r2=1897179&view=diff
==============================================================================
--- apr/apr/trunk/memory/unix/apr_pools.c (original)
+++ apr/apr/trunk/memory/unix/apr_pools.c Tue Jan 18 17:43:10 2022
@@ -2343,46 +2343,6 @@ APR_DECLARE(void) apr_pool_lock(apr_pool
 
 #endif /* !APR_POOL_DEBUG */
 
-apr_status_t apr__pool_unmanage(apr_pool_t *pool);
-
-/* 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_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/trunk/threadproc/beos/thread.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/threadproc/beos/thread.c?rev=1897179&r1=1897178&r2=1897179&view=diff
==============================================================================
--- apr/apr/trunk/threadproc/beos/thread.c (original)
+++ apr/apr/trunk/threadproc/beos/thread.c Tue Jan 18 17:43:10 2022
@@ -17,9 +17,6 @@
 #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, 
@@ -84,63 +81,56 @@ APR_DECLARE(apr_status_t) apr_thread_cre
 {
     int32 temp;
     apr_status_t stat;
+    apr_allocator_t *allocator;
     
     (*new) = (apr_thread_t *)apr_pcalloc(pool, sizeof(apr_thread_t));
     if ((*new) == NULL) {
         return APR_ENOMEM;
     }
 
-    (*new)->data = data;
-    (*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);
-        }
+    /* The thread can be detached anytime (from the creation or later with
+     * apr_thread_detach), so it needs its own pool and allocator to not
+     * depend on a 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.
+     */
+    stat = apr_allocator_create(&allocator);
+    if (stat != APR_SUCCESS) {
+        return stat;
     }
+    stat = apr_pool_create_unmanaged_ex(&(*new)->pool,
+                                        apr_pool_abort_get(pool),
+                                        allocator);
     if (stat != APR_SUCCESS) {
+        apr_allocator_destroy(allocator);
         return stat;
     }
+    apr_allocator_owner_set(allocator, (*new)->pool);
 
-    /* First we create the new thread...*/
-	if (attr)
-	    temp = attr->attr;
-	else
-	    temp = B_NORMAL_PRIORITY;
+    (*new)->data = data;
+    (*new)->func = func;
+    (*new)->exitval = -1;
+    (*new)->detached = (attr && apr_threadattr_detach_get(attr) == APR_DETACH);
+
+    if (attr)
+        temp = attr->attr;
+    else
+        temp = B_NORMAL_PRIORITY;
 
+    /* First we create the new thread...*/
     (*new)->td = spawn_thread((thread_func)dummy_worker, 
                               "apr thread", 
                               temp, 
                               (*new));
 
     /* Now we try to run it...*/
-    if (resume_thread((*new)->td) == B_NO_ERROR) {
-        return APR_SUCCESS;
+    if (resume_thread((*new)->td) != B_NO_ERROR) {
+        stat = errno;
+        apr_pool_destroy((*new)->pool);
+        return stat;
     }
-    else {
-        return errno;
-    } 
+
+    return APR_SUCCESS;
 }
 
 APR_DECLARE(apr_os_thread_t) apr_os_thread_current(void)
@@ -195,8 +185,6 @@ APR_DECLARE(apr_status_t) apr_thread_det
     }
 
     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;
     }

Modified: apr/apr/trunk/threadproc/netware/thread.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/threadproc/netware/thread.c?rev=1897179&r1=1897178&r2=1897179&view=diff
==============================================================================
--- apr/apr/trunk/threadproc/netware/thread.c (original)
+++ apr/apr/trunk/threadproc/netware/thread.c Tue Jan 18 17:43:10 2022
@@ -21,9 +21,6 @@
 
 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)
 {
@@ -88,14 +85,48 @@ apr_status_t apr_thread_create(apr_threa
 {
     apr_status_t stat;
     unsigned long flags = NX_THR_BIND_CONTEXT;
-    char threadName[NX_MAX_OBJECT_NAME_LEN+1];
     size_t stack_size = APR_DEFAULT_STACK_SIZE;
+    apr_allocator_t *allocator;
+    
+    (*new) = (apr_thread_t *)apr_pcalloc(pool, sizeof(apr_thread_t));
+    if ((*new) == NULL) {
+        return APR_ENOMEM;
+    }
+
+    /* The thread can be detached anytime (from the creation or later with
+     * apr_thread_detach), so it needs its own pool and allocator to not
+     * depend on a 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.
+     */
+    stat = apr_allocator_create(&allocator);
+    if (stat != APR_SUCCESS) {
+        return stat;
+    }
+    stat = apr_pool_create_unmanaged_ex(&(*new)->pool,
+                                        apr_pool_abort_get(pool),
+                                        allocator);
+    if (stat != APR_SUCCESS) {
+        apr_allocator_destroy(allocator);
+        return stat;
+    }
+    apr_allocator_owner_set(allocator, (*new)->pool);
 
+    (*new)->data = data;
+    (*new)->func = func;
+    (*new)->exitval = -1;
+    (*new)->detached = (attr && apr_threadattr_detach_get(attr) == APR_DETACH);
     if (attr && attr->thread_name) {
-        strncpy (threadName, attr->thread_name, NX_MAX_OBJECT_NAME_LEN);
+        (*new)->thread_name = apr_pstrndup(pool, ttr->thread_name,
+                                           NX_MAX_OBJECT_NAME_LEN);
     }
     else {
-        sprintf(threadName, "APR_thread %04ld", ++thread_count);
+        (*new)->thread_name = apr_psprintf(pool, "APR_thread %04d",
+                                           ++thread_count);
+    }
+    if ((*new)->thread_name == NULL) {
+        apr_pool_destroy((*new)->pool);
+        return APR_ENOMEM;
     }
 
     /* An original stack size of 0 will allow NXCreateThread() to
@@ -107,45 +138,6 @@ apr_status_t apr_thread_create(apr_threa
         stack_size = attr->stack_size;
     }
     
-    (*new) = (apr_thread_t *)apr_pcalloc(pool, sizeof(apr_thread_t));
-
-    if ((*new) == NULL) {
-        return APR_ENOMEM;
-    }
-    
-    (*new)->data = data;
-    (*new)->func = func;
-    (*new)->thread_name = (char*)apr_pstrdup(pool, threadName);
-
-    (*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 && attr->detach) {
         flags |= NX_THR_DETACHED;
     }
@@ -158,19 +150,21 @@ apr_status_t apr_thread_create(apr_threa
         /* unsigned long flags */             NX_CTX_NORMAL,
         /* int *error */                      &stat);
                                                                            
-    stat = NXContextSetName(
+    (void) NXContextSetName(
         /* NXContext_t ctx */  (*new)->ctx,
-        /* const char *name */ threadName);
+        /* const char *name */ (*new)->thread_name);
 
     stat = NXThreadCreate(
         /* NXContext_t context */     (*new)->ctx,
         /* unsigned long flags */     flags,
         /* NXThreadId_t *thread_id */ &(*new)->td);
 
-    if (stat == 0)
-        return APR_SUCCESS;
+    if (stat) {
+        apr_pool_destroy((*new)->pool);
+        return stat;
+    }
         
-    return(stat); /* if error */    
+    return APR_SUCCESS;
 }
 
 apr_os_thread_t apr_os_thread_current()
@@ -223,8 +217,6 @@ apr_status_t apr_thread_detach(apr_threa
         return APR_EINVAL;
     }
 
-    /* Detach from the parent pool too */
-    apr__pool_unmanage(thd->pool);
     thd->detached = 1;
 
     return APR_SUCCESS;

Modified: apr/apr/trunk/threadproc/os2/thread.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/threadproc/os2/thread.c?rev=1897179&r1=1897178&r2=1897179&view=diff
==============================================================================
--- apr/apr/trunk/threadproc/os2/thread.c (original)
+++ apr/apr/trunk/threadproc/os2/thread.c Tue Jan 18 17:43:10 2022
@@ -24,8 +24,6 @@
 #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)
@@ -70,9 +68,9 @@ APR_DECLARE(apr_status_t) apr_threadattr
     return APR_ENOTIMPL;
 }
 
-static void apr_thread_begin(void *arg)
+static void dummy_worker(void *opaque)
 {
-  apr_thread_t *thread = (apr_thread_t *)arg;
+  apr_thread_t *thread = (apr_thread_t *)opaque;
   apr_pool_owner_set(thread->pool, 0);
   thread->exitval = thread->func(thread, thread->data);
   if (thd->attr->attr & APR_THREADATTR_DETACHED) {
@@ -88,70 +86,51 @@ APR_DECLARE(apr_status_t) apr_thread_cre
 {
     apr_status_t stat;
     apr_thread_t *thread;
- 
-    thread = (apr_thread_t *)apr_pcalloc(pool, sizeof(apr_thread_t));
-    *new = thread;
-
+    apr_allocator_t *allocator;
+    
+    *new = thread = (apr_thread_t *)apr_pcalloc(pool, sizeof(apr_thread_t));
     if (thread == NULL) {
         return APR_ENOMEM;
     }
 
-    thread->attr = attr;
-    thread->func = func;
-    thread->data = data;
-
-    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);
-        }
+    /* The thread can be detached anytime (from the creation or later with
+     * apr_thread_detach), so it needs its own pool and allocator to not
+     * depend on a 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.
+     */
+    stat = apr_allocator_create(&allocator);
+    if (stat != APR_SUCCESS) {
+        return stat;
     }
+    stat = apr_pool_create_unmanaged_ex(&thread->pool,
+                                        apr_pool_abort_get(pool),
+                                        allocator);
     if (stat != APR_SUCCESS) {
+        apr_allocator_destroy(allocator);
         return stat;
     }
+    apr_allocator_owner_set(allocator, thread->pool);
 
+    thread->func = func;
+    thread->data = data;
     if (attr == NULL) {
-        stat = apr_threadattr_create(&thread->attr, thread->pool);
-        
+        stat = apr_threadattr_create(&attr, thread->pool);
         if (stat != APR_SUCCESS) {
+            apr_pool_destroy(thread->pool);
             return stat;
         }
     }
+    thread->attr = attr;
 
-    thread->tid = _beginthread(apr_thread_begin, NULL, 
+    thread->tid = _beginthread(dummy_worker, NULL, 
                                thread->attr->stacksize > 0 ?
                                thread->attr->stacksize : APR_THREAD_STACKSIZE,
                                thread);
-        
     if (thread->tid < 0) {
-        return errno;
+        stat = errno;
+        apr_pool_destroy(thread->pool);
+        return stat;
     }
 
     return APR_SUCCESS;
@@ -208,8 +187,6 @@ APR_DECLARE(apr_status_t) apr_thread_det
         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/trunk/threadproc/unix/thread.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/threadproc/unix/thread.c?rev=1897179&r1=1897178&r2=1897179&view=diff
==============================================================================
--- apr/apr/trunk/threadproc/unix/thread.c (original)
+++ apr/apr/trunk/threadproc/unix/thread.c Tue Jan 18 17:43:10 2022
@@ -22,9 +22,6 @@
 
 #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)
 {
@@ -160,49 +157,39 @@ APR_DECLARE(apr_status_t) apr_thread_cre
 {
     apr_status_t stat;
     pthread_attr_t *temp;
-
+    apr_allocator_t *allocator;
+    
     (*new) = (apr_thread_t *)apr_pcalloc(pool, sizeof(apr_thread_t));
-
     if ((*new) == NULL) {
         return APR_ENOMEM;
     }
 
-    (*new)->td = (pthread_t *)apr_pcalloc(pool, sizeof(pthread_t));
-
-    if ((*new)->td == NULL) {
-        return APR_ENOMEM;
+    /* The thread can be detached anytime (from the creation or later with
+     * apr_thread_detach), so it needs its own pool and allocator to not
+     * depend on a 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.
+     */
+    stat = apr_allocator_create(&allocator);
+    if (stat != APR_SUCCESS) {
+        return stat;
+    }
+    stat = apr_pool_create_unmanaged_ex(&(*new)->pool,
+                                        apr_pool_abort_get(pool),
+                                        allocator);
+    if (stat != APR_SUCCESS) {
+        apr_allocator_destroy(allocator);
+        return stat;
     }
+    apr_allocator_owner_set(allocator, (*new)->pool);
 
     (*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;
+    (*new)->td = (pthread_t *)apr_pcalloc(pool, sizeof(pthread_t));
+    if ((*new)->td == NULL) {
+        apr_pool_destroy((*new)->pool);
+        return APR_ENOMEM;
     }
 
     if (attr)
@@ -210,16 +197,15 @@ APR_DECLARE(apr_status_t) apr_thread_cre
     else
         temp = NULL;
 
-    if ((stat = pthread_create((*new)->td, temp, dummy_worker, (*new))) == 0) {
-        return APR_SUCCESS;
-    }
-    else {
+    if ((stat = pthread_create((*new)->td, temp, dummy_worker, (*new)))) {
 #ifdef HAVE_ZOS_PTHREADS
         stat = errno;
 #endif
-
+        apr_pool_destroy((*new)->pool);
         return stat;
     }
+
+    return APR_SUCCESS;
 }
 
 APR_DECLARE(apr_os_thread_t) apr_os_thread_current(void)
@@ -280,8 +266,6 @@ APR_DECLARE(apr_status_t) apr_thread_det
 #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/trunk/threadproc/win32/thread.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/threadproc/win32/thread.c?rev=1897179&r1=1897178&r2=1897179&view=diff
==============================================================================
--- apr/apr/trunk/threadproc/win32/thread.c (original)
+++ apr/apr/trunk/threadproc/win32/thread.c Tue Jan 18 17:43:10 2022
@@ -28,9 +28,6 @@
 /* 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)
 {
@@ -95,45 +92,36 @@ APR_DECLARE(apr_status_t) apr_thread_cre
                                             void *data, apr_pool_t *pool)
 {
     apr_status_t stat;
-	unsigned temp;
+    unsigned temp;
     HANDLE handle;
-
+    apr_allocator_t *allocator;
+    
     (*new) = (apr_thread_t *)apr_pcalloc(pool, sizeof(apr_thread_t));
-
     if ((*new) == NULL) {
         return APR_ENOMEM;
     }
 
-    (*new)->data = data;
-    (*new)->func = func;
-
-    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);
-        }
+    /* The thread can be detached anytime (from the creation or later with
+     * apr_thread_detach), so it needs its own pool and allocator to not
+     * depend on a 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.
+     */
+    stat = apr_allocator_create(&allocator);
+    if (stat != APR_SUCCESS) {
+        return stat;
     }
+    stat = apr_pool_create_unmanaged_ex(&(*new)->pool,
+                                        apr_pool_abort_get(pool),
+                                        allocator);
     if (stat != APR_SUCCESS) {
+        apr_allocator_destroy(allocator);
         return stat;
     }
+    apr_allocator_owner_set(allocator, (*new)->pool);
+
+    (*new)->data = data;
+    (*new)->func = func;
 
     /* Use 0 for default Thread Stack Size, because that will
      * default the stack to the same size as the calling thread.
@@ -142,13 +130,16 @@ APR_DECLARE(apr_status_t) apr_thread_cre
                         (DWORD) (attr ? attr->stacksize : 0),
                         (unsigned int (APR_THREAD_FUNC *)(void *))dummy_worker,
                         (*new), 0, &temp)) == 0) {
-        return APR_FROM_OS_ERROR(_doserrno);
+        stat = APR_FROM_OS_ERROR(_doserrno);
+        apr_pool_destroy((*new)->pool);
+        return stat;
     }
     if (attr && attr->detach) {
         CloseHandle(handle);
     }
-    else
+    else {
         (*new)->td = handle;
+    }
 
     return APR_SUCCESS;
 }
@@ -201,8 +192,6 @@ APR_DECLARE(apr_status_t) apr_thread_det
     }
 
     if (CloseHandle(thd->td)) {
-        /* Detach from the parent pool too */
-        apr__pool_unmanage(thd->pool);
         thd->td = NULL;
         return APR_SUCCESS;
     }