[BZ #2843]

2006-08-12  Ulrich Drepper  <drepper@redhat.com>
	[BZ #2843]
	* pthread_join.c (pthread_join): Account for self being canceled
	when checking for deadlocks.
	* tst-join5.c: Cleanups.  Allow to be used in tst-join6.
	(tf1): Don't print anything after pthread_join returns, this would be
	another cancellation point.
	(tf2): Likewise.
	* tst-join6.c: New file.
	* Makefile (tests): Add tst-join6.

nptl/ChangeLog

@@ -1,3 +1,15 @@
+2006-08-12  Ulrich Drepper  <drepper@redhat.com>
+
+	[BZ #2843]
+	* pthread_join.c (pthread_join): Account for self being canceled
+	when checking for deadlocks.
+	* tst-join5.c: Cleanups.  Allow to be used in tst-join6.
+	(tf1): Don't print anything after pthread_join returns, this would be
+	another cancellation point.
+	(tf2): Likewise.
+	* tst-join6.c: New file.
+	* Makefile (tests): Add tst-join6.
+
 2006-08-03  Ulrich Drepper  <drepper@redhat.com>
 
 	[BZ #2892]

nptl/Makefile

@@ -224,7 +224,7 @@ tests = tst-typesizes \
 	tst-basic1 tst-basic2 tst-basic3 tst-basic4 tst-basic5 tst-basic6 \
 	tst-kill1 tst-kill2 tst-kill3 tst-kill4 tst-kill5 tst-kill6 \
 	tst-raise1 \
-	tst-join1 tst-join2 tst-join3 tst-join4 tst-join5 \
+	tst-join1 tst-join2 tst-join3 tst-join4 tst-join5 tst-join6 \
 	tst-detach1 \
 	tst-eintr1 tst-eintr2 tst-eintr3 tst-eintr4 tst-eintr5 \
 	tst-tsd1 tst-tsd2 tst-tsd3 tst-tsd4 tst-tsd5 \

nptl/pthread_join.c

@@ -1,4 +1,4 @@
-/* Copyright (C) 2002, 2003, 2005 Free Software Foundation, Inc.
+/* Copyright (C) 2002, 2003, 2005, 2006 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Ulrich Drepper <drepper@redhat.com>, 2002.
 
@@ -27,7 +27,11 @@
 static void
 cleanup (void *arg)
 {
-  *(void **) arg = NULL;
+  /* If we already changed the waiter ID, reset it.  The call cannot
+     fail for any reason but the thread not having done that yet so
+     there is no reason for a loop.  */
+  atomic_compare_and_exchange_bool_acq ((struct pthread **) arg, NULL,
+					THREAD_SELF);
 }
 
 
@@ -36,7 +40,6 @@ pthread_join (threadid, thread_return)
      pthread_t threadid;
      void **thread_return;
 {
-  struct pthread *self;
   struct pthread *pd = (struct pthread *) threadid;
 
   /* Make sure the descriptor is valid.  */
@@ -49,41 +52,41 @@ pthread_join (threadid, thread_return)
     /* We cannot wait for the thread.  */
     return EINVAL;
 
-  self = THREAD_SELF;
-  if (pd == self
-      || (self->joinid == pd
-	  && (pd->cancelhandling
-	      & (CANCELING_BITMASK | CANCELED_BITMASK | EXITING_BITMASK
-		 | TERMINATED_BITMASK)) == 0))
+  struct pthread *self = THREAD_SELF;
+  int result = 0;
+
+  /* During the wait we change to asynchronous cancellation.  If we
+     are canceled the thread we are waiting for must be marked as
+     un-wait-ed for again.  */
+  pthread_cleanup_push (cleanup, &pd->joinid);
+
+  /* Switch to asynchronous cancellation.  */
+  int oldtype = CANCEL_ASYNC ();
+
+  if ((pd == self
+       || (self->joinid == pd
+	   && (pd->cancelhandling
+	       & (CANCELING_BITMASK | CANCELED_BITMASK | EXITING_BITMASK
+		  | TERMINATED_BITMASK)) == 0))
+      && !CANCEL_ENABLED_AND_CANCELED (self->cancelhandling))
     /* This is a deadlock situation.  The threads are waiting for each
        other to finish.  Note that this is a "may" error.  To be 100%
        sure we catch this error we would have to lock the data
        structures but it is not necessary.  In the unlikely case that
        two threads are really caught in this situation they will
        deadlock.  It is the programmer's problem to figure this
        out.  */
-    return EDEADLK;
-
+    result = EDEADLK;
   /* Wait for the thread to finish.  If it is already locked something
      is wrong.  There can only be one waiter.  */
-  if (__builtin_expect (atomic_compare_and_exchange_bool_acq (&pd->joinid,
-							      self,
-							      NULL), 0))
+  else if (__builtin_expect (atomic_compare_and_exchange_bool_acq (&pd->joinid,
+								   self,
+								   NULL), 0))
     /* There is already somebody waiting for the thread.  */
-    return EINVAL;
-
-
-  /* During the wait we change to asynchronous cancellation.  If we
-     are cancelled the thread we are waiting for must be marked as
-     un-wait-ed for again.  */
-  pthread_cleanup_push (cleanup, &pd->joinid);
-
-  /* Switch to asynchronous cancellation.  */
-  int oldtype = CANCEL_ASYNC ();
-
-
-  /* Wait for the child.  */
-  lll_wait_tid (pd->tid);
+    result = EINVAL;
+  else
+    /* Wait for the child.  */
+    lll_wait_tid (pd->tid);
 
 
   /* Restore cancellation mode.  */
@@ -93,16 +96,19 @@ pthread_join (threadid, thread_return)
   pthread_cleanup_pop (0);
 
 
-  /* We mark the thread as terminated and as joined.  */
-  pd->tid = -1;
+  if (__builtin_expect (result == 0, 1))
+    {
+      /* We mark the thread as terminated and as joined.  */
+      pd->tid = -1;
 
-  /* Store the return value if the caller is interested.  */
-  if (thread_return != NULL)
-    *thread_return = pd->result;
+      /* Store the return value if the caller is interested.  */
+      if (thread_return != NULL)
+	*thread_return = pd->result;
 
 
-  /* Free the TCB.  */
-  __free_tcb (pd);
+      /* Free the TCB.  */
+      __free_tcb (pd);
+    }
 
-  return 0;
+  return result;
 }

nptl/tst-join5.c

@@ -1,4 +1,4 @@
-/* Copyright (C) 2003 Free Software Foundation, Inc.
+/* Copyright (C) 2003, 2006 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Ulrich Drepper <drepper@redhat.com>, 2003.
 
@@ -21,120 +21,187 @@
 #include <pthread.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <time.h>
+#include <unistd.h>
+#include <sys/syscall.h>
+
+
+#define wait_code()							      \
+  do {									      \
+    struct timespec ts = { .tv_sec = 0, .tv_nsec = 200000000 };		      \
+    while (syscall (__NR_nanosleep, &ts, &ts) < 0)			      \
+      /* nothing */;							      \
+  } while (0)
+
+
+#ifdef WAIT_IN_CHILD
+static pthread_barrier_t b;
+#endif
 
 
 static void *
 tf1 (void *arg)
 {
-  pthread_join ((pthread_t) arg, NULL);
+#ifdef WAIT_IN_CHILD
+  int e = pthread_barrier_wait (&b);
+  if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
+    {
+      printf ("%s: barrier_wait failed\n", __func__);
+      exit (1);
+    }
 
-  puts ("1st join returned");
+  wait_code ();
+#endif
 
-  return (void *) 1l;
+  pthread_join ((pthread_t) arg, NULL);
+
+  exit (42);
 }
 
 
 static void *
 tf2 (void *arg)
 {
-  pthread_join ((pthread_t) arg, NULL);
+#ifdef WAIT_IN_CHILD
+  int e = pthread_barrier_wait (&b);
+  if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
+    {
+      printf ("%s: barrier_wait failed\n", __func__);
+      exit (1);
+    }
 
-  puts ("2nd join returned");
+  wait_code ();
+#endif
+  pthread_join ((pthread_t) arg, NULL);
 
-  return (void *) 1l;
+  exit (43);
 }
 
 
 static int
 do_test (void)
 {
+#ifdef WAIT_IN_CHILD
+  if (pthread_barrier_init (&b, NULL, 2) != 0)
+    {
+      puts ("barrier_init failed");
+      return 1;
+    }
+#endif
+
   pthread_t th;
 
   int err = pthread_join (pthread_self (), NULL);
   if (err == 0)
     {
       puts ("1st circular join succeeded");
-      exit (1);
+      return 1;
     }
   if (err != EDEADLK)
     {
       printf ("1st circular join %d, not EDEADLK\n", err);
-      exit (1);
+      return 1;
     }
 
   if (pthread_create (&th, NULL, tf1, (void *) pthread_self ()) != 0)
     {
       puts ("1st create failed");
-      exit (1);
+      return 1;
     }
 
+#ifndef WAIT_IN_CHILD
+  wait_code ();
+#endif
+
   if (pthread_cancel (th) != 0)
     {
       puts ("cannot cancel 1st thread");
-      exit (1);
+      return 1;
+    }
+
+#ifdef WAIT_IN_CHILD
+  int e = pthread_barrier_wait (&b);
+  if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
+    {
+      printf ("%s: barrier_wait failed\n", __func__);
+      return 1;
     }
+#endif
 
   void *r;
   err = pthread_join (th, &r);
   if (err != 0)
     {
       printf ("cannot join 1st thread: %d\n", err);
-      exit (1);
+      return 1;
     }
   if (r != PTHREAD_CANCELED)
     {
       puts ("1st thread not canceled");
-      exit (1);
+      return 1;
     }
 
   err = pthread_join (pthread_self (), NULL);
   if (err == 0)
     {
       puts ("2nd circular join succeeded");
-      exit (1);
+      return 1;
     }
   if (err != EDEADLK)
     {
       printf ("2nd circular join %d, not EDEADLK\n", err);
-      exit (1);
+      return 1;
     }
 
   if (pthread_create (&th, NULL, tf2, (void *) pthread_self ()) != 0)
     {
       puts ("2nd create failed");
-      exit (1);
+      return 1;
     }
 
+#ifndef WAIT_IN_CHILD
+  wait_code ();
+#endif
+
   if (pthread_cancel (th) != 0)
     {
       puts ("cannot cancel 2nd thread");
-      exit (1);
+      return 1;
     }
 
+#ifdef WAIT_IN_CHILD
+  e = pthread_barrier_wait (&b);
+  if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
+    {
+      printf ("%s: barrier_wait failed\n", __func__);
+      return 1;
+    }
+#endif
+
   if (pthread_join (th, &r) != 0)
     {
       puts ("cannot join 2nd thread");
-      exit (1);
+      return 1;
     }
   if (r != PTHREAD_CANCELED)
     {
       puts ("2nd thread not canceled");
-      exit (1);
+      return 1;
     }
 
   err = pthread_join (pthread_self (), NULL);
   if (err == 0)
     {
-      puts ("2nd circular join succeeded");
-      exit (1);
+      puts ("3rd circular join succeeded");
+      return 1;
     }
   if (err != EDEADLK)
     {
-      printf ("2nd circular join %d, not EDEADLK\n", err);
-      exit (1);
+      printf ("3rd circular join %d, not EDEADLK\n", err);
+      return 1;
     }
 
-  exit (0);
+  return 0;
 }
 
 #define TEST_FUNCTION do_test ()

nptl/tst-join6.c

@@ -0,0 +1,2 @@
+#define WAIT_IN_CHILD 1
+#include "tst-join5.c"