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 02:28:14 UTC

[PATCH] New APR thread API as discussed on list

Ok here goes again, hopefully third try's a charm.


This patch updates the APR thread API as discussed on the list
over the last week or so. Basicly, the following changes were made
and are included in this patch:

- worker_function prototype (apr_thread_start_t) now takes two parameters,
  the apr private data (apr_thread_t*) and the application private
  data (void*).

- Applications' worker_thread() routines may now use the apr_thread_pool_get
  routines to access the pool. These are implemented using the
  APR_POOL_*_ACCESSOR() macros.


This patch was tested on UNIX. For the sake of the reviewer, I will followup
this messages with patches that update testthread.c and another that
will implement the changes for httpd. Also, once this gets commited,
I have a fix for apr_thread_join() that needs to get commited. :)

-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/24 00:11:19
@@ -125,7 +125,10 @@
 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 *);
+/**
+ * The prototype for any APR thread worker functions.
+ */
+typedef void *(APR_THREAD_FUNC *apr_thread_start_t)(apr_thread_t*, void*);
 
 enum kill_conditions {
     kill_never,                 /* process is never sent any signals */
@@ -599,6 +602,14 @@
  * apr_status_t apr_signal_thread((int)(*signal_handler)(int signum))
  */
 APR_DECLARE(apr_status_t) apr_signal_thread(int(*signal_handler)(int signum));
+
+/**
+ * Get the child-pool used by the thread from the thread info.
+ * @return apr_pool_t the pool
+ * @deffunc apr_pool_t apr_thread_info_pool_get(apr_thread_info_t *i)
+ */
+APR_POOL_DECLARE_ACCESSOR(thread);
+
 #endif /* APR_HAS_THREADS */
 
 #ifdef __cplusplus
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/24 00:11:19
@@ -80,6 +80,8 @@
 struct apr_thread_t {
     apr_pool_t *cntxt;
     thread_id td;
+    void *data;
+    apr_thread_start_t func;
 };
 
 struct apr_threadattr_t {
Index: include/arch/os2/threadproc.h
===================================================================
RCS file: /home/cvspublic/apr/include/arch/os2/threadproc.h,v
retrieving revision 1.9
diff -u -r1.9 threadproc.h
--- include/arch/os2/threadproc.h	2001/02/16 04:15:50	1.9
+++ include/arch/os2/threadproc.h	2001/07/24 00:11:19
@@ -95,7 +95,5 @@
     apr_int32_t detached;
 };
 
-typedef void (*os2_thread_start_t)(void *);
-
 #endif  /* ! THREAD_PROC_H */
 
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/24 00:11:19
@@ -90,6 +90,8 @@
 struct apr_thread_t {
     apr_pool_t *cntxt;
     pthread_t *td;
+    void *data;
+    apr_thread_start_t func;
 };
 
 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/24 00:11:19
@@ -66,6 +66,8 @@
     HANDLE td;
     apr_int32_t cancel;
     apr_int32_t cancel_how;
+    void *data;
+    apr_thread_start_t func;
 };
 
 struct apr_threadattr_t {
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/24 00:11:19
@@ -88,6 +88,12 @@
 	return APR_NOTDETACH;
 }
 
+void *dummy_worker(void *opaque)
+{
+    apr_thread_t *thd = (apr_thread_t*)opaque;
+    return thd->func(thd, thd->data);
+}
+
 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)
@@ -101,6 +107,8 @@
     }
 
     (*new)->cntxt = cont;
+    (*new)->data = data;
+    (*new)->func = func;
 
     /* First we create the new thread...*/
 	if (attr)
@@ -113,7 +121,7 @@
         return stat;
     }
 
-    (*new)->td = spawn_thread((thread_func)func, "apr thread", temp, data);
+    (*new)->td = spawn_thread((thread_func)dummy_func, "apr thread", temp, (*new));
     /* Now we try to run it...*/
     if (resume_thread((*new)->td) == B_NO_ERROR) {
         return APR_SUCCESS;
@@ -191,3 +199,5 @@
     (*thd)->td = *thethd;
     return APR_SUCCESS;
 }
+
+APR_POOL_IMPLEMENT_ACCESSOR_X(thread, cntxt)
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/24 00:11:19
@@ -95,7 +95,7 @@
 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, thread->data);
 }
 
 
@@ -131,13 +131,9 @@
             return stat;
         }
     }
-    
-    if (thread->attr->attr & APR_THREADATTR_DETACHED)
-        thread->tid = _beginthread((os2_thread_start_t)func, NULL, 
-                                   APR_THREAD_STACKSIZE, data);
-    else
-        thread->tid = _beginthread(apr_thread_begin, NULL, 
-                                   APR_THREAD_STACKSIZE, thread);
+
+    thread->tid = _beginthread(apr_thread_begin, NULL, 
+                               APR_THREAD_STACKSIZE, thread);
         
     if (thread->tid < 0) {
         return errno;
@@ -235,3 +231,5 @@
 {
     return apr_pool_userdata_set(data, key, cleanup, thread->cntxt);
 }
+
+APR_POOL_IMPLEMENT_ACCESSOR_X(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/24 00:11:19
@@ -116,6 +116,12 @@
     return APR_NOTDETACH;
 }
 
+void *dummy_worker(void *opaque)
+{
+    apr_thread_t *thread = (apr_thread_t*)opaque;
+    return thread->func(thread, thread->data);
+}
+
 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)
@@ -136,18 +142,20 @@
     }
 
     (*new)->cntxt = cont;
-
+    (*new)->data = data;
+    (*new)->func = func;
+    
     if (attr)
         temp = attr->attr;
     else
         temp = NULL;
-    
+
     stat = apr_pool_create(&(*new)->cntxt, cont);
     if (stat != APR_SUCCESS) {
         return stat;
     }
 
-    if ((stat = pthread_create((*new)->td, temp, func, data)) == 0) {
+    if ((stat = pthread_create((*new)->td, temp, dummy_worker, (*new))) == 0) {
         return APR_SUCCESS;
     }
     else {
@@ -240,6 +248,9 @@
     (*thd)->td = thethd;
     return APR_SUCCESS;
 }
+
+APR_POOL_IMPLEMENT_ACCESSOR_X(thread, cntxt)
+
 #endif  /* HAVE_PTHREAD_H */
 #endif  /* APR_HAS_THREADS */
 
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/24 00:11:19
@@ -89,6 +89,12 @@
     return APR_NOTDETACH;
 }
 
+void *dummy_func(void *opaque)
+{
+    apr_thread_t *thd = (apr_thread_t *)opaque;
+    return thd->func(thd, thd->data);
+}
+
 APR_DECLARE(apr_status_t) apr_thread_create(apr_thread_t **new,
                                             apr_threadattr_t *attr,
                                             apr_thread_start_t func,
@@ -105,6 +111,8 @@
     }
 
     (*new)->cntxt = cont;
+    (*new)->data = data;
+    (*new)->func = func;
     
     stat = apr_pool_create(&(*new)->cntxt, cont);
     if (stat != APR_SUCCESS) {
@@ -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 (((*new)->td = (HANDLE *)_beginthreadex(NULL, 0, (unsigned int (APR_THREAD_FUNC *)(void *))dummy_func,
+                                               (*new), 0, &temp)) == 0) {
         lasterror = apr_get_os_error();
         return APR_EEXIST; 
         /* MSVC++ doc doesn't mention any additional error info 
@@ -207,3 +215,4 @@
     return APR_SUCCESS;
 }
 
+APR_POOL_IMPLEMENT_ACCESSOR_X(thread, cntxt)


[PATCH] updates to httpd to work with the new APR thread API

Posted by Aaron Bannert <aa...@ebuilt.com>.
And now the updates to httpd to get it working under httpd [cross posted
to new-httpd because I'll need someone with httpd commit access to update
this once the changes to APR have been commited].


This patch is just the bare minimum to get httpd working with the
new thread API. Changes affected the threaded and perchild MPMs. I'll
revisit both of these MPMs once I have a better idea what can change to
take advantage of the new API (it looks like we are using static/global
variables to pass the parent pool around to the threads...), but for
now this keeps up to date with APR and changes no real functionality.

-aaron

(patch follows quote)


On Mon, Jul 23, 2001 at 05:28:14PM -0700, Aaron Bannert wrote:
> Ok here goes again, hopefully third try's a charm.
> 
> 
> This patch updates the APR thread API as discussed on the list
> over the last week or so. Basicly, the following changes were made
> and are included in this patch:
> 
> - worker_function prototype (apr_thread_start_t) now takes two parameters,
>   the apr private data (apr_thread_t*) and the application private
>   data (void*).
> 
> - Applications' worker_thread() routines may now use the apr_thread_pool_get
>   routines to access the pool. These are implemented using the
>   APR_POOL_*_ACCESSOR() macros.
> 
> 
> This patch was tested on UNIX. For the sake of the reviewer, I will followup
> this messages with patches that update testthread.c and another that
> will implement the changes for httpd. Also, once this gets commited,
> I have a fix for apr_thread_join() that needs to get commited. :)



Index: server/mpm/perchild/perchild.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/server/mpm/perchild/perchild.c,v
retrieving revision 1.71
diff -u -r1.71 perchild.c
--- server/mpm/perchild/perchild.c	2001/07/18 20:45:35	1.71
+++ server/mpm/perchild/perchild.c	2001/07/24 00:40:46
@@ -512,7 +512,7 @@
     }
 }
 
-static void *worker_thread(void *);
+static void *worker_thread(apr_thread_t *, void *);
 
 /* Starts a thread as long as we're below max_threads */
 static int start_thread(void)
@@ -579,7 +579,7 @@
 
 /* idle_thread_count should be incremented before starting a worker_thread */
 
-static void *worker_thread(void *arg)
+static void *worker_thread(apr_thread_t *thd, void *arg)
 {
     apr_socket_t *csd = NULL;
     apr_pool_t *tpool;		/* Pool for this thread           */
Index: server/mpm/threaded/threaded.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/server/mpm/threaded/threaded.c,v
retrieving revision 1.46
diff -u -r1.46 threaded.c
--- server/mpm/threaded/threaded.c	2001/07/18 20:45:36	1.46
+++ server/mpm/threaded/threaded.c	2001/07/24 00:40:47
@@ -520,7 +520,7 @@
     apr_lock_release(pipe_of_death_mutex);
 }
 
-static void * worker_thread(void * dummy)
+static void * worker_thread(apr_thread_t *thd, void * dummy)
 {
     proc_info * ti = dummy;
     int process_slot = ti->pid;
@@ -671,7 +671,7 @@
     return 0;
 }
 
-static void *start_threads(void * dummy)
+static void *start_threads(apr_thread_t *thd, void * dummy)
 {
     thread_starter *ts = dummy;
     apr_thread_t **threads = ts->threads;


[PATCH] updates to httpd to work with the new APR thread API

Posted by Aaron Bannert <aa...@ebuilt.com>.
And now the updates to httpd to get it working under httpd [cross posted
to new-httpd because I'll need someone with httpd commit access to update
this once the changes to APR have been commited].


This patch is just the bare minimum to get httpd working with the
new thread API. Changes affected the threaded and perchild MPMs. I'll
revisit both of these MPMs once I have a better idea what can change to
take advantage of the new API (it looks like we are using static/global
variables to pass the parent pool around to the threads...), but for
now this keeps up to date with APR and changes no real functionality.

-aaron

(patch follows quote)


On Mon, Jul 23, 2001 at 05:28:14PM -0700, Aaron Bannert wrote:
> Ok here goes again, hopefully third try's a charm.
> 
> 
> This patch updates the APR thread API as discussed on the list
> over the last week or so. Basicly, the following changes were made
> and are included in this patch:
> 
> - worker_function prototype (apr_thread_start_t) now takes two parameters,
>   the apr private data (apr_thread_t*) and the application private
>   data (void*).
> 
> - Applications' worker_thread() routines may now use the apr_thread_pool_get
>   routines to access the pool. These are implemented using the
>   APR_POOL_*_ACCESSOR() macros.
> 
> 
> This patch was tested on UNIX. For the sake of the reviewer, I will followup
> this messages with patches that update testthread.c and another that
> will implement the changes for httpd. Also, once this gets commited,
> I have a fix for apr_thread_join() that needs to get commited. :)



Index: server/mpm/perchild/perchild.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/server/mpm/perchild/perchild.c,v
retrieving revision 1.71
diff -u -r1.71 perchild.c
--- server/mpm/perchild/perchild.c	2001/07/18 20:45:35	1.71
+++ server/mpm/perchild/perchild.c	2001/07/24 00:40:46
@@ -512,7 +512,7 @@
     }
 }
 
-static void *worker_thread(void *);
+static void *worker_thread(apr_thread_t *, void *);
 
 /* Starts a thread as long as we're below max_threads */
 static int start_thread(void)
@@ -579,7 +579,7 @@
 
 /* idle_thread_count should be incremented before starting a worker_thread */
 
-static void *worker_thread(void *arg)
+static void *worker_thread(apr_thread_t *thd, void *arg)
 {
     apr_socket_t *csd = NULL;
     apr_pool_t *tpool;		/* Pool for this thread           */
Index: server/mpm/threaded/threaded.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/server/mpm/threaded/threaded.c,v
retrieving revision 1.46
diff -u -r1.46 threaded.c
--- server/mpm/threaded/threaded.c	2001/07/18 20:45:36	1.46
+++ server/mpm/threaded/threaded.c	2001/07/24 00:40:47
@@ -520,7 +520,7 @@
     apr_lock_release(pipe_of_death_mutex);
 }
 
-static void * worker_thread(void * dummy)
+static void * worker_thread(apr_thread_t *thd, void * dummy)
 {
     proc_info * ti = dummy;
     int process_slot = ti->pid;
@@ -671,7 +671,7 @@
     return 0;
 }
 
-static void *start_threads(void * dummy)
+static void *start_threads(apr_thread_t *thd, void * dummy)
 {
     thread_starter *ts = dummy;
     apr_thread_t **threads = ts->threads;


[PATCH] testthread.c updates to work with new thread API

Posted by Aaron Bannert <aa...@ebuilt.com>.
On Mon, Jul 23, 2001 at 05:28:14PM -0700, Aaron Bannert wrote:
> This patch was tested on UNIX. For the sake of the reviewer, I will followup
> this messages with patches that update testthread.c and another that
> will implement the changes for httpd. Also, once this gets commited,
> I have a fix for apr_thread_join() that needs to get commited. :)

Like promised, here's the testthread.c patch. This patch changes
three things:

- Added #include <stdlib.h> to make -Wall happy.

- Changed the thread_func*() routines to work with the new API. (Also
  added the calls to apr_thread_pool_get() to illustrate that the
  pool accessors are working).

- Added a new test to check if the return value from the thread_func()
  is getting passed back properly. ** This test properly detects the
  fact that apr_thread_join() is broken -- subsequent patch to come :) **

-aaron




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/24 00:11:39
@@ -58,6 +58,7 @@
 #include "apr_general.h"
 #include "errno.h"
 #include <stdio.h>
+#include <stdlib.h>
 #ifdef BEOS
 #include <unistd.h>
 #endif
@@ -72,57 +73,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_t *thd, void *data);
+void * APR_THREAD_FUNC thread_func2(apr_thread_t *thd, void *data);
+void * APR_THREAD_FUNC thread_func3(apr_thread_t *thd, void *data);
+void * APR_THREAD_FUNC thread_func4(apr_thread_t *thd, void *data);
 
 
 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_t *thd, void *data)
 {
     int i;
+    apr_pool_t *pool = apr_thread_pool_get(thd);
     for (i = 0; i < 10000; i++) {
         apr_lock_acquire(thread_lock);
         x++;
         apr_lock_release(thread_lock);
     }
+    apr_thread_exit(thd, &exit_ret_val);
     return NULL;
 } 
 
-void * APR_THREAD_FUNC thread_func2(void *data)
+void * APR_THREAD_FUNC thread_func2(apr_thread_t *thd, void *data)
 {
     int i;
+    apr_pool_t *pool = apr_thread_pool_get(thd);
     for (i = 0; i < 10000; i++) {
         apr_lock_acquire(thread_lock);
         x++;
         apr_lock_release(thread_lock);
     }
+    apr_thread_exit(thd, &exit_ret_val);
     return NULL;
 } 
 
-void * APR_THREAD_FUNC thread_func3(void *data)
+void * APR_THREAD_FUNC thread_func3(apr_thread_t *thd, void *data)
 {
     int i;
+    apr_pool_t *pool = apr_thread_pool_get(thd);
     for (i = 0; i < 10000; i++) {
         apr_lock_acquire(thread_lock);
         x++;
         apr_lock_release(thread_lock);
     }
+    apr_thread_exit(thd, &exit_ret_val);
     return NULL;
 } 
 
-void * APR_THREAD_FUNC thread_func4(void *data)
+void * APR_THREAD_FUNC thread_func4(apr_thread_t *thd, void *data)
 {
     int i;
+    apr_pool_t *pool = apr_thread_pool_get(thd);
     for (i = 0; i < 10000; i++) {
         apr_lock_acquire(thread_lock);
         x++;
         apr_lock_release(thread_lock);
     }
+    apr_thread_exit(thd, &exit_ret_val);
     return NULL;
 } 
 
@@ -172,7 +182,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) {