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 2022/01/19 22:32:44 UTC
svn commit: r1897221 - in /apr/apr/branches/1.8.x: ./ threadproc/beos/thread.c threadproc/netware/thread.c threadproc/os2/thread.c threadproc/unix/thread.c threadproc/win32/thread.c
Author: ylavic
Date: Wed Jan 19 22:32:44 2022
New Revision: 1897221
URL: http://svn.apache.org/viewvc?rev=1897221&view=rev
Log:
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.
apr_thread: Follow up to r1897197: Safer apr_thread_join().
Make sure apr_thread_join() behaves correctly w.r.t. the returned value
and pool destroy for all archs.
Merge r1897197, r1897198 from trunk.
Submitted by: ylavic
Modified:
apr/apr/branches/1.8.x/ (props changed)
apr/apr/branches/1.8.x/threadproc/beos/thread.c
apr/apr/branches/1.8.x/threadproc/netware/thread.c
apr/apr/branches/1.8.x/threadproc/os2/thread.c
apr/apr/branches/1.8.x/threadproc/unix/thread.c
apr/apr/branches/1.8.x/threadproc/win32/thread.c
Propchange: apr/apr/branches/1.8.x/
------------------------------------------------------------------------------
Merged /apr/apr/trunk:r1897197-1897198
Modified: apr/apr/branches/1.8.x/threadproc/beos/thread.c
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.8.x/threadproc/beos/thread.c?rev=1897221&r1=1897220&r2=1897221&view=diff
==============================================================================
--- apr/apr/branches/1.8.x/threadproc/beos/thread.c (original)
+++ apr/apr/branches/1.8.x/threadproc/beos/thread.c Wed Jan 19 22:32:44 2022
@@ -81,12 +81,8 @@ APR_DECLARE(apr_status_t) apr_thread_cre
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
@@ -97,15 +93,21 @@ APR_DECLARE(apr_status_t) apr_thread_cre
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;
@@ -118,14 +120,12 @@ APR_DECLARE(apr_status_t) apr_thread_cre
/* 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;
}
@@ -162,21 +162,19 @@ APR_DECLARE(apr_status_t) apr_thread_joi
}
ret = wait_for_thread(thd->td, &rv);
- if (ret == B_NO_ERROR) {
- *retval = rv;
- return APR_SUCCESS;
- }
- else {
+ if (ret != B_NO_ERROR) {
/* if we've missed the thread's death, did we set an exit value prior
* to it's demise? If we did return that.
*/
- if (thd->exitval != -1) {
- *retval = thd->exitval;
- apr_pool_destroy(thd->pool);
- return APR_SUCCESS;
- } else
- return ret;
+ if (thd->exitval == -1) {
+ return errno;
+ }
+ rv = thd->exitval;
}
+
+ *retval = rv;
+ apr_pool_destroy(thd->pool);
+ return APR_SUCCESS;
}
APR_DECLARE(apr_status_t) apr_thread_detach(apr_thread_t *thd)
Modified: apr/apr/branches/1.8.x/threadproc/netware/thread.c
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.8.x/threadproc/netware/thread.c?rev=1897221&r1=1897220&r2=1897221&view=diff
==============================================================================
--- apr/apr/branches/1.8.x/threadproc/netware/thread.c (original)
+++ apr/apr/branches/1.8.x/threadproc/netware/thread.c Wed Jan 19 22:32:44 2022
@@ -86,12 +86,8 @@ apr_status_t apr_thread_create(apr_threa
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
@@ -102,29 +98,35 @@ apr_status_t apr_thread_create(apr_threa
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;
}
@@ -159,7 +161,7 @@ apr_status_t apr_thread_create(apr_threa
/* NXThreadId_t *thread_id */ &(*new)->td);
if (stat) {
- apr_pool_destroy((*new)->pool);
+ apr_pool_destroy(p);
return stat;
}
Modified: apr/apr/branches/1.8.x/threadproc/os2/thread.c
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.8.x/threadproc/os2/thread.c?rev=1897221&r1=1897220&r2=1897221&view=diff
==============================================================================
--- apr/apr/branches/1.8.x/threadproc/os2/thread.c (original)
+++ apr/apr/branches/1.8.x/threadproc/os2/thread.c Wed Jan 19 22:32:44 2022
@@ -84,14 +84,9 @@ APR_DECLARE(apr_status_t) apr_thread_cre
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
@@ -102,33 +97,39 @@ APR_DECLARE(apr_status_t) apr_thread_cre
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;
}
Modified: apr/apr/branches/1.8.x/threadproc/unix/thread.c
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.8.x/threadproc/unix/thread.c?rev=1897221&r1=1897220&r2=1897221&view=diff
==============================================================================
--- apr/apr/branches/1.8.x/threadproc/unix/thread.c (original)
+++ apr/apr/branches/1.8.x/threadproc/unix/thread.c Wed Jan 19 22:32:44 2022
@@ -157,12 +157,8 @@ APR_DECLARE(apr_status_t) apr_thread_cre
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
@@ -173,21 +169,27 @@ APR_DECLARE(apr_status_t) apr_thread_cre
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;
}
@@ -200,7 +202,7 @@ APR_DECLARE(apr_status_t) apr_thread_cre
#ifdef HAVE_ZOS_PTHREADS
stat = errno;
#endif
- apr_pool_destroy((*new)->pool);
+ apr_pool_destroy(p);
return stat;
}
@@ -233,24 +235,22 @@ APR_DECLARE(apr_status_t) apr_thread_joi
apr_thread_t *thd)
{
apr_status_t stat;
- apr_status_t *thread_stat;
+ void *thread_stat;
if (thd->detached) {
return APR_EINVAL;
}
- if ((stat = pthread_join(*thd->td,(void *)&thread_stat)) == 0) {
- *retval = thd->exitval;
- apr_pool_destroy(thd->pool);
- return APR_SUCCESS;
- }
- else {
+ if ((stat = pthread_join(*thd->td, &thread_stat))) {
#ifdef HAVE_ZOS_PTHREADS
stat = errno;
#endif
-
return stat;
}
+
+ *retval = thd->exitval;
+ apr_pool_destroy(thd->pool);
+ return APR_SUCCESS;
}
APR_DECLARE(apr_status_t) apr_thread_detach(apr_thread_t *thd)
Modified: apr/apr/branches/1.8.x/threadproc/win32/thread.c
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.8.x/threadproc/win32/thread.c?rev=1897221&r1=1897220&r2=1897221&view=diff
==============================================================================
--- apr/apr/branches/1.8.x/threadproc/win32/thread.c (original)
+++ apr/apr/branches/1.8.x/threadproc/win32/thread.c Wed Jan 19 22:32:44 2022
@@ -94,12 +94,8 @@ APR_DECLARE(apr_status_t) apr_thread_cre
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
@@ -110,15 +106,21 @@ APR_DECLARE(apr_status_t) apr_thread_cre
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_cre
(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;
}
#else
@@ -195,7 +197,6 @@ APR_DECLARE(apr_status_t) apr_thread_joi
if (rv == APR_SUCCESS) {
CloseHandle(thd->td);
apr_pool_destroy(thd->pool);
- thd->td = NULL;
}
return rv;