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...@clove.org> on 2001/10/01 22:26:05 UTC

[PATCH] try 2: fix apr_thread_exit() param type to be usable

On Mon, Sep 24, 2001 at 03:25:28PM -0700, Aaron Bannert wrote:
> apr_thread_exit() takes a parameter signaling the return status of
> the thread as it is exiting. This patch change the parameter from
> a pointer-to-apr_status_t to just an apr_status_t. The reason for
> this is that the return value cannot* be allocated from heapspace,
> since the only heapspace available to the thread right then is
> about to be destroyed.
> 
> (* Unless you do something gross like use a global pool or a static,
> but that defeats the whole purpose of thread-specific pools.)
> 
> This patch passes the modified testthread program on UNIX. I'm not
> so sure about the semantics of the various other platforms, but
> the changes in this patch are small enough that they can be reviewed
> easily by our platform gurus.
> 
> This patch will have to be applied with a [small] patch to httpd-2.0
> so we don't break the build over there. If I can get a +1 from someone
> on each platform {beos,netware,os2,win32} then I'll produce the httpd
> patch and we can proceed from there.

I have only heard back on this from OtherBill for win32. I'm still
looking for some reassurance from the netware/beos crowds that I'm not
doing anything terrible to their implementation.

For the record, here's a slightly modified version that OtherBill
suggested (only affects win32). Please get back to me on this so I can
proceed to fix httpd and then we can start using the thread return status
from dead threads.

-aaron



Index: include/apr_thread_proc.h
===================================================================
RCS file: /home/cvspublic/apr/include/apr_thread_proc.h,v
retrieving revision 1.75
diff -u -r1.75 apr_thread_proc.h
--- include/apr_thread_proc.h	2001/09/20 08:59:30	1.75
+++ include/apr_thread_proc.h	2001/10/01 20:16:24
@@ -216,7 +216,7 @@
  * @param retval The return value to pass back to any thread that cares
  */
 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.
Index: include/arch/beos/threadproc.h
===================================================================
RCS file: /home/cvspublic/apr/include/arch/beos/threadproc.h,v
retrieving revision 1.19
diff -u -r1.19 threadproc.h
--- include/arch/beos/threadproc.h	2001/09/11 13:33:39	1.19
+++ include/arch/beos/threadproc.h	2001/10/01 20:16:24
@@ -82,6 +82,7 @@
     thread_id td;
     void *data;
     apr_thread_start_t func;
+    apr_status_t rv;
 };
 
 struct apr_threadattr_t {
Index: include/arch/netware/threadproc.h
===================================================================
RCS file: /home/cvspublic/apr/include/arch/netware/threadproc.h,v
retrieving revision 1.2
diff -u -r1.2 threadproc.h
--- include/arch/netware/threadproc.h	2001/09/10 22:39:37	1.2
+++ include/arch/netware/threadproc.h	2001/10/01 20:16:24
@@ -67,6 +67,7 @@
     apr_pool_t *cntxt;
     NXContext_t ctx;
     NXThreadId_t td;
+    apr_status_t rv;
 };
 
 struct apr_threadattr_t {
Index: include/arch/unix/threadproc.h
===================================================================
RCS file: /home/cvspublic/apr/include/arch/unix/threadproc.h,v
retrieving revision 1.20
diff -u -r1.20 threadproc.h
--- include/arch/unix/threadproc.h	2001/08/27 03:17:15	1.20
+++ include/arch/unix/threadproc.h	2001/10/01 20:16:25
@@ -92,6 +92,7 @@
     pthread_t *td;
     void *data;
     apr_thread_start_t func;
+    apr_status_t rv;
 };
 
 struct apr_threadattr_t {
Index: include/arch/win32/threadproc.h
===================================================================
RCS file: /home/cvspublic/apr/include/arch/win32/threadproc.h,v
retrieving revision 1.16
diff -u -r1.16 threadproc.h
--- include/arch/win32/threadproc.h	2001/09/09 06:03:05	1.16
+++ include/arch/win32/threadproc.h	2001/10/01 20:16:25
@@ -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: threadproc/beos/thread.c
===================================================================
RCS file: /home/cvspublic/apr/threadproc/beos/thread.c,v
retrieving revision 1.27
diff -u -r1.27 thread.c
--- threadproc/beos/thread.c	2001/09/11 13:33:39	1.27
+++ threadproc/beos/thread.c	2001/10/01 20:16:25
@@ -141,18 +141,20 @@
     return tid1 == tid2;
 }
 
-APR_DECLARE(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)
 {
+    thd->rv = retval;
     apr_pool_destroy(thd->cntxt);
-	exit_thread ((status_t)(*retval));
+	exit_thread(NULL);
 	return APR_SUCCESS;
 }
 
 APR_DECLARE(apr_status_t) apr_thread_join(apr_status_t *retval, apr_thread_t *thd)
 {
-    status_t rv = 0, ret;
-    if ((ret = wait_for_thread(thd->td,&rv)) == B_NO_ERROR) {
-        *retval = rv;
+    status_t ret;
+    /* FIXME: Is it ok to pass in a NULL as the retval? */
+    if ((ret = wait_for_thread(thd->td, NULL)) == B_NO_ERROR) {
+        *retval = thd->rv;
         return APR_SUCCESS;
     }
     else {
Index: threadproc/netware/thread.c
===================================================================
RCS file: /home/cvspublic/apr/threadproc/netware/thread.c,v
retrieving revision 1.4
diff -u -r1.4 thread.c
--- threadproc/netware/thread.c	2001/09/10 22:42:22	1.4
+++ threadproc/netware/thread.c	2001/10/01 20:16:25
@@ -175,10 +175,11 @@
 }
 
 apr_status_t apr_thread_exit(apr_thread_t *thd,
-                                          apr_status_t *retval)
+                             apr_status_t retval)
 {
+    thd->rv = retval;
     apr_pool_destroy(thd->cntxt);
-    NXThreadExit((void *)*retval);
+    NXThreadExit(NULL);
     return APR_SUCCESS;
 }
 
@@ -188,7 +189,8 @@
     apr_status_t  stat;    
     NXThreadId_t dthr;
 
-    if ((stat = NXThreadJoin(thd->td, &dthr, (void *)&retval)) == 0) {
+    if ((stat = NXThreadJoin(thd->td, &dthr, NULL)) == 0) {
+        *retval = thd->rv;
         return APR_SUCCESS;
     }
     else {
@@ -253,4 +255,5 @@
     return APR_SUCCESS;
 }
 
-*/
\ No newline at end of file
+*/
+
Index: threadproc/os2/thread.c
===================================================================
RCS file: /home/cvspublic/apr/threadproc/os2/thread.c,v
retrieving revision 1.29
diff -u -r1.29 thread.c
--- threadproc/os2/thread.c	2001/09/02 05:34:55	1.29
+++ threadproc/os2/thread.c	2001/10/01 20:16:25
@@ -154,9 +154,9 @@
 
 
 
-APR_DECLARE(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)
 {
-    thd->rv = *retval;
+    thd->rv = retval;
     _endthread();
     return -1; /* If we get here something's wrong */
 }
@@ -176,7 +176,7 @@
     if (rc == ERROR_INVALID_THREADID)
         rc = 0; /* Thread had already terminated */
 
-    *retval = (apr_status_t)thd->rv;
+    *retval = thd->rv;
     return APR_OS2_STATUS(rc);
 }
 
Index: threadproc/unix/thread.c
===================================================================
RCS file: /home/cvspublic/apr/threadproc/unix/thread.c,v
retrieving revision 1.48
diff -u -r1.48 thread.c
--- threadproc/unix/thread.c	2001/09/02 03:44:37	1.48
+++ threadproc/unix/thread.c	2001/10/01 20:16:26
@@ -176,10 +176,11 @@
     return pthread_equal(tid1, tid2);
 }
 
-APR_DECLARE(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)
 {
+    thd->rv = retval;
     apr_pool_destroy(thd->cntxt);
-    pthread_exit(retval);
+    pthread_exit(NULL);
     return APR_SUCCESS;
 }
 
@@ -189,7 +190,7 @@
     apr_status_t *thread_stat;
 
     if ((stat = pthread_join(*thd->td,(void *)&thread_stat)) == 0) {
-        *retval = thread_stat ? *thread_stat : APR_SUCCESS;
+        *retval = thd->rv;
         return APR_SUCCESS;
     }
     else {
Index: threadproc/win32/thread.c
===================================================================
RCS file: /home/cvspublic/apr/threadproc/win32/thread.c,v
retrieving revision 1.39
diff -u -r1.39 thread.c
--- threadproc/win32/thread.c	2001/09/01 16:48:58	1.39
+++ threadproc/win32/thread.c	2001/10/01 20:16:26
@@ -140,10 +140,11 @@
 }
 
 APR_DECLARE(apr_status_t) apr_thread_exit(apr_thread_t *thd,
-                                          apr_status_t *retval)
+                                          apr_status_t retval)
 {
+    thd->rv = retval;
     apr_pool_destroy(thd->cntxt);
-    _endthreadex(*retval);
+    _endthreadex(0);
 	return APR_SUCCESS;
 }
 
@@ -153,10 +154,8 @@
     apr_status_t stat;
 
     if ((stat = WaitForSingleObject(thd->td, INFINITE)) == WAIT_OBJECT_0) {
-        if (GetExitCodeThread(thd->td, retval) == 0) {
-            return APR_SUCCESS;
-        }
-        return apr_get_os_error();
+        *retval = thd->rv;
+        return APR_SUCCESS;
     }
     else {
         return stat;

Re: [PATCH] try 2: fix apr_thread_exit() param type to be usable

Posted by Aaron Bannert <aa...@clove.org>.
On Mon, Oct 01, 2001 at 01:26:05PM -0700, Aaron Bannert wrote:
> For the record, here's a slightly modified version that OtherBill
> suggested (only affects win32). Please get back to me on this so I can
> proceed to fix httpd and then we can start using the thread return status
> from dead threads.

Dang, I forgot the test/testthread.c modifications:


Index: test/testthread.c
===================================================================
RCS file: /home/cvspublic/apr/test/testthread.c,v
retrieving revision 1.24
diff -u -r1.24 testthread.c
--- test/testthread.c	2001/09/02 03:41:17	1.24
+++ test/testthread.c	2001/10/01 20:28:20
@@ -102,7 +102,7 @@
         x++;
         apr_lock_release(thread_lock);
     }
-    apr_thread_exit(thd, &exit_ret_val);
+    apr_thread_exit(thd, exit_ret_val);
     return NULL;
 } 
 
@@ -117,7 +117,7 @@
         x++;
         apr_lock_release(thread_lock);
     }
-    apr_thread_exit(thd, &exit_ret_val);
+    apr_thread_exit(thd, exit_ret_val);
     return NULL;
 } 
 
@@ -132,7 +132,7 @@
         x++;
         apr_lock_release(thread_lock);
     }
-    apr_thread_exit(thd, &exit_ret_val);
+    apr_thread_exit(thd, exit_ret_val);
     return NULL;
 } 
 
@@ -147,7 +147,7 @@
         x++;
         apr_lock_release(thread_lock);
     }
-    apr_thread_exit(thd, &exit_ret_val);
+    apr_thread_exit(thd, exit_ret_val);
     return NULL;
 }