This patch provides a few fixes and a couple changes to the APR
threads API as discussed on this list. This does not implement the
controversial issues from the list, but merly a couple bug fixes I've
found and a widening of the worker_fn() parameters to make it possible
for applications to call functions like apr_thread_exit() and actually
use the childpool that is implicitly created.
In all cases they were tested on Unix (using my Linux 2.4 box). I
attempted to get the code for beos, os2, and win32 as close to
what I think it should be, but others will have to verify my changes:
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.
3) Changed the prototype for apr_thread_start_t to pass in the pool context
and the apr_thread_t structure for a couple reasons:
a) apr_thread_exit() requires the apr_thread_t, this makes it explicit
to the application developer where that comes from.
b) apr_thread_create() was creating a child-pool but only saving that
child pool in the private platform-specific apr_thread_t struct
definition, making it unavailable to the actual thread. Now it is
directly available and the app programmer knows exactly from which
pool to do allocations/cleanup registrations.
Like I said, I tested this on my Linux box, and did my best on the rest.
Win32 was the most unique implementation, so it would be best if someone
could run the new test/testthread to see if that was even working before
and that it works now (it was broken on unix and beos, os2 was ok IIRC).
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 06:27:04
@@ -125,7 +125,7 @@
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 *);
+typedef void *(APR_THREAD_FUNC *apr_thread_start_t)(void *, apr_thread_t *,
apr_pool_t *);
enum kill_conditions {
kill_never, /* process is never sent any signals */
Index: include/arch/beos/threadproc.h
===================================================================
RCS file: /home/cvspublic/apr/include/arch/beos/threadproc.h,v
retrieving revision 1.17
diff -u -r1.17 threadproc.h
--- include/arch/beos/threadproc.h 2001/02/16 04:15:49 1.17
+++ include/arch/beos/threadproc.h 2001/07/18 06:27:05
@@ -80,6 +80,8 @@
struct apr_thread_t {
apr_pool_t *cntxt;
thread_id td;
+ apr_thread_start_t func;
+ void *data;
};
struct apr_threadattr_t {
Index: include/arch/unix/threadproc.h
===================================================================
RCS file: /home/cvspublic/apr/include/arch/unix/threadproc.h,v
retrieving revision 1.18
diff -u -r1.18 threadproc.h
--- include/arch/unix/threadproc.h 2001/04/11 02:01:18 1.18
+++ include/arch/unix/threadproc.h 2001/07/18 06:27:05
@@ -90,6 +90,8 @@
struct apr_thread_t {
apr_pool_t *cntxt;
pthread_t *td;
+ apr_thread_start_t func;
+ void *data;
};
struct apr_threadattr_t {
Index: include/arch/win32/threadproc.h
===================================================================
RCS file: /home/cvspublic/apr/include/arch/win32/threadproc.h,v
retrieving revision 1.13
diff -u -r1.13 threadproc.h
--- include/arch/win32/threadproc.h 2001/02/16 04:15:52 1.13
+++ include/arch/win32/threadproc.h 2001/07/18 06:27:05
@@ -66,6 +66,8 @@
HANDLE td;
apr_int32_t cancel;
apr_int32_t cancel_how;
+ apr_thread_start_t func;
+ void *data;
};
struct apr_threadattr_t {
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 06:27:05
@@ -72,17 +72,18 @@
}
#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(void *data, apr_thread_t *thread,
apr_pool_t *pool);
+void * APR_THREAD_FUNC thread_func2(void *data, apr_thread_t *thread,
apr_pool_t *pool);
+void * APR_THREAD_FUNC thread_func3(void *data, apr_thread_t *thread,
apr_pool_t *pool);
+void * APR_THREAD_FUNC thread_func4(void *data, apr_thread_t *thread,
apr_pool_t *pool);
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(void *data, apr_thread_t *thread,
apr_pool_t *pool)
{
int i;
for (i = 0; i < 10000; i++) {
@@ -90,10 +91,11 @@
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(void *data, apr_thread_t *thread,
apr_pool_t *pool)
{
int i;
for (i = 0; i < 10000; i++) {
@@ -101,10 +103,11 @@
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(void *data, apr_thread_t *thread,
apr_pool_t *pool)
{
int i;
for (i = 0; i < 10000; i++) {
@@ -112,10 +115,11 @@
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(void *data, apr_thread_t *thread,
apr_pool_t *pool)
{
int i;
for (i = 0; i < 10000; i++) {
@@ -123,6 +127,7 @@
x++;
apr_lock_release(thread_lock);
}
+ apr_thread_exit(thread, &exit_ret_val);
return NULL;
}
@@ -172,7 +177,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 06:27:05
@@ -88,19 +88,28 @@
return APR_NOTDETACH;
}
+void *dummy_func(void *opaque)
+{
+ apr_thread_t *thread = (apr_thread_t*)opaque;
+ return thread->func(thread->data, thread, thread->cntxt);
+}
+
apr_status_t apr_thread_create(apr_thread_t **new, apr_threadattr_t *attr,
apr_thread_start_t func, void *data,
apr_pool_t *cont)
{
int32 temp;
apr_status_t stat;
+ struct thread_info_t *thr_info;
+ apr_thread_t *thread;
- (*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,12 +117,16 @@
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;
+ thread->func = func;
+ thread->data = data;
+
+ thread->td = spawn_thread((thread_func)dummy_func, "apr thread", temp,
thread);
/* Now we try to run it...*/
if (resume_thread((*new)->td) == B_NO_ERROR) {
return APR_SUCCESS;
@@ -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 06:27:05
@@ -91,11 +91,10 @@
}
-
static void apr_thread_begin(void *arg)
{
apr_thread_t *thread = (apr_thread_t *)arg;
- thread->rv = thread->func(thread->data);
+ thread->rv = thread->func(thread->data, thread, thread->cntxt);
}
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 06:27:05
@@ -116,38 +116,49 @@
return APR_NOTDETACH;
}
+void *dummy_func(void *opaque)
+{
+ apr_thread_t *thread = (apr_thread_t*)opaque;
+ return thread->func(thread->data, thread, thread->cntxt);
+}
+
apr_status_t apr_thread_create(apr_thread_t **new, apr_threadattr_t *attr,
apr_thread_start_t func, void *data,
apr_pool_t *cont)
{
apr_status_t stat;
pthread_attr_t *temp;
+ struct thread_info_t *thr_info;
+ apr_thread_t *thread;
- (*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;
+ thread->func = func;
+ thread->data = data;
+
+ stat = apr_pool_create(&thread->cntxt, cont);
if (stat != APR_SUCCESS) {
return stat;
}
- if ((stat = pthread_create((*new)->td, temp, func, data)) == 0) {
+ if ((stat = pthread_create(thread->td, temp, dummy_func, thread)) == 0) {
return APR_SUCCESS;
}
else {
@@ -155,7 +166,7 @@
stat = errno;
#endif
return stat;
- }
+ }
}
apr_os_thread_t apr_os_thread_current(void)
@@ -177,9 +188,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 06:27:06
@@ -89,6 +89,12 @@
return APR_NOTDETACH;
}
+void *dummy_func(void *opaque)
+{
+ apr_thread_t *thread = (apr_thread_t*)opaque;
+ return thread->func(thread->data, thread, thread->cntxt);
+}
+
APR_DECLARE(apr_status_t) apr_thread_create(apr_thread_t **new,
apr_threadattr_t *attr,
apr_thread_start_t func,
@@ -97,16 +103,18 @@
apr_status_t stat;
unsigned temp;
int lasterror;
+ apr_thread_t *thread;
- (*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;
}
@@ -114,8 +122,8 @@
/* 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 *))dummy_func,
+ thread, 0, &temp)) == 0) {
lasterror = apr_get_os_error();
return APR_EEXIST;
/* MSVC++ doc doesn't mention any additional error info
@@ -124,7 +132,7 @@
}
if (attr && attr->detach) {
- CloseHandle((*new)->td);
+ CloseHandle(thread->td);
}
return APR_SUCCESS;
