[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.

5 files changed, 148 insertions, 61 deletions

diff --git a/nptl/ChangeLog b/nptl/ChangeLog
index f5243c8091..8e156c5299 100644
--- a/nptl/ChangeLog
+++ b/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]
diff --git a/nptl/Makefile b/nptl/Makefile
index d0f8286fb8..cce512f33b 100644
--- a/nptl/Makefile
+++ b/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 \
diff --git a/nptl/pthread_join.c b/nptl/pthread_join.c
index 70dc81a023..c88d85b52f 100644
--- a/nptl/pthread_join.c
+++ b/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,12 +52,23 @@ 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
@@ -62,28 +76,17 @@ pthread_join (threadid, thread_return)
        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;
 }
diff --git a/nptl/tst-join5.c b/nptl/tst-join5.c
index 589fac6b5f..db005f5b71 100644
--- a/nptl/tst-join5.c
+++ b/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 ()
diff --git a/nptl/tst-join6.c b/nptl/tst-join6.c
new file mode 100644
index 0000000000..0c9e7c056b
--- /dev/null
+++ b/nptl/tst-join6.c
@@ -0,0 +1,2 @@
+#define WAIT_IN_CHILD 1
+#include "tst-join5.c"