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/24 08:27:17 UTC

[PATCH] Fix to apr_thread_exit() and apr_thread_join()

This patch does two specific things:

- changes apr_thread_exit() to take an apr_status_t, instead of
  an (apr_status_t*) (* note my motivation for this below)

- fixes apr_thread_join()

as well as:
- updates testthread.c to work with these changes, and verify that
  apr_thread_exit() and apr_thread_join() are working the way they
  say they do.



*) I was motivated to change this prototype for a couple reasons: The
simple reason was that we probably don't want threads allocating
a single (int) worth of heap-space for the return value. The second,
and more important reason is that the heap space where this memory
would be allocated from is *destroyed* one line *before* the thread
exits. That is not helpful. So to avoid all this confusion, this patch
simply stores the return value in the thread (it's just an int) and
then apr_thread_join() pulls it back out.

If this patch is commited, I'll supply a patch to update the relevant
parts of httpd. Applying this patch will not break httpd.

-aaron



Index: srclib/apr/include/apr_thread_proc.h
===================================================================
RCS file: /home/cvspublic/apr/include/apr_thread_proc.h,v
retrieving revision 1.66
diff -u -r1.66 apr_thread_proc.h
--- srclib/apr/include/apr_thread_proc.h	2001/07/24 05:16:32	1.66
+++ srclib/apr/include/apr_thread_proc.h	2001/07/24 06:13:00
@@ -205,11 +205,11 @@
  * @deffunc apr_status_t apr_thread_exit(apr_thread_t *thd, apr_status_t *retval)
  */
 APR_DECLARE(apr_status_t) apr_thread_exit(apr_thread_t *thd, 
-                                          apr_status_t *retval);
+                                          apr_status_t retval);
 
 /**
  * block until the desired thread stops executing.
- * @param retval The return value from the dead thread.
+ * @param retval The return value from the dead thread is stored at this address.
  * @param thd The thread to join
  * @deffunc apr_status_t apr_thread_join(apr_status_t *retval, apr_thread_t *thd);
  */
Index: srclib/apr/include/arch/beos/threadproc.h
===================================================================
RCS file: /home/cvspublic/apr/include/arch/beos/threadproc.h,v
retrieving revision 1.18
diff -u -r1.18 threadproc.h
--- srclib/apr/include/arch/beos/threadproc.h	2001/07/24 05:16:32	1.18
+++ srclib/apr/include/arch/beos/threadproc.h	2001/07/24 06:13:00
@@ -82,6 +82,7 @@
     thread_id td;
     void *data;
     apr_thread_start_t func;
+    apr_status_t rv;
 };
 
 struct apr_threadattr_t {
Index: srclib/apr/include/arch/os2/threadproc.h
===================================================================
RCS file: /home/cvspublic/apr/include/arch/os2/threadproc.h,v
retrieving revision 1.10
diff -u -r1.10 threadproc.h
--- srclib/apr/include/arch/os2/threadproc.h	2001/07/24 05:16:32	1.10
+++ srclib/apr/include/arch/os2/threadproc.h	2001/07/24 06:13:00
@@ -74,7 +74,7 @@
     unsigned long tid;
     apr_thread_start_t func;
     void *data;
-    void *rv;
+    apr_status_t rv;
 };
 
 struct apr_threadkey_t {
Index: srclib/apr/include/arch/unix/threadproc.h
===================================================================
RCS file: /home/cvspublic/apr/include/arch/unix/threadproc.h,v
retrieving revision 1.19
diff -u -r1.19 threadproc.h
--- srclib/apr/include/arch/unix/threadproc.h	2001/07/24 05:16:32	1.19
+++ srclib/apr/include/arch/unix/threadproc.h	2001/07/24 06:13:00
@@ -92,6 +92,7 @@
     pthread_t *td;
     void *data;
     apr_thread_start_t func;
+    apr_status_t rv;
 };
 
 struct apr_threadattr_t {
Index: srclib/apr/include/arch/win32/threadproc.h
===================================================================
RCS file: /home/cvspublic/apr/include/arch/win32/threadproc.h,v
retrieving revision 1.14
diff -u -r1.14 threadproc.h
--- srclib/apr/include/arch/win32/threadproc.h	2001/07/24 05:16:32	1.14
+++ srclib/apr/include/arch/win32/threadproc.h	2001/07/24 06:13:01
@@ -68,6 +68,7 @@
     apr_int32_t cancel_how;
     void *data;
     apr_thread_start_t func;
+    apr_status_t rv;
 };
 
 struct apr_threadattr_t {
Index: srclib/apr/threadproc/beos/thread.c
===================================================================
RCS file: /home/cvspublic/apr/threadproc/beos/thread.c,v
retrieving revision 1.23
diff -u -r1.23 thread.c
--- srclib/apr/threadproc/beos/thread.c	2001/07/24 05:16:32	1.23
+++ srclib/apr/threadproc/beos/thread.c	2001/07/24 06:13:01
@@ -141,16 +141,18 @@
     return tid1 == tid2;
 }
 
-apr_status_t apr_thread_exit(apr_thread_t *thd, apr_status_t *retval)
+apr_status_t apr_thread_exit(apr_thread_t *thd, apr_status_t retval)
 {
     apr_pool_destroy(thd->cntxt);
-	exit_thread ((status_t)retval);
-	return APR_SUCCESS;
+    thd->rv = retval;
+    exit_thread ((status_t)retval); /* FIXME: nothing needs to be returned on thread exit */
+    return APR_SUCCESS;
 }
 
 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) {
+        *retval = thd->rv;
         return APR_SUCCESS;
     }
     else {
Index: srclib/apr/threadproc/os2/thread.c
===================================================================
RCS file: /home/cvspublic/apr/threadproc/os2/thread.c,v
retrieving revision 1.23
diff -u -r1.23 thread.c
--- srclib/apr/threadproc/os2/thread.c	2001/07/24 05:16:32	1.23
+++ srclib/apr/threadproc/os2/thread.c	2001/07/24 06:13:01
@@ -154,7 +154,7 @@
 
 
 
-apr_status_t apr_thread_exit(apr_thread_t *thd, apr_status_t *retval)
+apr_status_t apr_thread_exit(apr_thread_t *thd, apr_status_t retval)
 {
     thd->rv = retval;
     _endthread();
Index: srclib/apr/threadproc/unix/thread.c
===================================================================
RCS file: /home/cvspublic/apr/threadproc/unix/thread.c,v
retrieving revision 1.40
diff -u -r1.40 thread.c
--- srclib/apr/threadproc/unix/thread.c	2001/07/24 05:16:32	1.40
+++ srclib/apr/threadproc/unix/thread.c	2001/07/24 06:13:01
@@ -176,18 +176,20 @@
     return pthread_equal(tid1, tid2);
 }
 
-apr_status_t apr_thread_exit(apr_thread_t *thd, apr_status_t *retval)
+apr_status_t apr_thread_exit(apr_thread_t *thd, apr_status_t retval)
 {
     apr_pool_destroy(thd->cntxt);
-    pthread_exit(retval);
-    return APR_SUCCESS;
+    thd->rv = retval;
+    pthread_exit(NULL);
+    return -1;  /* If we get here something's wrong */
 }
 
 apr_status_t apr_thread_join(apr_status_t *retval, apr_thread_t *thd)
 {
     apr_status_t stat;
 
-    if ((stat = pthread_join(*thd->td,(void *)&retval)) == 0) {
+    if ((stat = pthread_join(*thd->td, NULL)) == 0) {
+        *retval = thd->rv;
         return APR_SUCCESS;
     }
     else {
Index: srclib/apr/threadproc/win32/thread.c
===================================================================
RCS file: /home/cvspublic/apr/threadproc/win32/thread.c,v
retrieving revision 1.33
diff -u -r1.33 thread.c
--- srclib/apr/threadproc/win32/thread.c	2001/07/24 05:16:32	1.33
+++ srclib/apr/threadproc/win32/thread.c	2001/07/24 06:13:01
@@ -139,10 +139,11 @@
 }
 
 APR_DECLARE(apr_status_t) apr_thread_exit(apr_thread_t *thd,
-                                          apr_status_t *retval)
+                                          apr_status_t retval)
 {
     apr_pool_destroy(thd->cntxt);
-    _endthreadex(*retval);
+    thd->rv = retval;
+    _endthreadex(*retval); /* FIXME: nothing needs to be returned on thread exit */
 	return APR_SUCCESS;
 }
 
@@ -153,6 +154,7 @@
 
     if ((stat = WaitForSingleObject(thd->td, INFINITE)) == WAIT_OBJECT_0) {
         if (GetExitCodeThread(thd->td, retval) == 0) {
+            *retval = thd->rv;
             return APR_SUCCESS;
         }
         return apr_get_os_error();



Index: srclib/apr/test/testthread.c
===================================================================
RCS file: /home/cvspublic/apr/test/testthread.c,v
retrieving revision 1.18
diff -u -r1.18 testthread.c
--- srclib/apr/test/testthread.c	2001/07/24 05:17:26	1.18
+++ srclib/apr/test/testthread.c	2001/07/24 06:14:00
@@ -88,12 +88,13 @@
 {
     int i;
     apr_pool_t *pool = apr_thread_pool_get(thd);
+    pool = NULL; /* gets rid of the -Wall warning */
     for (i = 0; i < 10000; i++) {
         apr_lock_acquire(thread_lock);
         x++;
         apr_lock_release(thread_lock);
     }
-    apr_thread_exit(thd, &exit_ret_val);
+    apr_thread_exit(thd, exit_ret_val);
     return NULL;
 } 
 
@@ -101,12 +102,13 @@
 {
     int i;
     apr_pool_t *pool = apr_thread_pool_get(thd);
+    pool = NULL; /* gets rid of the -Wall warning */
     for (i = 0; i < 10000; i++) {
         apr_lock_acquire(thread_lock);
         x++;
         apr_lock_release(thread_lock);
     }
-    apr_thread_exit(thd, &exit_ret_val);
+    apr_thread_exit(thd, exit_ret_val);
     return NULL;
 } 
 
@@ -114,12 +116,13 @@
 {
     int i;
     apr_pool_t *pool = apr_thread_pool_get(thd);
+    pool = NULL; /* gets rid of the -Wall warning */
     for (i = 0; i < 10000; i++) {
         apr_lock_acquire(thread_lock);
         x++;
         apr_lock_release(thread_lock);
     }
-    apr_thread_exit(thd, &exit_ret_val);
+    apr_thread_exit(thd, exit_ret_val);
     return NULL;
 } 
 
@@ -127,12 +130,13 @@
 {
     int i;
     apr_pool_t *pool = apr_thread_pool_get(thd);
+    pool = NULL; /* gets rid of the -Wall warning */
     for (i = 0; i < 10000; i++) {
         apr_lock_acquire(thread_lock);
         x++;
         apr_lock_release(thread_lock);
     }
-    apr_thread_exit(thd, &exit_ret_val);
+    apr_thread_exit(thd, exit_ret_val);
     return NULL;
 }