Re: Re-use of user-defined stacks for threads

[Wolfram Gloger <Wolfram dot Gloger at dent dot med dot uni-muenchen dot de>]

Hello,

This is my third (and hopefully final) attempt at resolving the
pthread_join() problem with user-defined stacks.  I'll also be sending
a testcase for the problem in a separate mail.

> 2000-12-02  Wolfram Gloger  <wg@malloc.de>
> 
> 	* manager.c (__pthread_handle_free): New function.
...

Further investigation showed that while this patch didn't hurt, it
didn't help much either.  A method to make the manager thread wait
synchronously for thread _exit_ really is required.  Because I didn't
find such a method anywhere in the existing code, I created a new
request REQ_WAIT.  Proposed patch appended below.

Regards,
Wolfram.

2000-12-10  Wolfram Gloger  <wg@malloc.de>

	* internals.h: New request type REQ_WAIT.  Declare
	__pthread_handle_free.

	* manager.c (__pthread_manager): Handle REQ_WAIT request, wait
	synchronously for an exiting thread.
	(__pthread_handle_free): New function.

	* join.c (pthread_exit): For user-defined stack, don't wake up
	joining thread immediately but let the manager do it (via
	REQ_WAIT) when the thread really has exited.
	(pthread_join): When joining a thread with user-defined stack,
	wait for p_exited rather than just p_terminated.  Reclaim thread
	resources immediately in the context of the joining thread rather
	than via the manager.

--- internals.h	2000/12/03 10:42:39	1.1
+++ internals.h	2000/12/11 10:27:38
@@ -199,7 +199,7 @@
   pthread_descr req_thread;     /* Thread doing the request */
   enum {                        /* Request kind */
     REQ_CREATE, REQ_FREE, REQ_PROCESS_EXIT, REQ_MAIN_THREAD_EXIT,
-    REQ_POST, REQ_DEBUG, REQ_KICK
+    REQ_POST, REQ_DEBUG, REQ_KICK, REQ_WAIT
   } req_kind;
   union {                       /* Arguments for request */
     struct {                    /* For REQ_CREATE: */
@@ -215,6 +215,9 @@
       int code;                 /*   exit status */
     } exit;
     void * post;                /* For REQ_POST: the semaphore */
+    struct {                    /* FOR REQ_WAIT */
+      pthread_descr th;         /*   waiting thread */
+    } wait;
   } req_args;
 };
 
@@ -467,6 +470,9 @@
 int __pthread_timedsuspend_new(pthread_descr self, const struct timespec *abs);
 
 void __pthread_wait_for_restart_signal(pthread_descr self);
+
+/* Assumes handle->h_lock is already locked.  */
+void __pthread_handle_free(pthread_handle handle);
 
 int __pthread_yield (void);
 
--- manager.c	2000/12/03 10:36:33	1.1
+++ manager.c	2000/12/11 10:29:21
@@ -106,6 +106,7 @@
 static void pthread_handle_exit(pthread_descr issuing_thread, int exitcode);
 static void pthread_reap_children(void);
 static void pthread_kill_all_threads(int sig, int main_thread_also);
+static void pthread_exited(pid_t pid);
 
 /* The server thread managing requests for thread creation and termination */
 
@@ -210,6 +211,15 @@
 	/* This is just a prod to get the manager to reap some
 	   threads right away, avoiding a potential delay at shutdown. */
 	break;
+      case REQ_WAIT:
+	/* Synchronously wait for a thread exiting. */
+	if (!request.req_thread->p_exited &&
+	    (waitpid(request.req_thread->p_pid, NULL, __WCLONE) ==
+	     request.req_thread->p_pid))
+	  pthread_exited(request.req_thread->p_pid);
+	if (request.req_args.wait.th != NULL)
+	  restart(request.req_args.wait.th);
+	break;
       }
     }
   }
@@ -791,7 +801,6 @@
 static void pthread_handle_free(pthread_t th_id)
 {
   pthread_handle handle = thread_handle(th_id);
-  pthread_descr th;
 
   __pthread_lock(&handle->h_lock, NULL);
   if (nonexisting_handle(handle, th_id)) {
@@ -800,7 +809,15 @@
     __pthread_unlock(&handle->h_lock);
     return;
   }
-  th = handle->h_descr;
+  __pthread_handle_free(handle);
+}
+
+/* Assumes handle->h_lock is already locked.  */
+
+void __pthread_handle_free(pthread_handle handle)
+{
+  pthread_descr th = handle->h_descr;
+
   if (th->p_exited) {
     __pthread_unlock(&handle->h_lock);
     pthread_free(th);
--- join.c	2000/12/03 10:37:10	1.1
+++ join.c	2000/12/11 10:29:06
@@ -28,6 +28,7 @@
   pthread_descr self = thread_self();
   pthread_descr joining;
   struct pthread_request request;
+  int userstack;
 
   /* Reset the cancellation flag to avoid looping if the cleanup handlers
      contain cancellation points */
@@ -63,9 +64,24 @@
   THREAD_SETMEM(self, p_terminated, 1);
   /* See if someone is joining on us */
   joining = THREAD_GETMEM(self, p_joining);
+  /* For a user-defined stack, we must wait until the real exit() has
+     completed before restarting the joining thread.  */
+  userstack = THREAD_GETMEM(self, p_userstack);
   __pthread_unlock(THREAD_GETMEM(self, p_lock));
   /* Restart joining thread if any */
-  if (joining != NULL) restart(joining);
+  if (joining != NULL) {
+    if (!userstack)
+      restart(joining);
+    else {
+      /* Make the manager wait synchronously for the exit() below to
+         have completed.  */
+      request.req_thread = self;
+      request.req_kind = REQ_WAIT;
+      request.req_args.wait.th = joining;
+      __libc_write(__pthread_manager_request, (char *)&request,
+		   sizeof(request));
+    }
+  }
   /* If this is the initial thread, block until all threads have terminated.
      If another thread calls exit, we'll be terminated from our signal
      handler. */
@@ -106,11 +122,10 @@
 int pthread_join(pthread_t thread_id, void ** thread_return)
 {
   volatile pthread_descr self = thread_self();
-  struct pthread_request request;
   pthread_handle handle = thread_handle(thread_id);
   pthread_descr th;
   pthread_extricate_if extr;
-  int already_canceled = 0;
+  int already_canceled = 0, userstack;
 
   /* Set up extrication interface */
   extr.pu_object = handle;
@@ -131,14 +146,23 @@
     __pthread_unlock(&handle->h_lock);
     return EINVAL;
   }
+  userstack = th->p_userstack;
   /* If not terminated yet, suspend ourselves. */
-  if (! th->p_terminated) {
+  if (userstack ? !th->p_exited : !th->p_terminated) {
     /* Register extrication interface */
     __pthread_set_own_extricate_if(self, &extr);
     if (!(THREAD_GETMEM(self, p_canceled)
-	&& THREAD_GETMEM(self, p_cancelstate) == PTHREAD_CANCEL_ENABLE))
+	&& THREAD_GETMEM(self, p_cancelstate) == PTHREAD_CANCEL_ENABLE)) {
       th->p_joining = self;
-    else
+      if (th->p_terminated) {
+	struct pthread_request request;
+	request.req_thread = th;
+	request.req_kind = REQ_WAIT;
+	request.req_args.wait.th = self;
+	__libc_write(__pthread_manager_request, (char *)&request,
+		     sizeof(request));
+      }
+    } else
       already_canceled = 1;
     __pthread_unlock(&handle->h_lock);
 
@@ -161,15 +185,9 @@
   }
   /* Get return value */
   if (thread_return != NULL) *thread_return = th->p_retval;
-  __pthread_unlock(&handle->h_lock);
-  /* Send notification to thread manager */
-  if (__pthread_manager_request >= 0) {
-    request.req_thread = self;
-    request.req_kind = REQ_FREE;
-    request.req_args.free.thread_id = thread_id;
-    __libc_write(__pthread_manager_request,
-		 (char *) &request, sizeof(request));
-  }
+  /* Try to reclaim resources immediately; if the user supplied a
+     stack, it must be reusable as soon as pthread_join() returns.  */
+  __pthread_handle_free(handle);
   return 0;
 }