This is the mail archive of the libc-hacker@sources.redhat.com mailing list for the glibc project.
Note that libc-hacker is a closed list. You may look at the archives of this list, but subscription and posting are not open.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
Hi! This is just a RFC patch, I was lazy and have not updated i386/x86_64 assembly, just the arches using sysdeps/pthread/pthread_cond*.c. http://www.opengroup.org/onlinepubs/009695399/functions/pthread_cond_destroy.html has: It shall be safe to destroy an initialized condition variable upon which no threads are currently blocked. Attempting to destroy a condition variable upon which other threads are currently blocked results in undefined behavior. which is not that clear if a pthread_cond_wait'ing thread where pthread_cond_broadcast/pthread_cond_signal has been already called to wake it up must be considered as no longer blocked. But later on in the informative section: A condition variable can be destroyed immediately after all the threads that are blocked on it are awakened. For example, consider the following code: and the example is basically what tst-cond20.c below does. As we are out of bits in at least 64-bit pthread_cond_t, I chose to use just one bit of the former __clock (nor __nwaiters) for the clock type (pthread_condattr_setclock allows CLOCK_REALTIME/CLOCK_MONOTONIC only) and the remaining bits for the current number of waiters. pthread_cond_destroy will with this patch return EBUSY if it sees waiters that have not a corresponding pthread_cond_signal or broadcast called after them, and if it sees waiters that were already woken up, but have not reacquired the condvar's internal lock yet and still will need to use the pthread_cond_t structure, it waits for all of them to leave. Tested on ppc32. 2004-08-31 Jakub Jelinek <jakub@redhat.com> [BZ #342] * Makefile (tests): Add tst-cond20. * tst-cond20.c: New test. * sysdeps/unix/sysv/linux/alpha/bits/pthreadtypes.h (pthread_cond_t): Rename __data.__clock to __data.__nwaiters, make it unsigned int. * sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h (pthread_cond_t): Likewise. * sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h (pthread_cond_t): Likewise. * sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h (pthread_cond_t): Likewise. * sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h (pthread_cond_t): Likewise. * sysdeps/unix/sysv/linux/ia64/bits/pthreadtypes.h (pthread_cond_t): Likewise. * sysdeps/unix/sysv/linux/lowlevelcond.sym (cond_clock): Remove. (cond_nwaiters): New. * pthread_cond_destroy.c (__pthread_cond_destroy): Return EBUSY if there are waiters not signalled yet. Wait until all already signalled waiters wake up. * sysdeps/pthread/pthread_cond_wait.c (__condvar_cleanup): Decrement __nwaiters. If pthread_cond_destroy has been called and this is the last waiter, signal pthread_cond_destroy caller and avoid using the pthread_cond_t structure after unlock. (__pthread_cond_wait): Increment __nwaiters in the beginning, decrement it when leaving. If pthread_cond_destroy has been called and this is the last waiter, signal pthread_cond_destroy caller. * sysdeps/pthread/pthread_cond_timedwait.c (__pthread_cond_timedwait): Likewise. Read clock type from the least significant bit of __nwaiters instead of __clock. * pthread_condattr_setclock.c (pthread_condattr_setclock): Encode clock type just in the second bit of value. * pthread_condattr_getclock.c (pthread_condattr_getclock): Decode clock type just from the second bit of value. * pthread_cond_init.c (__pthread_cond_init): Initialize __nwaiters instead of __clock, just from second bit of condattr's value. --- libc/nptl/tst-cond20.c.jj 2004-08-30 13:57:20.000000000 +0200 +++ libc/nptl/tst-cond20.c 2004-08-30 15:54:47.000000000 +0200 @@ -0,0 +1,160 @@ +/* Copyright (C) 2004 Free Software Foundation, Inc. + This file is part of the GNU C Library. + Contributed by Jakub Jelinek <jakub@redhat.com>, 2004. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, write to the Free + Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA + 02111-1307 USA. */ + +#include <pthread.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> + +#define N 10 +#define ROUNDS 1000 +static pthread_cond_t cond = PTHREAD_COND_INITIALIZER; +static pthread_cond_t cond2 = PTHREAD_COND_INITIALIZER; +static pthread_mutex_t mut = PTHREAD_MUTEX_INITIALIZER; +static pthread_barrier_t b; +static int count; + +static void * +tf (void *p) +{ + int i; + for (i = 0; i < ROUNDS; ++i) + { + pthread_mutex_lock (&mut); + + if (++count == N) + pthread_cond_signal (&cond2); + + pthread_cond_wait (&cond, &mut); + + pthread_mutex_unlock (&mut); + + int err = pthread_barrier_wait (&b); + if (err != 0 && err != PTHREAD_BARRIER_SERIAL_THREAD) + { + puts ("child: barrier_wait failed"); + exit (1); + } + + err = pthread_barrier_wait (&b); + if (err != 0 && err != PTHREAD_BARRIER_SERIAL_THREAD) + { + puts ("child: barrier_wait failed"); + exit (1); + } + } + + return NULL; +} + + +static int +do_test (void) +{ + if (pthread_barrier_init (&b, NULL, N + 1) != 0) + { + puts ("barrier_init failed"); + return 1; + } + + pthread_mutex_lock (&mut); + + int i, j, err; + pthread_t th[N]; + for (i = 0; i < N; ++i) + if ((err = pthread_create (&th[i], NULL, tf, NULL)) != 0) + { + printf ("cannot create thread %d: %s\n", i, strerror (err)); + return 1; + } + + for (i = 0; i < ROUNDS; ++i) + { + pthread_cond_wait (&cond2, &mut); + + if (i & 1) + pthread_mutex_unlock (&mut); + + if (i & 2) + pthread_cond_broadcast (&cond); + else if (i & 4) + for (j = 0; j < N; ++j) + pthread_cond_signal (&cond); + else + { + for (j = 0; j < (i / 8) % N; ++j) + pthread_cond_signal (&cond); + pthread_cond_broadcast (&cond); + } + + if ((i & 1) == 0) + pthread_mutex_unlock (&mut); + + err = pthread_cond_destroy (&cond); + if (err) + { + printf ("pthread_cond_destroy failed: %s\n", strerror (err)); + return 1; + } + + /* Now clobber the cond variable which has been successfully + destroyed above. */ + memset (&cond, (char) i, sizeof (cond)); + + err = pthread_barrier_wait (&b); + if (err != 0 && err != PTHREAD_BARRIER_SERIAL_THREAD) + { + puts ("parent: barrier_wait failed"); + return 1; + } + + pthread_mutex_lock (&mut); + + err = pthread_barrier_wait (&b); + if (err != 0 && err != PTHREAD_BARRIER_SERIAL_THREAD) + { + puts ("parent: barrier_wait failed"); + return 1; + } + + count = 0; + err = pthread_cond_init (&cond, NULL); + if (err) + { + printf ("pthread_cond_init failed: %s\n", strerror (err)); + return 1; + } + } + + for (i = 0; i < N; ++i) + if ((err = pthread_join (th[i], NULL)) != 0) + { + printf ("failed to join thread %d: %s\n", i, strerror (err)); + return 1; + } + + puts ("done"); + + return 0; +} + + +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c" --- libc/nptl/sysdeps/unix/sysv/linux/alpha/bits/pthreadtypes.h.jj 2004-06-11 14:45:52.000000000 +0200 +++ libc/nptl/sysdeps/unix/sysv/linux/alpha/bits/pthreadtypes.h 2004-08-31 11:07:23.268203397 +0200 @@ -81,7 +81,7 @@ typedef union unsigned long long int __wakeup_seq; unsigned long long int __woken_seq; void *__mutex; - int __clock; + unsigned int __nwaiters; unsigned int __broadcast_seq; } __data; char __size[__SIZEOF_PTHREAD_COND_T]; --- libc/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h.jj 2004-06-11 14:45:53.000000000 +0200 +++ libc/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h 2004-08-31 11:07:29.136146174 +0200 @@ -100,7 +100,7 @@ typedef union unsigned long long int __wakeup_seq; unsigned long long int __woken_seq; void *__mutex; - int __clock; + unsigned int __nwaiters; unsigned int __broadcast_seq; } __data; char __size[__SIZEOF_PTHREAD_COND_T]; --- libc/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h.jj 2004-06-11 14:45:53.000000000 +0200 +++ libc/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h 2004-08-31 09:34:46.134724009 +0200 @@ -101,7 +101,7 @@ typedef union unsigned long long int __wakeup_seq; unsigned long long int __woken_seq; void *__mutex; - int __clock; + unsigned int __nwaiters; unsigned int __broadcast_seq; } __data; char __size[__SIZEOF_PTHREAD_COND_T]; --- libc/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h.jj 2004-06-11 14:45:53.000000000 +0200 +++ libc/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h 2004-08-31 11:07:34.984092621 +0200 @@ -101,7 +101,7 @@ typedef union unsigned long long int __wakeup_seq; unsigned long long int __woken_seq; void *__mutex; - int __clock; + unsigned int __nwaiters; unsigned int __broadcast_seq; } __data; char __size[__SIZEOF_PTHREAD_COND_T]; --- libc/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h.jj 2004-06-18 13:23:43.000000000 +0200 +++ libc/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h 2004-08-31 11:07:41.428931611 +0200 @@ -82,7 +82,7 @@ typedef union unsigned long long int __wakeup_seq; unsigned long long int __woken_seq; void *__mutex; - int __clock; + unsigned int __nwaiters; unsigned int __broadcast_seq; } __data; char __size[__SIZEOF_PTHREAD_COND_T]; --- libc/nptl/sysdeps/unix/sysv/linux/ia64/bits/pthreadtypes.h.jj 2004-06-11 14:45:53.000000000 +0200 +++ libc/nptl/sysdeps/unix/sysv/linux/ia64/bits/pthreadtypes.h 2004-08-31 11:07:48.933579783 +0200 @@ -81,7 +81,7 @@ typedef union unsigned long long int __wakeup_seq; unsigned long long int __woken_seq; void *__mutex; - int __clock; + unsigned int __nwaiters; unsigned int __broadcast_seq; } __data; char __size[__SIZEOF_PTHREAD_COND_T]; --- libc/nptl/sysdeps/unix/sysv/linux/lowlevelcond.sym.jj 2004-08-31 09:42:07.257565326 +0200 +++ libc/nptl/sysdeps/unix/sysv/linux/lowlevelcond.sym 2004-08-31 09:42:24.334540875 +0200 @@ -5,7 +5,7 @@ cond_lock offsetof (pthread_cond_t, __data.__lock) cond_futex offsetof (pthread_cond_t, __data.__futex) -cond_clock offsetof (pthread_cond_t, __data.__clock) +cond_nwaiters offsetof (pthread_cond_t, __data.__nwaiters) total_seq offsetof (pthread_cond_t, __data.__total_seq) wakeup_seq offsetof (pthread_cond_t, __data.__wakeup_seq) woken_seq offsetof (pthread_cond_t, __data.__woken_seq) --- libc/nptl/sysdeps/pthread/pthread_cond_timedwait.c.jj 2004-06-18 13:23:41.000000000 +0200 +++ libc/nptl/sysdeps/pthread/pthread_cond_timedwait.c 2004-08-31 10:39:15.175944516 +0200 @@ -67,6 +67,7 @@ __pthread_cond_timedwait (cond, mutex, a /* We have one new user of the condvar. */ ++cond->__data.__total_seq; ++cond->__data.__futex; + cond->__data.__nwaiters += 2; /* Remember the mutex we are using here. If there is already a different address store this is a bad user bug. Do not store @@ -98,7 +99,8 @@ __pthread_cond_timedwait (cond, mutex, a INTERNAL_SYSCALL_DECL (err); int ret; ret = INTERNAL_SYSCALL (clock_gettime, err, 2, - cond->__data.__clock, &rt); + (cond->__data.__nwaiters & 1) + ? CLOCK_MONOTONIC : CLOCK_REALTIME, &rt); # ifndef __ASSUME_POSIX_TIMERS if (__builtin_expect (INTERNAL_SYSCALL_ERROR_P (ret, err), 0)) { @@ -185,6 +187,15 @@ __pthread_cond_timedwait (cond, mutex, a ++cond->__data.__woken_seq; bc_out: + + cond->__data.__nwaiters -= 2; + + /* If pthread_cond_destroy was called on this variable already, + notify the pthread_cond_destroy caller all waiters have left + and it can be successfully destroyed. */ + if (cond->__data.__total_seq == -1ULL && cond->__data.__nwaiters < 2) + lll_futex_wake (&cond->__data.__nwaiters, 1); + /* We are done with the condvar. */ lll_mutex_unlock (cond->__data.__lock); --- libc/nptl/sysdeps/pthread/pthread_cond_wait.c.jj 2004-06-11 14:45:52.000000000 +0200 +++ libc/nptl/sysdeps/pthread/pthread_cond_wait.c 2004-08-31 10:39:20.240044128 +0200 @@ -42,6 +42,7 @@ __condvar_cleanup (void *arg) { struct _condvar_cleanup_buffer *cbuffer = (struct _condvar_cleanup_buffer *) arg; + unsigned int destroying; /* We are going to modify shared data. */ lll_mutex_lock (cbuffer->cond->__data.__lock); @@ -55,11 +56,25 @@ __condvar_cleanup (void *arg) ++cbuffer->cond->__data.__futex; } + cbuffer->cond->__data.__nwaiters -= 2; + + /* If pthread_cond_destroy was called on this variable already, + notify the pthread_cond_destroy caller all waiters have left + and it can be successfully destroyed. */ + destroying = 0; + if (cbuffer->cond->__data.__total_seq == -1ULL + && cbuffer->cond->__data.__nwaiters < 2) + { + lll_futex_wake (&cbuffer->cond->__data.__nwaiters, 1); + destroying = 1; + } + /* We are done. */ lll_mutex_unlock (cbuffer->cond->__data.__lock); /* Wake everybody to make sure no condvar signal gets lost. */ - lll_futex_wake (&cbuffer->cond->__data.__futex, INT_MAX); + if (! destroying) + lll_futex_wake (&cbuffer->cond->__data.__futex, INT_MAX); /* Get the mutex before returning unless asynchronous cancellation is in effect. */ @@ -90,6 +105,7 @@ __pthread_cond_wait (cond, mutex) /* We have one new user of the condvar. */ ++cond->__data.__total_seq; ++cond->__data.__futex; + cond->__data.__nwaiters += 2; /* Remember the mutex we are using here. If there is already a different address store this is a bad user bug. Do not store @@ -145,6 +161,15 @@ __pthread_cond_wait (cond, mutex) ++cond->__data.__woken_seq; bc_out: + + cond->__data.__nwaiters -= 2; + + /* If pthread_cond_destroy was called on this varaible already, + notify the pthread_cond_destroy caller all waiters have left + and it can be successfully destroyed. */ + if (cond->__data.__total_seq == -1ULL && cond->__data.__nwaiters < 2) + lll_futex_wake (&cond->__data.__nwaiters, 1); + /* We are done with the condvar. */ lll_mutex_unlock (cond->__data.__lock); --- libc/nptl/pthread_condattr_setclock.c.jj 2003-07-31 21:12:48.000000000 +0200 +++ libc/nptl/pthread_condattr_setclock.c 2004-08-31 09:40:23.132008665 +0200 @@ -1,4 +1,4 @@ -/* Copyright (C) 2003 Free Software Foundation, Inc. +/* Copyright (C) 2003, 2004 Free Software Foundation, Inc. This file is part of the GNU C Library. Contributed by Ulrich Drepper <drepper@redhat.com>, 2003. @@ -61,7 +61,7 @@ pthread_condattr_setclock (attr, clock_i int *valuep = &((struct pthread_condattr *) attr)->value; - *valuep = (*valuep & ~0xfe) | (clock_id << 1); + *valuep = (*valuep & ~2) | ((clock_id == CLOCK_MONOTONIC) << 1); return 0; } --- libc/nptl/pthread_condattr_getclock.c.jj 2003-03-18 12:06:28.000000000 +0100 +++ libc/nptl/pthread_condattr_getclock.c 2004-08-31 09:39:55.739861102 +0200 @@ -1,4 +1,4 @@ -/* Copyright (C) 2003 Free Software Foundation, Inc. +/* Copyright (C) 2003, 2004 Free Software Foundation, Inc. This file is part of the GNU C Library. Contributed by Ulrich Drepper <drepper@redhat.com>, 2003. @@ -25,7 +25,7 @@ pthread_condattr_getclock (attr, clock_i const pthread_condattr_t *attr; clockid_t *clock_id; { - *clock_id = ((((const struct pthread_condattr *) attr)->value) & 0xfe) >> 1; - + *clock_id = ((((const struct pthread_condattr *) attr)->value) & 2) + ? CLOCK_MONOTONIC : CLOCK_REALTIME; return 0; } --- libc/nptl/pthread_cond_destroy.c.jj 2003-01-03 02:33:29.000000000 +0100 +++ libc/nptl/pthread_cond_destroy.c 2004-08-31 11:06:11.911065291 +0200 @@ -1,4 +1,4 @@ -/* Copyright (C) 2002, 2003 Free Software Foundation, Inc. +/* Copyright (C) 2002, 2003, 2004 Free Software Foundation, Inc. This file is part of the GNU C Library. Contributed by Ulrich Drepper <drepper@redhat.com>, 2002. @@ -25,6 +25,34 @@ int __pthread_cond_destroy (cond) pthread_cond_t *cond; { + /* Make sure we are along. */ + lll_mutex_lock (cond->__data.__lock); + + if (cond->__data.__nwaiters >= 2 + && cond->__data.__total_seq > cond->__data.__wakeup_seq) + { + /* If there are still some waiters which have not been + woken up, this is an application bug. */ + lll_mutex_unlock (cond->__data.__lock); + return EBUSY; + } + + /* Tell pthread_cond_*wait that this condvar is being destroyed. */ + cond->__data.__total_seq = -1ULL; + + /* If there are waiters which have been already signalled or + broadcasted, but still are using the pthread_cond_t structure, + pthread_cond_destroy needs to wait for them. */ + while (cond->__data.__nwaiters >= 2) + { + unsigned int nwaiters = cond->__data.__nwaiters; + lll_mutex_unlock (cond->__data.__lock); + + lll_futex_wait (&cond->__data.__nwaiters, nwaiters); + + lll_mutex_lock (cond->__data.__lock); + } + return 0; } versioned_symbol (libpthread, __pthread_cond_destroy, --- libc/nptl/Makefile.jj 2004-08-04 14:16:09.000000000 +0200 +++ libc/nptl/Makefile 2004-08-31 10:40:12.353779078 +0200 @@ -193,6 +193,7 @@ tests = tst-attr1 tst-attr2 tst-attr3 \ tst-cond1 tst-cond2 tst-cond3 tst-cond4 tst-cond5 tst-cond6 tst-cond7 \ tst-cond8 tst-cond9 tst-cond10 tst-cond11 tst-cond12 tst-cond13 \ tst-cond14 tst-cond15 tst-cond16 tst-cond17 tst-cond18 tst-cond19 \ + tst-cond20 \ tst-rwlock1 tst-rwlock2 tst-rwlock3 tst-rwlock4 tst-rwlock5 \ tst-rwlock6 tst-rwlock7 tst-rwlock8 tst-rwlock9 tst-rwlock10 \ tst-rwlock11 tst-rwlock12 tst-rwlock13 tst-rwlock14 \ --- libc/nptl/pthread_cond_init.c.jj 2004-06-11 14:45:51.000000000 +0200 +++ libc/nptl/pthread_cond_init.c 2004-08-31 09:41:32.135785921 +0200 @@ -32,8 +32,7 @@ __pthread_cond_init (cond, cond_attr) cond->__data.__lock = LLL_MUTEX_LOCK_INITIALIZER; cond->__data.__futex = 0; - cond->__data.__clock = (icond_attr == NULL - ? CLOCK_REALTIME : (icond_attr->value & 0xfe) >> 1); + cond->__data.__nwaiters = (icond_attr != NULL && (icond_attr->value & 2)); cond->__data.__total_seq = 0; cond->__data.__wakeup_seq = 0; cond->__data.__woken_seq = 0; Jakub
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |