diff options
author | Yann Ylavic <ylavic@apache.org> | 2022-01-19 11:27:35 +0000 |
---|---|---|
committer | Yann Ylavic <ylavic@apache.org> | 2022-01-19 11:27:35 +0000 |
commit | ea8d9ad623550b3ca466e272c8738002d2e0b088 (patch) | |
tree | 5a9aaee65ea66a401668f2169e88b15744ddd173 | |
parent | 7a52a7c060ebb42f6e83505755b49bc1e1acd183 (diff) | |
download | apr-ea8d9ad623550b3ca466e272c8738002d2e0b088.tar.gz |
apr_thread: Allocate the apr_thread_t struct on the thread's pool.
apr_thread_create() was allocating the created apr_thread_t on the given pool,
which caused e.g. short-living threads to leak memory on that pool without a
way to clear it (while some threads are still running).
Change this by allocating the apr_thread_t on the thread's pool itself, which
is safe in the implementations of all archs because none uses the apr_thread_t
after the thread exits, and it's safe for the users provided they don't use the
apr_thread_t for detached threads or for attached threads after the call to
apr_thread_join(). These are hardly new requirements though.
git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1897197 13f79535-47bb-0310-9956-ffa450edef68
-rw-r--r-- | threadproc/beos/thread.c | 24 | ||||
-rw-r--r-- | threadproc/netware/thread.c | 26 | ||||
-rw-r--r-- | threadproc/os2/thread.c | 41 | ||||
-rw-r--r-- | threadproc/unix/thread.c | 24 | ||||
-rw-r--r-- | threadproc/win32/thread.c | 20 |
5 files changed, 71 insertions, 64 deletions
diff --git a/threadproc/beos/thread.c b/threadproc/beos/thread.c index 5287ded8d..8dd95ea8b 100644 --- a/threadproc/beos/thread.c +++ b/threadproc/beos/thread.c @@ -82,12 +82,8 @@ 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; + apr_pool_t *p; - (*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 @@ -98,15 +94,21 @@ APR_DECLARE(apr_status_t) apr_thread_create(apr_thread_t **new, apr_threadattr_t if (stat != APR_SUCCESS) { return stat; } - stat = apr_pool_create_unmanaged_ex(&(*new)->pool, - apr_pool_abort_get(pool), + stat = apr_pool_create_unmanaged_ex(&p, apr_pool_abort_get(pool), allocator); if (stat != APR_SUCCESS) { apr_allocator_destroy(allocator); return stat; } - apr_allocator_owner_set(allocator, (*new)->pool); + apr_allocator_owner_set(allocator, p); + + (*new) = (apr_thread_t *)apr_pcalloc(p, sizeof(apr_thread_t)); + if ((*new) == NULL) { + apr_pool_destroy(p); + return APR_ENOMEM; + } + (*new)->pool = p; (*new)->data = data; (*new)->func = func; (*new)->exitval = -1; @@ -119,14 +121,12 @@ APR_DECLARE(apr_status_t) apr_thread_create(apr_thread_t **new, apr_threadattr_t /* First we create the new thread...*/ (*new)->td = spawn_thread((thread_func)dummy_worker, - "apr thread", - temp, - (*new)); + "apr thread", temp, (*new)); /* Now we try to run it...*/ if (resume_thread((*new)->td) != B_NO_ERROR) { stat = errno; - apr_pool_destroy((*new)->pool); + apr_pool_destroy(p); return stat; } diff --git a/threadproc/netware/thread.c b/threadproc/netware/thread.c index 34a5691f5..a661ef886 100644 --- a/threadproc/netware/thread.c +++ b/threadproc/netware/thread.c @@ -87,12 +87,8 @@ apr_status_t apr_thread_create(apr_thread_t **new, unsigned long flags = NX_THR_BIND_CONTEXT; size_t stack_size = APR_DEFAULT_STACK_SIZE; apr_allocator_t *allocator; + apr_pool_t *p; - (*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 @@ -103,29 +99,35 @@ apr_status_t apr_thread_create(apr_thread_t **new, if (stat != APR_SUCCESS) { return stat; } - stat = apr_pool_create_unmanaged_ex(&(*new)->pool, - apr_pool_abort_get(pool), + stat = apr_pool_create_unmanaged_ex(&p, apr_pool_abort_get(pool), allocator); if (stat != APR_SUCCESS) { apr_allocator_destroy(allocator); return stat; } - apr_allocator_owner_set(allocator, (*new)->pool); + apr_allocator_owner_set(allocator, p); + + (*new) = (apr_thread_t *)apr_pcalloc(p, sizeof(apr_thread_t)); + if ((*new) == NULL) { + apr_pool_destroy(p); + return APR_ENOMEM; + } + (*new)->pool = p; (*new)->data = data; (*new)->func = func; (*new)->exitval = -1; (*new)->detached = (attr && apr_threadattr_detach_get(attr) == APR_DETACH); if (attr && attr->thread_name) { - (*new)->thread_name = apr_pstrndup(pool, ttr->thread_name, + (*new)->thread_name = apr_pstrndup(p, ttr->thread_name, NX_MAX_OBJECT_NAME_LEN); } else { - (*new)->thread_name = apr_psprintf(pool, "APR_thread %04d", + (*new)->thread_name = apr_psprintf(p, "APR_thread %04d", ++thread_count); } if ((*new)->thread_name == NULL) { - apr_pool_destroy((*new)->pool); + apr_pool_destroy(p); return APR_ENOMEM; } @@ -160,7 +162,7 @@ apr_status_t apr_thread_create(apr_thread_t **new, /* NXThreadId_t *thread_id */ &(*new)->td); if (stat) { - apr_pool_destroy((*new)->pool); + apr_pool_destroy(p); return stat; } diff --git a/threadproc/os2/thread.c b/threadproc/os2/thread.c index 1e164ebd6..2891f0577 100644 --- a/threadproc/os2/thread.c +++ b/threadproc/os2/thread.c @@ -85,14 +85,9 @@ APR_DECLARE(apr_status_t) apr_thread_create(apr_thread_t **new, apr_threadattr_t apr_pool_t *pool) { apr_status_t stat; - apr_thread_t *thread; apr_allocator_t *allocator; + apr_pool_t *p; - *new = thread = (apr_thread_t *)apr_pcalloc(pool, sizeof(apr_thread_t)); - if (thread == 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 @@ -103,33 +98,39 @@ APR_DECLARE(apr_status_t) apr_thread_create(apr_thread_t **new, apr_threadattr_t if (stat != APR_SUCCESS) { return stat; } - stat = apr_pool_create_unmanaged_ex(&thread->pool, - apr_pool_abort_get(pool), + stat = apr_pool_create_unmanaged_ex(&p, apr_pool_abort_get(pool), allocator); if (stat != APR_SUCCESS) { apr_allocator_destroy(allocator); return stat; } - apr_allocator_owner_set(allocator, thread->pool); + apr_allocator_owner_set(allocator, p); + + (*new) = (apr_thread_t *)apr_pcalloc(p, sizeof(apr_thread_t)); + if ((*new) == NULL) { + apr_pool_destroy(p); + return APR_ENOMEM; + } - thread->func = func; - thread->data = data; + (*new)->pool = p; + (*new)->func = func; + (*new)->data = data; if (attr == NULL) { - stat = apr_threadattr_create(&attr, thread->pool); + stat = apr_threadattr_create(&attr, p); if (stat != APR_SUCCESS) { - apr_pool_destroy(thread->pool); + apr_pool_destroy(p); return stat; } } - thread->attr = attr; + (*new)->attr = attr; - thread->tid = _beginthread(dummy_worker, NULL, - thread->attr->stacksize > 0 ? - thread->attr->stacksize : APR_THREAD_STACKSIZE, - thread); - if (thread->tid < 0) { + (*new)->tid = _beginthread(dummy_worker, NULL, + (*new)->attr->stacksize > 0 ? + (*new)->attr->stacksize : APR_THREAD_STACKSIZE, + (*new)); + if ((*new)->tid < 0) { stat = errno; - apr_pool_destroy(thread->pool); + apr_pool_destroy(p); return stat; } diff --git a/threadproc/unix/thread.c b/threadproc/unix/thread.c index a91d8cd31..3b6ef2809 100644 --- a/threadproc/unix/thread.c +++ b/threadproc/unix/thread.c @@ -158,12 +158,8 @@ APR_DECLARE(apr_status_t) apr_thread_create(apr_thread_t **new, apr_status_t stat; pthread_attr_t *temp; apr_allocator_t *allocator; + apr_pool_t *p; - (*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 @@ -174,21 +170,27 @@ APR_DECLARE(apr_status_t) apr_thread_create(apr_thread_t **new, if (stat != APR_SUCCESS) { return stat; } - stat = apr_pool_create_unmanaged_ex(&(*new)->pool, - apr_pool_abort_get(pool), + stat = apr_pool_create_unmanaged_ex(&p, apr_pool_abort_get(pool), allocator); if (stat != APR_SUCCESS) { apr_allocator_destroy(allocator); return stat; } - apr_allocator_owner_set(allocator, (*new)->pool); + apr_allocator_owner_set(allocator, p); + + (*new) = (apr_thread_t *)apr_pcalloc(p, sizeof(apr_thread_t)); + if ((*new) == NULL) { + apr_pool_destroy(p); + return APR_ENOMEM; + } + (*new)->pool = p; (*new)->data = data; (*new)->func = func; (*new)->detached = (attr && apr_threadattr_detach_get(attr) == APR_DETACH); - (*new)->td = (pthread_t *)apr_pcalloc(pool, sizeof(pthread_t)); + (*new)->td = (pthread_t *)apr_pcalloc(p, sizeof(pthread_t)); if ((*new)->td == NULL) { - apr_pool_destroy((*new)->pool); + apr_pool_destroy(p); return APR_ENOMEM; } @@ -201,7 +203,7 @@ APR_DECLARE(apr_status_t) apr_thread_create(apr_thread_t **new, #ifdef HAVE_ZOS_PTHREADS stat = errno; #endif - apr_pool_destroy((*new)->pool); + apr_pool_destroy(p); return stat; } diff --git a/threadproc/win32/thread.c b/threadproc/win32/thread.c index ae0c40794..02956c59f 100644 --- a/threadproc/win32/thread.c +++ b/threadproc/win32/thread.c @@ -95,12 +95,8 @@ APR_DECLARE(apr_status_t) apr_thread_create(apr_thread_t **new, unsigned temp; HANDLE handle; apr_allocator_t *allocator; + apr_pool_t *p; - (*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 @@ -111,15 +107,21 @@ APR_DECLARE(apr_status_t) apr_thread_create(apr_thread_t **new, if (stat != APR_SUCCESS) { return stat; } - stat = apr_pool_create_unmanaged_ex(&(*new)->pool, - apr_pool_abort_get(pool), + stat = apr_pool_create_unmanaged_ex(&p, apr_pool_abort_get(pool), allocator); if (stat != APR_SUCCESS) { apr_allocator_destroy(allocator); return stat; } - apr_allocator_owner_set(allocator, (*new)->pool); + apr_allocator_owner_set(allocator, p); + + (*new) = (apr_thread_t *)apr_pcalloc(p, sizeof(apr_thread_t)); + if ((*new) == NULL) { + apr_pool_destroy(p); + return APR_ENOMEM; + } + (*new)->pool = p; (*new)->data = data; (*new)->func = func; @@ -131,7 +133,7 @@ APR_DECLARE(apr_status_t) apr_thread_create(apr_thread_t **new, (unsigned int (APR_THREAD_FUNC *)(void *))dummy_worker, (*new), 0, &temp)) == 0) { stat = APR_FROM_OS_ERROR(_doserrno); - apr_pool_destroy((*new)->pool); + apr_pool_destroy(p); return stat; } if (attr && attr->detach) { |