summaryrefslogtreecommitdiff
path: root/threadproc
diff options
context:
space:
mode:
authorYann Ylavic <ylavic@apache.org>2022-01-18 17:43:10 +0000
committerYann Ylavic <ylavic@apache.org>2022-01-18 17:43:10 +0000
commit7a52a7c060ebb42f6e83505755b49bc1e1acd183 (patch)
treeacd476b370e292dce6ef1ef8f22690d99315ac4d /threadproc
parent84651d9f47a2ec0ba2f3921a79683540ecf98029 (diff)
downloadapr-7a52a7c060ebb42f6e83505755b49bc1e1acd183.tar.gz
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 :) git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1897179 13f79535-47bb-0310-9956-ffa450edef68
Diffstat (limited to 'threadproc')
-rw-r--r--threadproc/beos/thread.c74
-rw-r--r--threadproc/netware/thread.c96
-rw-r--r--threadproc/os2/thread.c79
-rw-r--r--threadproc/unix/thread.c70
-rw-r--r--threadproc/win32/thread.c61
5 files changed, 155 insertions, 225 deletions
diff --git a/threadproc/beos/thread.c b/threadproc/beos/thread.c
index c1509da26..5287ded8d 100644
--- a/threadproc/beos/thread.c
+++ b/threadproc/beos/thread.c
@@ -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_create(apr_thread_t **new, apr_threadattr_t
{
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_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 0abbef34d..34a5691f5 100644
--- a/threadproc/netware/thread.c
+++ b/threadproc/netware/thread.c
@@ -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_thread_t **new,
{
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_thread_t **new,
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_thread_t **new,
/* 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_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 3fd9bd75f..1e164ebd6 100644
--- a/threadproc/os2/thread.c
+++ b/threadproc/os2/thread.c
@@ -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_guardsize_set(apr_threadattr_t *attr,
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_create(apr_thread_t **new, apr_threadattr_t
{
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_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 6399af18e..a91d8cd31 100644
--- a/threadproc/unix/thread.c
+++ b/threadproc/unix/thread.c
@@ -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_create(apr_thread_t **new,
{
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_create(apr_thread_t **new,
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_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 3f9880b6a..ae0c40794 100644
--- a/threadproc/win32/thread.c
+++ b/threadproc/win32/thread.c
@@ -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_create(apr_thread_t **new,
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_create(apr_thread_t **new,
(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_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;
}