This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [RFC] Add pthread_cond_timedwaitonclock_np
- From: Szabolcs Nagy <szabolcs dot nagy at arm dot com>
- To: Mike Crowe <mac at mcrowe dot com>, "libc-alpha at sourceware dot org" <libc-alpha at sourceware dot org>
- Date: Tue, 07 Jul 2015 15:28:56 +0100
- Subject: Re: [RFC] Add pthread_cond_timedwaitonclock_np
- Authentication-results: sourceware.org; auth=none
- References: <1436277389-22478-1-git-send-email-mac at mcrowe dot com>
On 07/07/15 14:56, Mike Crowe wrote:
> C++11's std::condition_variable::wait_until specifies the clock to be
> used at the time of the wait and permits separate waits on the same
> instance using different clocks.
>
> Unfortunately pthread_cond_timedwait always uses the clock that was
> specified (or defaulted) when pthread_cond_init was called. libstdc++'s
> current implementation converts all waits to
> std::chrono::system_clock (i.e. CLOCK_REALTIME) which can race against
> the system clock changing.
>
> Inventing a brand-new function pthread_cond_timedwaitonclock_np which
> accepts both the clock and the time point as parameters is
> straightforward and means that the C++11 standard behaviour can be
> implemented in libstdc++ on Linux at least.
>
do you have a reference to the rationale that introduced this api to C++?
this should be reported as an enhancement request to the austingroup
if there is real world need for it so all platforms implement it
consistently in the c runtime.
> (This patch is only to the C implementation of pthread_cond_timedwait so
> it is necessary to remove pthread_cond_timedwait.S and
> pthread_cond_wait.S to use it on x86 and x86_64 at the moment. If the
> approach is deemed acceptable I'll work on fixing those too along with
> the documentation.)
>
> Signed-off-by: Mike Crowe <mac@mcrowe.com>
> ---
> conform/data/pthread.h-data | 3 +
> nptl/Makefile | 4 +-
> nptl/Versions | 1 +
> nptl/pthread_cond_timedwait.c | 26 ++++--
> nptl/tst-cond11-onclock.c | 206 ++++++++++++++++++++++++++++++++++++++++++
> nptl/tst-cond5-onclock.c | 113 +++++++++++++++++++++++
> sysdeps/nptl/pthread.h | 13 +++
> 7 files changed, 356 insertions(+), 10 deletions(-)
> create mode 100644 nptl/tst-cond11-onclock.c
> create mode 100644 nptl/tst-cond5-onclock.c
>
> diff --git a/conform/data/pthread.h-data b/conform/data/pthread.h-data
> index c1e32c8..d52f298 100644
> --- a/conform/data/pthread.h-data
> +++ b/conform/data/pthread.h-data
> @@ -92,6 +92,9 @@ function int pthread_cond_destroy (pthread_cond_t*)
> function int pthread_cond_init (pthread_cond_t*, const pthread_condattr_t*)
> function int pthread_cond_signal (pthread_cond_t*)
> function int pthread_cond_timedwait (pthread_cond_t*, pthread_mutex_t*, const struct timespec*)
> +#if !defined POSIX && !defined UNIX98 && !defined XOPEN2K
> +function int pthread_cond_timedwaitonclock_np (pthread_cond_t*, pthread_mutex_t*, clockid_t, const struct timespec*)
> +#endif
> function int pthread_cond_wait (pthread_cond_t*, pthread_mutex_t*)
> function int pthread_condattr_destroy (pthread_condattr_t*)
> #if !defined POSIX && !defined UNIX98 && !defined XOPEN2K
> diff --git a/nptl/Makefile b/nptl/Makefile
> index 4544aa2..ff06939 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -221,8 +221,8 @@ tests = tst-typesizes \
> tst-mutexpi5a tst-mutexpi6 tst-mutexpi7 tst-mutexpi7a tst-mutexpi8 \
> tst-mutexpi9 \
> tst-spin1 tst-spin2 tst-spin3 tst-spin4 \
> - 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-cond1 tst-cond2 tst-cond3 tst-cond4 tst-cond5 tst-cond5-onclock tst-cond6 tst-cond7 \
> + tst-cond8 tst-cond9 tst-cond10 tst-cond11 tst-cond11-onclock tst-cond12 tst-cond13 \
> tst-cond14 tst-cond15 tst-cond16 tst-cond17 tst-cond18 tst-cond19 \
> tst-cond20 tst-cond21 tst-cond22 tst-cond23 tst-cond24 tst-cond25 \
> tst-cond-except \
> diff --git a/nptl/Versions b/nptl/Versions
> index 34e4b46..476301f 100644
> --- a/nptl/Versions
> +++ b/nptl/Versions
> @@ -267,6 +267,7 @@ libpthread {
> }
>
> GLIBC_2.22 {
> + pthread_cond_timedwaitonclock_np;
> }
>
> GLIBC_PRIVATE {
> diff --git a/nptl/pthread_cond_timedwait.c b/nptl/pthread_cond_timedwait.c
> index 10b0a61..a4c0ee3 100644
> --- a/nptl/pthread_cond_timedwait.c
> +++ b/nptl/pthread_cond_timedwait.c
> @@ -49,8 +49,9 @@ struct _condvar_cleanup_buffer
> };
>
> int
> -__pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
> - const struct timespec *abstime)
> +pthread_cond_timedwaitonclock_np (pthread_cond_t *cond, pthread_mutex_t *mutex,
> + clockid_t clockid,
> + const struct timespec *abstime)
> {
> struct _pthread_cleanup_buffer buffer;
> struct _condvar_cleanup_buffer cbuffer;
> @@ -120,8 +121,7 @@ __pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
> # ifdef __NR_clock_gettime
> INTERNAL_SYSCALL_DECL (err);
> (void) INTERNAL_VSYSCALL (clock_gettime, err, 2,
> - (cond->__data.__nwaiters
> - & ((1 << COND_NWAITERS_SHIFT) - 1)),
> + clockid,
> &rt);
> /* Convert the absolute timeout value to a relative timeout. */
> rt.tv_sec = abstime->tv_sec - rt.tv_sec;
> @@ -175,8 +175,7 @@ __pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
>
> if (pi_flag)
> {
> - unsigned int clockbit = (cond->__data.__nwaiters & 1
> - ? 0 : FUTEX_CLOCK_REALTIME);
> + unsigned int clockbit = clockid ? 0 : FUTEX_CLOCK_REALTIME;
> err = lll_futex_timed_wait_requeue_pi (&cond->__data.__futex,
> futex_val, abstime, clockbit,
> &mutex->__data.__lock,
> @@ -193,8 +192,7 @@ __pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
> err = lll_futex_timed_wait (&cond->__data.__futex,
> futex_val, &rt, pshared);
> #else
> - unsigned int clockbit = (cond->__data.__nwaiters & 1
> - ? 0 : FUTEX_CLOCK_REALTIME);
> + unsigned int clockbit = clockid ? 0 : FUTEX_CLOCK_REALTIME;
> err = lll_futex_timed_wait_bitset (&cond->__data.__futex, futex_val,
> abstime, clockbit, pshared);
> #endif
> @@ -264,5 +262,17 @@ __pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
> return err ?: result;
> }
>
> +int
> +__pthread_cond_timedwait (cond, mutex, abstime)
> + pthread_cond_t *cond;
> + pthread_mutex_t *mutex;
> + const struct timespec *abstime;
> +{
> + return pthread_cond_timedwaitonclock_np (cond, mutex,
> + (cond->__data.__nwaiters
> + & ((1 << COND_NWAITERS_SHIFT) - 1)),
> + abstime);
> +}
> +
> versioned_symbol (libpthread, __pthread_cond_timedwait, pthread_cond_timedwait,
> GLIBC_2_3_2);
> diff --git a/nptl/tst-cond11-onclock.c b/nptl/tst-cond11-onclock.c
> new file mode 100644
> index 0000000..15b3730
> --- /dev/null
> +++ b/nptl/tst-cond11-onclock.c
> @@ -0,0 +1,206 @@
> +/* Copyright (C) 2003-2014 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> + Contributed by Ulrich Drepper <drepper@redhat.com>, 2003.
> +
> + 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, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <errno.h>
> +#include <pthread.h>
> +#include <stdio.h>
> +#include <time.h>
> +#include <unistd.h>
> +
> +
> +#if defined _POSIX_CLOCK_SELECTION && _POSIX_CLOCK_SELECTION >= 0
> +static int
> +run_test (clockid_t attr_clock, clockid_t wait_clock)
> +{
> + pthread_condattr_t condattr;
> + pthread_cond_t cond;
> + pthread_mutexattr_t mutattr;
> + pthread_mutex_t mut;
> +
> + printf ("attr_clock = %d, wait_clock = %d\n", (int) attr_clock, (int) wait_clock);
> +
> + if (pthread_condattr_init (&condattr) != 0)
> + {
> + puts ("condattr_init failed");
> + return 1;
> + }
> +
> + if (pthread_condattr_setclock (&condattr, attr_clock) != 0)
> + {
> + puts ("condattr_setclock failed");
> + return 1;
> + }
> +
> + clockid_t cl2;
> + if (pthread_condattr_getclock (&condattr, &cl2) != 0)
> + {
> + puts ("condattr_getclock failed");
> + return 1;
> + }
> + if (attr_clock != cl2)
> + {
> + printf ("condattr_getclock returned wrong value: %d, expected %d\n",
> + (int) cl2, (int) attr_clock);
> + return 1;
> + }
> +
> + if (pthread_cond_init (&cond, &condattr) != 0)
> + {
> + puts ("cond_init failed");
> + return 1;
> + }
> +
> + if (pthread_condattr_destroy (&condattr) != 0)
> + {
> + puts ("condattr_destroy failed");
> + return 1;
> + }
> +
> + if (pthread_mutexattr_init (&mutattr) != 0)
> + {
> + puts ("mutexattr_init failed");
> + return 1;
> + }
> +
> + if (pthread_mutexattr_settype (&mutattr, PTHREAD_MUTEX_ERRORCHECK) != 0)
> + {
> + puts ("mutexattr_settype failed");
> + return 1;
> + }
> +
> + if (pthread_mutex_init (&mut, &mutattr) != 0)
> + {
> + puts ("mutex_init failed");
> + return 1;
> + }
> +
> + if (pthread_mutexattr_destroy (&mutattr) != 0)
> + {
> + puts ("mutexattr_destroy failed");
> + return 1;
> + }
> +
> + if (pthread_mutex_lock (&mut) != 0)
> + {
> + puts ("mutex_lock failed");
> + return 1;
> + }
> +
> + if (pthread_mutex_lock (&mut) != EDEADLK)
> + {
> + puts ("2nd mutex_lock did not return EDEADLK");
> + return 1;
> + }
> +
> + struct timespec ts;
> + if (clock_gettime (wait_clock, &ts) != 0)
> + {
> + puts ("clock_gettime failed");
> + return 1;
> + }
> +
> + /* Wait one second. */
> + ++ts.tv_sec;
> +
> + int e = pthread_cond_timedwaitonclock_np (&cond, &mut, wait_clock, &ts);
> + if (e == 0)
> + {
> + puts ("cond_timedwait succeeded");
> + return 1;
> + }
> + else if (e != ETIMEDOUT)
> + {
> + puts ("cond_timedwait did not return ETIMEDOUT");
> + return 1;
> + }
> +
> + struct timespec ts2;
> + if (clock_gettime (wait_clock, &ts2) != 0)
> + {
> + puts ("second clock_gettime failed");
> + return 1;
> + }
> +
> + if (ts2.tv_sec < ts.tv_sec
> + || (ts2.tv_sec == ts.tv_sec && ts2.tv_nsec < ts.tv_nsec))
> + {
> + puts ("timeout too short");
> + return 1;
> + }
> +
> + if (pthread_mutex_unlock (&mut) != 0)
> + {
> + puts ("mutex_unlock failed");
> + return 1;
> + }
> +
> + if (pthread_mutex_destroy (&mut) != 0)
> + {
> + puts ("mutex_destroy failed");
> + return 1;
> + }
> +
> + if (pthread_cond_destroy (&cond) != 0)
> + {
> + puts ("cond_destroy failed");
> + return 1;
> + }
> +
> + return 0;
> +}
> +#endif
> +
> +
> +static int
> +do_test (void)
> +{
> +#if !defined _POSIX_CLOCK_SELECTION || _POSIX_CLOCK_SELECTION == -1
> +
> + puts ("_POSIX_CLOCK_SELECTION not supported, test skipped");
> + return 0;
> +
> +#else
> +
> + int res = run_test (CLOCK_REALTIME, CLOCK_REALTIME);
> +
> +# if defined _POSIX_MONOTONIC_CLOCK && _POSIX_MONOTONIC_CLOCK >= 0
> +# if _POSIX_MONOTONIC_CLOCK == 0
> + int e = sysconf (_SC_MONOTONIC_CLOCK);
> + if (e < 0)
> + puts ("CLOCK_MONOTONIC not supported");
> + else if (e == 0)
> + {
> + puts ("sysconf (_SC_MONOTONIC_CLOCK) must not return 0");
> + res = 1;
> + }
> + else
> +# endif
> + res |= run_test (CLOCK_REALTIME, CLOCK_MONOTONIC);
> + res |= run_test (CLOCK_MONOTONIC, CLOCK_MONOTONIC);
> + res |= run_test (CLOCK_MONOTONIC, CLOCK_REALTIME);
> +# else
> + puts ("_POSIX_MONOTONIC_CLOCK not defined");
> +# endif
> +
> + return res;
> +#endif
> +}
> +
> +#define TIMEOUT 12
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
> diff --git a/nptl/tst-cond5-onclock.c b/nptl/tst-cond5-onclock.c
> new file mode 100644
> index 0000000..ddb0ad1
> --- /dev/null
> +++ b/nptl/tst-cond5-onclock.c
> @@ -0,0 +1,113 @@
> +/* Copyright (C) 2002-2014 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> + Contributed by Ulrich Drepper <drepper@redhat.com>, 2002.
> +
> + 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, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <errno.h>
> +#include <pthread.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <time.h>
> +#include <sys/time.h>
> +
> +
> +static pthread_mutex_t mut;
> +static pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
> +
> +
> +static int
> +do_test_onclock(clockid_t clockid)
> +{
> + pthread_mutexattr_t ma;
> + int err;
> + struct timespec ts;
> +
> + if (pthread_mutexattr_init (&ma) != 0)
> + {
> + puts ("mutexattr_init failed");
> + exit (1);
> + }
> +
> + if (pthread_mutexattr_settype (&ma, PTHREAD_MUTEX_ERRORCHECK) != 0)
> + {
> + puts ("mutexattr_settype failed");
> + exit (1);
> + }
> +
> + if (pthread_mutex_init (&mut, &ma) != 0)
> + {
> + puts ("mutex_init failed");
> + exit (1);
> + }
> +
> + /* Get the mutex. */
> + if (pthread_mutex_lock (&mut) != 0)
> + {
> + puts ("mutex_lock failed");
> + exit (1);
> + }
> +
> + /* Waiting for the condition will fail. But we want the timeout here. */
> + if (clock_gettime (clockid, &ts) != 0)
> + {
> + puts ("clock_gettime failed");
> + exit (1);
> + }
> +
> + ts.tv_nsec += 500000000;
> + if (ts.tv_nsec >= 1000000000)
> + {
> + ts.tv_nsec -= 1000000000;
> + ++ts.tv_sec;
> + }
> + err = pthread_cond_timedwaitonclock_np (&cond, &mut, clockid, &ts);
> + if (err == 0)
> + {
> + /* This could in theory happen but here without any signal and
> + additional waiter it should not. */
> + puts ("cond_timedwait succeeded");
> + exit (1);
> + }
> + else if (err != ETIMEDOUT)
> + {
> + printf ("cond_timedwait returned with %s\n", strerror (err));
> + exit (1);
> + }
> +
> + err = pthread_mutex_unlock (&mut);
> + if (err != 0)
> + {
> + printf ("mutex_unlock failed: %s\n", strerror (err));
> + exit (1);
> + }
> +
> + return 0;
> +}
> +
> +static int
> +do_test (void)
> +{
> + int rc;
> + rc = do_test_onclock(CLOCK_MONOTONIC);
> + if (rc == 0)
> + rc = do_test_onclock(CLOCK_REALTIME);
> +
> + return rc;
> +}
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
> diff --git a/sysdeps/nptl/pthread.h b/sysdeps/nptl/pthread.h
> index 89d0882..221924b 100644
> --- a/sysdeps/nptl/pthread.h
> +++ b/sysdeps/nptl/pthread.h
> @@ -1002,6 +1002,19 @@ extern int pthread_cond_timedwait (pthread_cond_t *__restrict __cond,
> const struct timespec *__restrict __abstime)
> __nonnull ((1, 2, 3));
>
> +/* Wait for condition variable COND to be signaled or broadcast until
> + ABSTIME measured by the specified clock. MUTEX is assumed to be
> + locked before. CLOCK is a clock specified. ABSTIME is an absolute
> + time specification against CLOCK's epoch.
> +
> + This function is a cancellation point and therefore not marked with
> + __THROW. */
> +extern int pthread_cond_timedwaitonclock_np (pthread_cond_t *__restrict __cond,
> + pthread_mutex_t *__restrict __mutex,
> + __clockid_t __clock_id,
> + const struct timespec *__restrict __abstime)
> + __nonnull ((1, 2, 4));
> +
> /* Functions for handling condition variable attributes. */
>
> /* Initialize condition variable attribute ATTR. */
> --
> 2.1.4
>
>
>