Re: linuxthread race condition

[Andreas Schwab <schwab at suse dot de>]

Jakub Jelinek <jakub@redhat.com> writes:

|> On Fri, Nov 30, 2001 at 10:42:51AM +0100, Andreas Schwab wrote:
|> > +  if (sp >= __pthread_initial_thread_bos)
|> > +    return &__pthread_initial_thread;
|> 
|> How could it be initial thread? Even if it could (it cannot), for
|> FLOATING_STACKS, this is totally bogus test (sp >= -1) and for non-floating
|> stacks there is no guarantee that there are no thread stacks above initial
|> thread stack (think about sparc64, where sp initial thread stack is below VM
|> hole, while thread stacks can be in the mmap area (after VM hole)).

Here is now the updated patch, again tested on ia64:

2001-11-30  Andreas Schwab  <schwab@suse.de>

	* pthread.c (pthread_handle_sigcancel) [THREAD_SELF]: Double check
	that self is the manager thread, and initialize the thread
	register if not.
	(thread_self_stack) [THREAD_SELF]: New function to find self via
	stack pointer.
	* manager.c (pthread_handle_create): Don't block cancel signal any
	more.

--- manager.c.~1.76.~	Thu Nov 29 10:40:10 2001
+++ manager.c	Fri Nov 30 12:26:10 2001
@@ -534,7 +534,6 @@
   size_t guardsize = 0;
   int pagesize = __getpagesize();
   int saved_errno;
-  sigset_t newmask, oldmask;
 
   /* First check whether we have to change the policy and if yes, whether
      we can  do this.  Normally this should be done by examining the
@@ -620,11 +619,6 @@
       if ((mask & (__pthread_threads_events.event_bits[idx]
 		   | event_maskp->event_bits[idx])) != 0)
 	{
-	  /* Block cancel signal in the child until it is fully
-	     initialized.  */
-	  sigemptyset(&newmask);
-	  sigaddset(&newmask, __pthread_sig_cancel);
-	  sigprocmask(SIG_BLOCK, &newmask, &oldmask);
 	  /* Lock the mutex the child will use now so that it will stop.  */
 	  __pthread_lock(new_thread->p_lock, NULL);
 
@@ -653,7 +647,6 @@
 			__pthread_sig_cancel, new_thread);
 #endif
 	  saved_errno = errno;
-	  sigprocmask(SIG_SETMASK, &oldmask, NULL);
 	  if (pid != -1)
 	    {
 	      /* Now fill in the information about the new thread in
@@ -679,11 +672,6 @@
     }
   if (pid == 0)
     {
-      /* Block cancel signal in the child until it is fully
-	 initialized.  */
-      sigemptyset(&newmask);
-      sigaddset(&newmask, __pthread_sig_cancel);
-      sigprocmask(SIG_BLOCK, &newmask, &oldmask);
 #ifdef NEED_SEPARATE_REGISTER_STACK
       pid = __clone2(pthread_start_thread,
 		     (void **)new_thread_bottom,
@@ -700,7 +688,6 @@
 		    __pthread_sig_cancel, new_thread);
 #endif /* !NEED_SEPARATE_REGISTER_STACK */
       saved_errno = errno;
-      sigprocmask(SIG_SETMASK, &oldmask, NULL);
     }
   /* Check if cloning succeeded */
   if (pid == -1) {
--- pthread.c.~1.88.~	Thu Nov 29 10:40:10 2001
+++ pthread.c	Fri Nov 30 12:27:44 2001
@@ -709,7 +709,7 @@
 
 #ifndef THREAD_SELF
 
-pthread_descr __pthread_find_self()
+pthread_descr __pthread_find_self(void)
 {
   char * sp = CURRENT_STACK_FRAME;
   pthread_handle h;
@@ -721,6 +721,21 @@
   return h->h_descr;
 }
 
+#else
+
+static pthread_descr thread_self_stack(void)
+{
+  char *sp = CURRENT_STACK_FRAME;
+  pthread_handle h;
+
+  if (sp >= __pthread_manager_thread_bos && sp < __pthread_manager_thread_tos)
+    return &__pthread_manager_thread;
+  h = __pthread_handles + 2;
+  while (! (sp <= (char *) h->h_descr && sp >= h->h_bottom))
+    h++;
+  return h->h_descr;
+}
+
 #endif
 
 /* Thread scheduling */
@@ -769,7 +784,7 @@
   return 0;
 }
 
-int __pthread_yield ()
+int __pthread_yield (void)
 {
   /* For now this is equivalent with the POSIX call.  */
   return sched_yield ();
@@ -841,8 +856,26 @@
 
   if (self == &__pthread_manager_thread)
     {
+#ifdef THREAD_SELF
+      /* A new thread might get a cancel signal before it is fully
+	 initialized, so that the thread register might still point to the
+	 manager thread.  Double check that this is really the manager
+	 thread.  */
+      pthread_descr real_self = thread_self_stack();
+      if (real_self == &__pthread_manager_thread)
+	{
+	  __pthread_manager_sighandler(sig);
+	  return;
+	}
+      /* Oops, thread_self() isn't working yet..  */
+      self = real_self;
+# ifdef INIT_THREAD_SELF
+      INIT_THREAD_SELF(self, self->p_nr);
+# endif
+#else
       __pthread_manager_sighandler(sig);
       return;
+#endif
     }
   if (__builtin_expect (__pthread_exit_requested, 0)) {
     /* Main thread should accumulate times for thread manager and its
@@ -884,7 +917,7 @@
    Notice that we can't free the stack segments, as the forked thread
    may hold pointers into them. */
 
-void __pthread_reset_main_thread()
+void __pthread_reset_main_thread(void)
 {
   pthread_descr self = thread_self();
   struct rlimit limit;

-- 
Andreas Schwab                                  "And now for something
Andreas.Schwab@suse.de				completely different."
SuSE Labs, SuSE GmbH, Schanzäckerstr. 10, D-90443 Nürnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5