You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Aaron Bannert <aa...@ebuilt.com> on 2001/07/18 19:18:05 UTC
[PATCH] (#2) Fix apr_thread_exit(), change apr_thread_start_t, etc...
[Update to previous patch of the same name, with changes discussed on the
list.]
1) Fix to apr_thread_join() -- it wasn't setting the return value correctly.
2) Added a new test in test/testthread.c to verify that the above works.
Also modified the test to use the new worker_fn() prototype (see below):
3) Changed the prototype for apr_thread_start_t to pass in a new struct:
apr_thread_param_t, which contains the apr_thread_t and the data.
This gives us:
a) usability of apr_thread_exit()
b) future access to this thread's *child* pool
(through APR_POOL_IMPLEMENT_ACCESSOR()) from the apr_thread_t param.
c) the ability to add new parameters to the worker_fn() later in a way
that won't immediately break everywhere apr_thread_create() is called.
This was tested for unix with my Linux 2.4 box. I did my best to get
the code on beos, os2, and win32 as close to what I think it should be,
but I'll need someone from each platform to verify it.
enjoy,
-aaron
Index: include/apr_thread_proc.h
===================================================================
RCS file: /home/cvspublic/apr/include/apr_thread_proc.h,v
retrieving revision 1.65
diff -u -r1.65 apr_thread_proc.h
--- include/apr_thread_proc.h 2001/06/06 18:11:06 1.65
+++ include/apr_thread_proc.h 2001/07/18 17:03:33
@@ -125,7 +125,13 @@
typedef struct apr_other_child_rec_t apr_other_child_rec_t;
#endif /* APR_HAS_OTHER_CHILD */
-typedef void *(APR_THREAD_FUNC *apr_thread_start_t)(void *);
+struct apr_thread_param_t {
+ apr_thread_t *t;
+ void *data;
+};
+typedef struct apr_thread_param_t apr_thread_param_t;
+
+typedef void *(APR_THREAD_FUNC *apr_thread_start_t)(apr_thread_param_t *);
enum kill_conditions {
kill_never, /* process is never sent any signals */
Index: test/testthread.c
===================================================================
RCS file: /home/cvspublic/apr/test/testthread.c,v
retrieving revision 1.17
diff -u -r1.17 testthread.c
--- test/testthread.c 2001/03/14 15:56:44 1.17
+++ test/testthread.c 2001/07/18 17:03:33
@@ -72,57 +72,66 @@
}
#else /* !APR_HAS_THREADS */
-void * APR_THREAD_FUNC thread_func1(void *data);
-void * APR_THREAD_FUNC thread_func2(void *data);
-void * APR_THREAD_FUNC thread_func3(void *data);
-void * APR_THREAD_FUNC thread_func4(void *data);
+void * APR_THREAD_FUNC thread_func1(apr_thread_param_t *thrparm);
+void * APR_THREAD_FUNC thread_func2(apr_thread_param_t *thrparm);
+void * APR_THREAD_FUNC thread_func3(apr_thread_param_t *thrparm);
+void * APR_THREAD_FUNC thread_func4(apr_thread_param_t *thrparm);
apr_lock_t *thread_lock;
apr_pool_t *context;
int x = 0;
+apr_status_t exit_ret_val = 123; /* just some made up number to check on later */
-void * APR_THREAD_FUNC thread_func1(void *data)
+void * APR_THREAD_FUNC thread_func1(apr_thread_param_t *thrparm)
{
int i;
+ apr_thread_t *thread = thrparm->t;
for (i = 0; i < 10000; i++) {
apr_lock_acquire(thread_lock);
x++;
apr_lock_release(thread_lock);
}
+ apr_thread_exit(thread, &exit_ret_val);
return NULL;
}
-void * APR_THREAD_FUNC thread_func2(void *data)
+void * APR_THREAD_FUNC thread_func2(apr_thread_param_t *thrparm)
{
int i;
+ apr_thread_t *thread = thrparm->t;
for (i = 0; i < 10000; i++) {
apr_lock_acquire(thread_lock);
x++;
apr_lock_release(thread_lock);
}
+ apr_thread_exit(thread, &exit_ret_val);
return NULL;
}
-void * APR_THREAD_FUNC thread_func3(void *data)
+void * APR_THREAD_FUNC thread_func3(apr_thread_param_t *thrparm)
{
int i;
+ apr_thread_t *thread = thrparm->t;
for (i = 0; i < 10000; i++) {
apr_lock_acquire(thread_lock);
x++;
apr_lock_release(thread_lock);
}
+ apr_thread_exit(thread, &exit_ret_val);
return NULL;
}
-void * APR_THREAD_FUNC thread_func4(void *data)
+void * APR_THREAD_FUNC thread_func4(apr_thread_param_t *thrparm)
{
int i;
+ apr_thread_t *thread = thrparm->t;
for (i = 0; i < 10000; i++) {
apr_lock_acquire(thread_lock);
x++;
apr_lock_release(thread_lock);
}
+ apr_thread_exit(thread, &exit_ret_val);
return NULL;
}
@@ -172,7 +181,15 @@
apr_thread_join(&s2, t2);
apr_thread_join(&s3, t3);
apr_thread_join(&s4, t4);
- fprintf (stdout, "OK\n");
+ fprintf (stdout, "OK\n");
+
+ fprintf(stdout, "Checking thread's returned value.......");
+ if (s1 != exit_ret_val || s2 != exit_ret_val ||
+ s3 != exit_ret_val || s4 != exit_ret_val) {
+ fprintf(stderr, "Invalid return value (not expected value)\n");
+ exit(-1);
+ }
+ fprintf(stdout, "OK\n");
fprintf(stdout, "Checking if locks worked.......");
if (x != 40000) {
Index: threadproc/beos/thread.c
===================================================================
RCS file: /home/cvspublic/apr/threadproc/beos/thread.c,v
retrieving revision 1.22
diff -u -r1.22 thread.c
--- threadproc/beos/thread.c 2001/06/14 18:52:05 1.22
+++ threadproc/beos/thread.c 2001/07/18 17:03:33
@@ -94,13 +94,17 @@
{
int32 temp;
apr_status_t stat;
+ struct thread_info_t *thr_info;
+ apr_thread_t *thread;
+ apr_thread_param_t *thrparm;
- (*new) = (apr_thread_t *)apr_palloc(cont, sizeof(apr_thread_t));
- if ((*new) == NULL) {
+ thread = (apr_thread_t *)apr_palloc(cont, sizeof(apr_thread_t));
+ (*new) = thread;
+ if (thread == NULL) {
return APR_ENOMEM;
}
- (*new)->cntxt = cont;
+ thread->cntxt = cont;
/* First we create the new thread...*/
if (attr)
@@ -108,14 +112,23 @@
else
temp = B_NORMAL_PRIORITY;
- stat = apr_pool_create(&(*new)->cntxt, cont);
+ stat = apr_pool_create(&thread->cntxt, cont);
if (stat != APR_SUCCESS) {
return stat;
}
- (*new)->td = spawn_thread((thread_func)func, "apr thread", temp, data);
+ thread->cntxt = cont;
+
+ thrparm = (apr_thread_param_t *)apr_palloc(cont, sizeof(apr_thread_param_t));
+ if (thrparm == NULL) {
+ return APR_ENOMEM;
+ }
+ thrparm->t = thread;
+ thrparm->data = data;
+
+ thread->td = spawn_thread((thread_func)func, "apr thread", temp, thrparm);
/* Now we try to run it...*/
- if (resume_thread((*new)->td) == B_NO_ERROR) {
+ if (resume_thread(thread->td) == B_NO_ERROR) {
return APR_SUCCESS;
}
else {
@@ -142,7 +155,10 @@
apr_status_t apr_thread_join(apr_status_t *retval, apr_thread_t *thd)
{
- if (wait_for_thread(thd->td,(void *)&retval) == B_NO_ERROR) {
+ apr_status_t *thrstat;
+
+ if (wait_for_thread(thd->td,(void *)&thrstat) == B_NO_ERROR) {
+ *retval = *thrstat;
return APR_SUCCESS;
}
else {
Index: threadproc/os2/thread.c
===================================================================
RCS file: /home/cvspublic/apr/threadproc/os2/thread.c,v
retrieving revision 1.22
diff -u -r1.22 thread.c
--- threadproc/os2/thread.c 2001/06/16 01:27:15 1.22
+++ threadproc/os2/thread.c 2001/07/18 17:03:33
@@ -94,8 +94,18 @@
static void apr_thread_begin(void *arg)
{
- apr_thread_t *thread = (apr_thread_t *)arg;
- thread->rv = thread->func(thread->data);
+ apr_thread_param_t *thrparm;
+ apr_thread_t *thread = (apr_thread_t *)arg;
+
+ thrparm = (apr_thread_param_t *)apr_palloc(cont, sizeof(apr_thread_param_t));
+ if (thrparm == NULL) {
+ return APR_ENOMEM;
+ }
+
+ thrparm->t = thread;
+ thrparm->data = data;
+
+ thread->rv = thread->func(thrparm);
}
@@ -132,12 +142,23 @@
}
}
- if (thread->attr->attr & APR_THREADATTR_DETACHED)
+ if (thread->attr->attr & APR_THREADATTR_DETACHED) {
+ apr_thread_param_t *thrparm;
+
+ thrparm = (apr_thread_param_t *)apr_palloc(cont, sizeof(apr_thread_param_t));
+ if (thrparm == NULL) {
+ return APR_ENOMEM;
+ }
+
+ thrparm->t = thread;
+ thrparm->data = data;
+
thread->tid = _beginthread((os2_thread_start_t)func, NULL,
- APR_THREAD_STACKSIZE, data);
- else
+ APR_THREAD_STACKSIZE, thrparm);
+ } else {
thread->tid = _beginthread(apr_thread_begin, NULL,
APR_THREAD_STACKSIZE, thread);
+ }
if (thread->tid < 0) {
return errno;
Index: threadproc/unix/thread.c
===================================================================
RCS file: /home/cvspublic/apr/threadproc/unix/thread.c,v
retrieving revision 1.39
diff -u -r1.39 thread.c
--- threadproc/unix/thread.c 2001/06/14 18:52:09 1.39
+++ threadproc/unix/thread.c 2001/07/18 17:03:34
@@ -122,32 +122,44 @@
{
apr_status_t stat;
pthread_attr_t *temp;
+ struct thread_info_t *thr_info;
+ apr_thread_t *thread;
+ apr_thread_param_t *thrparm;
- (*new) = (apr_thread_t *)apr_pcalloc(cont, sizeof(apr_thread_t));
+ thread = (apr_thread_t *)apr_pcalloc(cont, sizeof(apr_thread_t));
+ (*new) = thread;
- if ((*new) == NULL) {
+ if (thread == NULL) {
return APR_ENOMEM;
}
- (*new)->td = (pthread_t *)apr_pcalloc(cont, sizeof(pthread_t));
+ thread->td = (pthread_t *)apr_pcalloc(cont, sizeof(pthread_t));
- if ((*new)->td == NULL) {
+ if (thread->td == NULL) {
return APR_ENOMEM;
}
- (*new)->cntxt = cont;
-
if (attr)
temp = attr->attr;
else
temp = NULL;
-
- stat = apr_pool_create(&(*new)->cntxt, cont);
+
+ thread->cntxt = cont;
+
+ stat = apr_pool_create(&thread->cntxt, cont);
if (stat != APR_SUCCESS) {
return stat;
}
+
+ thrparm = (apr_thread_param_t *)apr_pcalloc(cont, sizeof(apr_thread_param_t));
+ if (thrparm == NULL) {
+ return APR_ENOMEM;
+ }
- if ((stat = pthread_create((*new)->td, temp, func, data)) == 0) {
+ thrparm->t = thread;
+ thrparm->data = data;
+
+ if ((stat = pthread_create(thread->td, temp, func, thrparm)) == 0) {
return APR_SUCCESS;
}
else {
@@ -155,7 +167,7 @@
stat = errno;
#endif
return stat;
- }
+ }
}
apr_os_thread_t apr_os_thread_current(void)
@@ -177,9 +189,11 @@
apr_status_t apr_thread_join(apr_status_t *retval, apr_thread_t *thd)
{
+ apr_status_t *thrstat;
apr_status_t stat;
- if ((stat = pthread_join(*thd->td,(void *)&retval)) == 0) {
+ if ((stat = pthread_join(*thd->td,(void **)&thrstat)) == 0) {
+ *retval = *thrstat;
return APR_SUCCESS;
}
else {
Index: threadproc/win32/thread.c
===================================================================
RCS file: /home/cvspublic/apr/threadproc/win32/thread.c,v
retrieving revision 1.32
diff -u -r1.32 thread.c
--- threadproc/win32/thread.c 2001/07/06 14:20:03 1.32
+++ threadproc/win32/thread.c 2001/07/18 17:03:34
@@ -97,25 +97,36 @@
apr_status_t stat;
unsigned temp;
int lasterror;
+ apr_thread_t *thread;
+ apr_thread_param_t *thrparm;
- (*new) = (apr_thread_t *)apr_palloc(cont, sizeof(apr_thread_t));
+ thread = (apr_thread_t *)apr_palloc(cont, sizeof(apr_thread_t));
+ (*new) = thread;
- if ((*new) == NULL) {
+ if (thread == NULL) {
return APR_ENOMEM;
}
- (*new)->cntxt = cont;
+ thread->cntxt = cont;
- stat = apr_pool_create(&(*new)->cntxt, cont);
+ stat = apr_pool_create(&thread->cntxt, cont);
if (stat != APR_SUCCESS) {
return stat;
}
+ thrparm = (apr_thread_param_t *)apr_palloc(cont, sizeof(apr_thread_param_t));
+ if (thrparm == NULL) {
+ return APR_ENOMEM;
+ }
+
+ thrparm->t = thread;
+ thrparm->data = data;
+
/* Use 0 for Thread Stack Size, because that will default the stack to the
* same size as the calling thread.
*/
- if (((*new)->td = (HANDLE *)_beginthreadex(NULL, 0, (unsigned int (APR_THREAD_FUNC *)(void *))func,
- data, 0, &temp)) == 0) {
+ if ((thread->td = (HANDLE *)_beginthreadex(NULL, 0, (unsigned int (APR_THREAD_FUNC *)(void *))func,
+ thrparm, 0, &temp)) == 0) {
lasterror = apr_get_os_error();
return APR_EEXIST;
/* MSVC++ doc doesn't mention any additional error info
@@ -124,7 +135,7 @@
}
if (attr && attr->detach) {
- CloseHandle((*new)->td);
+ CloseHandle(thread->td);
}
return APR_SUCCESS;