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

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 {

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 {

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 {

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 {

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 ()
 {

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 {

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;
 }