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/03 22:59:01 UTC

svn commit: r1884078 - 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: Thu Dec  3 22:59:01 2020
New Revision: 1884078

URL: http://svn.apache.org/viewvc?rev=1884078&view=rev
Log:
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.

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=1884078&r1=1884077&r2=1884078&view=diff
==============================================================================
--- apr/apr/trunk/memory/unix/apr_pools.c (original)
+++ apr/apr/trunk/memory/unix/apr_pools.c Thu Dec  3 22:59:01 2020
@@ -2341,6 +2341,44 @@ 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_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=1884078&r1=1884077&r2=1884078&view=diff
==============================================================================
--- apr/apr/trunk/threadproc/beos/thread.c (original)
+++ apr/apr/trunk/threadproc/beos/thread.c Thu Dec  3 22:59:01 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, 
@@ -90,7 +93,35 @@ APR_DECLARE(apr_status_t) apr_thread_cre
     (*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_cre
 	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_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=1884078&r1=1884077&r2=1884078&view=diff
==============================================================================
--- apr/apr/trunk/threadproc/netware/thread.c (original)
+++ apr/apr/trunk/threadproc/netware/thread.c Thu Dec  3 22:59:01 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)
 {
@@ -113,9 +116,32 @@ apr_status_t apr_thread_create(apr_threa
     (*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_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=1884078&r1=1884077&r2=1884078&view=diff
==============================================================================
--- apr/apr/trunk/threadproc/os2/thread.c (original)
+++ apr/apr/trunk/threadproc/os2/thread.c Thu Dec  3 22:59:01 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));
@@ -95,8 +99,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;
     }
@@ -172,7 +208,10 @@ 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=1884078&r1=1884077&r2=1884078&view=diff
==============================================================================
--- apr/apr/trunk/threadproc/unix/thread.c (original)
+++ apr/apr/trunk/threadproc/unix/thread.c Thu Dec  3 22:59:01 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)
 {
@@ -172,8 +175,32 @@ APR_DECLARE(apr_status_t) apr_thread_cre
 
     (*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_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=1884078&r1=1884077&r2=1884078&view=diff
==============================================================================
--- apr/apr/trunk/threadproc/win32/thread.c (original)
+++ apr/apr/trunk/threadproc/win32/thread.c Thu Dec  3 22:59:01 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)
 {
@@ -103,7 +106,31 @@ APR_DECLARE(apr_status_t) apr_thread_cre
 
     (*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_det
     }
 
     if (CloseHandle(thd->td)) {
+        /* Detach from the parent pool too */
+        apr__pool_unmanage(thd->pool);
         thd->td = NULL;
         return APR_SUCCESS;
     }