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.


git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1884078 13f79535-47bb-0310-9956-ffa450edef68

5 files changed, 165 insertions, 11 deletions

diff --git a/threadproc/beos/thread.c b/threadproc/beos/thread.c
index 61d7fd0d7..c1509da26 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, 
@@ -90,7 +93,35 @@ APR_DECLARE(apr_status_t) apr_thread_create(apr_thread_t **new, apr_threadattr_t
     (*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);
+        }
+    }
+    if (stat != APR_SUCCESS) {
+        return stat;
+    }
 
     /* First we create the new thread...*/
 	if (attr)
@@ -98,11 +129,6 @@ APR_DECLARE(apr_status_t) apr_thread_create(apr_thread_t **new, apr_threadattr_t
 	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, 
@@ -169,6 +195,8 @@ APR_DECLARE(apr_status_t) apr_thread_detach(apr_thread_t *thd)
     }
 
     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;
     }
diff --git a/threadproc/netware/thread.c b/threadproc/netware/thread.c
index 4864f5e57..0abbef34d 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)
 {
@@ -113,9 +116,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);
+
     (*new)->detached = (attr && apr_threadattr_detach_get(attr) == APR_DETACH);
-    
-    stat = apr_pool_create(&(*new)->pool, pool);
+    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;
     }
@@ -197,7 +223,10 @@ apr_status_t apr_thread_detach(apr_thread_t *thd)
         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 d7da4e128..3fd9bd75f 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));
@@ -95,8 +99,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;
     }
@@ -172,7 +208,10 @@ APR_DECLARE(apr_status_t) apr_thread_detach(apr_thread_t *thd)
         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 976a254fa..6399af18e 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)
 {
@@ -172,8 +175,32 @@ 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);
-    stat = apr_pool_create(&(*new)->pool, pool);
+    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;
     }
@@ -253,6 +280,8 @@ APR_DECLARE(apr_status_t) apr_thread_detach(apr_thread_t *thd)
 #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 757a9f0d5..b5a6f1722 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)
 {
@@ -103,7 +106,31 @@ APR_DECLARE(apr_status_t) apr_thread_create(apr_thread_t **new,
 
     (*new)->data = data;
     (*new)->func = func;
-    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;
     }
@@ -187,6 +214,8 @@ APR_DECLARE(apr_status_t) apr_thread_detach(apr_thread_t *thd)
     }
 
     if (CloseHandle(thd->td)) {
+        /* Detach from the parent pool too */
+        apr__pool_unmanage(thd->pool);
         thd->td = NULL;
         return APR_SUCCESS;
     }