From 22bb134c318365dbac664e46c193538c14bf3c42 Mon Sep 17 00:00:00 2001 From: Ulrich Drepper Date: Sun, 13 Aug 2006 01:56:09 +0000 Subject: [PATCH] [BZ #2843] 2006-08-12 Ulrich Drepper [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 | 12 +++++ nptl/Makefile | 2 +- nptl/pthread_join.c | 78 ++++++++++++++++-------------- nptl/tst-join5.c | 115 +++++++++++++++++++++++++++++++++++--------- nptl/tst-join6.c | 2 + 5 files changed, 148 insertions(+), 61 deletions(-) create mode 100644 nptl/tst-join6.c 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 + + [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 [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 , 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 , 2003. @@ -21,120 +21,187 @@ #include #include #include +#include +#include +#include + + +#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) { +#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); + } + + wait_code (); +#endif + pthread_join ((pthread_t) arg, NULL); - puts ("1st join returned"); - - return (void *) 1l; + exit (42); } static void * tf2 (void *arg) { +#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); + } + + wait_code (); +#endif pthread_join ((pthread_t) arg, NULL); - puts ("2nd join returned"); - - 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"