[RFCv4] Add pthread_cond_timedwaitonclock_np

[Mike Crowe]

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 condition
variable 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.

Let's invent 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.

Earlier versions of this patch were proposed in 2015 but further progress
stalled waiting for the assembly implementation of pthread_cond_timedwait.S
to be removed by Torvald Riegel. This happened in
ed19993b5b0d05d62cc883571519a67dae481a14.

See the following threads for previous versions and discussion:

* https://sourceware.org/ml/libc-alpha/2015-07/msg00193.html
* https://sourceware.org/ml/libc-alpha/2015-08/msg00186.html
* https://sourceware.org/ml/libc-alpha/2015-08/msg00230.html

Some of the abilist updates for other architectures appear to have gone
missing during the rebase. If this change is reviewed favourably then I
shall try to include them.

Signed-off-by: Mike Crowe <***@mcrowe.com>
---
ChangeLog | 48 +++++
manual/threads.texi | 21 +++
nptl/Makefile | 4 +-
nptl/Versions | 7 +
nptl/forward.c | 5 +
nptl/nptl-init.c | 1 +
nptl/pthreadP.h | 4 +
nptl/pthread_cond_wait.c | 28 ++-
nptl/tst-cond11-onclock.c | 209 +++++++++++++++++++++
nptl/tst-cond5-onclock.c | 114 +++++++++++
sysdeps/nptl/pthread-functions.h | 4 +
sysdeps/nptl/pthread.h | 15 ++
sysdeps/unix/sysv/linux/arm/libc.abilist | 1 +
sysdeps/unix/sysv/linux/arm/libpthread.abilist | 1 +
sysdeps/unix/sysv/linux/i386/libc.abilist | 1 +
sysdeps/unix/sysv/linux/i386/libpthread.abilist | 1 +
.../sysv/linux/m68k/coldfire/libpthread.abilist | 1 +
.../unix/sysv/linux/m68k/m680x0/libpthread.abilist | 1 +
sysdeps/unix/sysv/linux/x86_64/64/libc.abilist | 1 +
.../unix/sysv/linux/x86_64/64/libpthread.abilist | 2 +
sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist | 1 +
.../unix/sysv/linux/x86_64/x32/libpthread.abilist | 1 +
22 files changed, 466 insertions(+), 5 deletions(-)
create mode 100644 nptl/tst-cond11-onclock.c
create mode 100644 nptl/tst-cond5-onclock.c

diff --git a/ChangeLog b/ChangeLog
index 46a70ff7ce..385deb472f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,51 @@
[phone] Mike Crowe <***@mcrowe.com>
+
+ * sysdeps/nptl/pthread.h: Add pthread_cond_timedwaitonclock_np
+ declaration
+ * nptl/pthread_cond.c:
+ (__pthread_cond_wait_common): Add clockid parameter to determine
+ which clock to use.
+ (__pthread_cond_wait): Add dummy clockid parameter when calling
+ __pthread_cond_wait_common.
+ (__pthread_cond_timedwait): Read default clock set during
+ pthread_cond_init to supply as clockid parameter in call to
+ __pthread_cond_wait_common.
+ (__pthread_cond_timedwaitonclock_np): New function that supports
+ specifying clock to use at call time rather than initialisation
+ time.
+ (pthread_cond_timdwaitonclock_np): Weak alias for
+ __pthread_cond_timedwaitonclock_np
+ * nptl/tst-cond5-onclock.c: New file to test
+ pthread_cond_timedwaitonclock_np
+ * nptl/tst-cond11-onclock.c: New file to test
+ pthread_cond_timedwaitonclock_np
+ * nptl/Makefile: Add tst-cond5-onclock and tst-cond11-onclock
+ * nptl/Versions (GLIBC_2.26): Add pthread_cond_timedwaitonclock_np
+ to both libc and libpthread
+ * nptl/pthread_functions.h: Add
+ ptr___pthread_cond_timedwaitonclock_np
+ * nptl/forward.c (pthread_functions): Forward
+ pthread_cond_timedwaitonclock_np from libc to libpthread3
+ * nptl/nptl-init.c: Forwarding function initialisation for
+ pthread_cond_timedwaitonclock_np
+ * nptl/pthreadP.h (__pthread_cond_timedwait_np): Add
+ * manual/threads.texi: Add documentation for
+ pthread_cond_timedwaitonclock_np
+ * sysdeps/unix/sysv/linux/aarch64/libc.abilist: Add
+ pthread_cond_timedwaitonclock_np
+ * sysdeps/unix/sysv/linux/arm/libc.abilist: Likewise
+ * sysdeps/unix/sysv/linux/arm/libpthread.abilist: Likewise
+ * sysdeps/unix/sysv/linux/i386/libc.abilist: Likewise
+ * sysdeps/unix/sysv/linux/i386/libpthread.abilist: Likewise
+ * sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist: Likewise
+ * sysdeps/unix/sysv/linux/m68k/coldfire/libpthread.abilist: Likewise
+ * sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist: Likewise
+ * sysdeps/unix/sysv/linux/m68k/m680x0/libpthread.abilist: Likewise
+ * sysdeps/unix/sysv/linux/x86_64/64/libc.abilist: Likewise
+ * sysdeps/unix/sysv/linux/x86_64/64/libpthread.abilist: Likewise
+ * sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist: Likewise
+ * sysdeps/unix/sysv/linux/x86_64/x32/libpthread.abilist: Likewise
+
2017-06-21 Florian Weimer <***@redhat.com>

* intl/loadmsgcat.c: Remove alloca support.
diff --git a/manual/threads.texi b/manual/threads.texi
index 769d974d50..8b3652c859 100644
--- a/manual/threads.texi
+++ b/manual/threads.texi
@@ -124,6 +124,27 @@ The system does not have sufficient memory.
@end table
@end deftypefun

+@comment pthread.h
+@comment GNU
+@deftypefun int pthread_cond_timedwaitonclock_np (pthread_cond_t *@var{cond}, pthread_mutex_t *@var{mutex},
+ clockid_t @var{clockid}, const struct timespec *@var{abstime})
+@safety{@prelim{}@mtsafe{}@asunsafe{@asulock{}}@acunsafe{@aculock}}
+@c If exactly the same function with arguments is called from a signal
+@c handler that interrupts between the mutex unlock and sleep then it
+@c will unlock the mutex twice resulting in undefined behaviour. Keep
+@c in mind that the unlock and sleep are only atomic with respect to other
+@c threads (really a happens-after relationship for pthread_cond_broadcast
+@c and pthread_cond_signal).
+@c In the AC case we would cancel the thread and the mutex would remain
+@c locked and we can't recover from that.
+Behaves like POSIX @code{pthread_cond_timedwait} except the time
+@var{abstime} is measured against the clock specified by @var{clockid}
+rather than the clock specified or defaulted when @code{pthread_cond_init}
+was called. Currently, @code{clockid} must be either @code{CLOCK_MONOTONIC}
+or @code{CLOCK_REALTIME}. This variant is necessary in order to implement
+C++11's @code{std::condition_variable::wait_until} method correctly.
+@end deftypefun
+
@c FIXME these are undocumented:
@c pthread_atfork
@c pthread_attr_destroy
diff --git a/nptl/Makefile b/nptl/Makefile
index 853da72e74..8cf661dc64 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -235,8 +235,8 @@ tests = tst-attr1 tst-attr2 tst-attr3 tst-default-attr \
tst-mutexpi5a tst-mutexpi6 tst-mutexpi7 tst-mutexpi7a \
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 0ae5def464..1cf8c2fbad 100644
--- a/nptl/Versions
+++ b/nptl/Versions
@@ -28,6 +28,9 @@ libc {
pthread_cond_wait; pthread_cond_signal;
pthread_cond_broadcast; pthread_cond_timedwait;
}
+ GLIBC_2.26 {
+ pthread_cond_timedwaitonclock_np;
+ }
GLIBC_PRIVATE {
__libc_alloca_cutoff;
# Internal libc interface to libpthread
@@ -265,6 +268,10 @@ libpthread {
GLIBC_2.22 {
}

+ GLIBC_2.26 {
+ pthread_cond_timedwaitonclock_np;
+ }
+
GLIBC_PRIVATE {
__pthread_initialize_minimal;
__pthread_clock_gettime; __pthread_clock_settime;
diff --git a/nptl/forward.c b/nptl/forward.c
index ac96765f29..c5fcb2f066 100644
--- a/nptl/forward.c
+++ b/nptl/forward.c
@@ -164,6 +164,11 @@ FORWARD (__pthread_cond_timedwait,
const struct timespec *abstime), (cond, mutex, abstime), 0)
versioned_symbol (libc, __pthread_cond_timedwait, pthread_cond_timedwait,
GLIBC_2_3_2);
+FORWARD (__pthread_cond_timedwaitonclock_np,
+ (pthread_cond_t *cond, pthread_mutex_t *mutex, clockid_t clockid,
+ const struct timespec *abstime), (cond, mutex, clockid, abstime),
+ 0)
+weak_alias (__pthread_cond_timedwaitonclock_np, pthread_cond_timedwaitonclock_np);


FORWARD (pthread_equal, (pthread_t thread1, pthread_t thread2),
diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c
index 29216077a2..b913515fe9 100644
--- a/nptl/nptl-init.c
+++ b/nptl/nptl-init.c
@@ -106,6 +106,7 @@ static const struct pthread_functions pthread_functions =
.ptr___pthread_cond_signal = __pthread_cond_signal,
.ptr___pthread_cond_wait = __pthread_cond_wait,
.ptr___pthread_cond_timedwait = __pthread_cond_timedwait,
+ .ptr___pthread_cond_timedwaitonclock_np = __pthread_cond_timedwaitonclock_np,
# if SHLIB_COMPAT(libpthread, GLIBC_2_0, GLIBC_2_3_2)
.ptr___pthread_cond_broadcast_2_0 = __pthread_cond_broadcast_2_0,
.ptr___pthread_cond_destroy_2_0 = __pthread_cond_destroy_2_0,
diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
index 7fc1e50f78..c7e7587479 100644
--- a/nptl/pthreadP.h
+++ b/nptl/pthreadP.h
@@ -488,6 +488,10 @@ extern int __pthread_cond_wait (pthread_cond_t *cond, pthread_mutex_t *mutex);
extern int __pthread_cond_timedwait (pthread_cond_t *cond,
pthread_mutex_t *mutex,
const struct timespec *abstime);
+extern int __pthread_cond_timedwaitonclock_np (pthread_cond_t *cond,
+ pthread_mutex_t *mutex,
+ clockid_t clockid,
+ const struct timespec *abstime);
extern int __pthread_condattr_destroy (pthread_condattr_t *attr);
extern int __pthread_condattr_init (pthread_condattr_t *attr);
extern int __pthread_key_create (pthread_key_t *key, void (*destr) (void *));
diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c
index 7812b94a3a..d0166b86d5 100644
--- a/nptl/pthread_cond_wait.c
+++ b/nptl/pthread_cond_wait.c
@@ -378,6 +378,7 @@ __condvar_cleanup_waiting (void *arg)
*/
static __always_inline int
__pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
+ clockid_t clockid,
const struct timespec *abstime)
{
const int maxspin = 0;
@@ -510,7 +511,7 @@ __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
if (__glibc_unlikely (abstime->tv_sec < 0))
err = ETIMEDOUT;

- else if ((flags & __PTHREAD_COND_CLOCK_MONOTONIC_MASK) != 0)
+ else if (clockid == CLOCK_MONOTONIC)
{
/* CLOCK_MONOTONIC is requested. */
struct timespec rt;
@@ -652,7 +653,7 @@ __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
int
__pthread_cond_wait (pthread_cond_t *cond, pthread_mutex_t *mutex)
{
- return __pthread_cond_wait_common (cond, mutex, NULL);
+ return __pthread_cond_wait_common (cond, mutex, CLOCK_REALTIME, NULL);
}

/* See __pthread_cond_wait_common. */
@@ -664,10 +665,31 @@ __pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
it can assume that abstime is not NULL. */
if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
return EINVAL;
- return __pthread_cond_wait_common (cond, mutex, abstime);
+
+ clockid_t clockid = ((cond->__data.__wrefs & __PTHREAD_COND_CLOCK_MONOTONIC_MASK) != 0) ? CLOCK_MONOTONIC : CLOCK_REALTIME;
+ return __pthread_cond_wait_common (cond, mutex, clockid, abstime);
+}
+
+/* See __pthread_cond_wait_common. */
+int
+__pthread_cond_timedwaitonclock_np (pthread_cond_t *cond, pthread_mutex_t *mutex,
+ clockid_t clockid,
+ const struct timespec *abstime)
+{
+ /* Check parameter validity. This should also tell the compiler that
+ it can assume that abstime is not NULL. */
+ if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
+ return EINVAL;
+
+ /* We only support CLOCK_REALTIME and CLOCK_MONOTONIC */
+ if (clockid != CLOCK_REALTIME && clockid != CLOCK_MONOTONIC)
+ return EINVAL;
+
+ return __pthread_cond_wait_common (cond, mutex, clockid, abstime);
}

versioned_symbol (libpthread, __pthread_cond_wait, pthread_cond_wait,
GLIBC_2_3_2);
versioned_symbol (libpthread, __pthread_cond_timedwait, pthread_cond_timedwait,
GLIBC_2_3_2);
+weak_alias (__pthread_cond_timedwaitonclock_np, pthread_cond_timedwaitonclock_np);
diff --git a/nptl/tst-cond11-onclock.c b/nptl/tst-cond11-onclock.c
new file mode 100644
index 0000000000..439cd404de
--- /dev/null
+++ b/nptl/tst-cond11-onclock.c
@@ -0,0 +1,209 @@
+/* Test pthread_cond_timedwaitonclock_np, based on tst-cond11.c
+ Copyright (C) 2003-2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+ Contributed by Mike Crowe <***@mcrowe.com>, 2015.
+
+ 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 0000000000..adb3342781
--- /dev/null
+++ b/nptl/tst-cond5-onclock.c
@@ -0,0 +1,114 @@
+/* Test pthread_cond_timedwaitonclock_np, based on tst-cond5.c
+ Copyright (C) 2002-2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+ Contributed by Mike Crowe <***@mcrowe.com>, 2015.
+
+ 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-functions.h b/sysdeps/nptl/pthread-functions.h
index 4006fc6c25..8225fdbfca 100644
--- a/sysdeps/nptl/pthread-functions.h
+++ b/sysdeps/nptl/pthread-functions.h
@@ -55,6 +55,10 @@ struct pthread_functions
int (*ptr___pthread_cond_wait) (pthread_cond_t *, pthread_mutex_t *);
int (*ptr___pthread_cond_timedwait) (pthread_cond_t *, pthread_mutex_t *,
const struct timespec *);
+ int (*ptr___pthread_cond_timedwaitonclock_np) (pthread_cond_t *,
+ pthread_mutex_t *,
+ clockid_t,
+ const struct timespec *);
int (*ptr___pthread_cond_broadcast_2_0) (pthread_cond_2_0_t *);
int (*ptr___pthread_cond_destroy_2_0) (pthread_cond_2_0_t *);
int (*ptr___pthread_cond_init_2_0) (pthread_cond_2_0_t *,
diff --git a/sysdeps/nptl/pthread.h b/sysdeps/nptl/pthread.h
index 632ea7bc36..e95df8f592 100644
--- a/sysdeps/nptl/pthread.h
+++ b/sysdeps/nptl/pthread.h
@@ -1003,6 +1003,21 @@ extern int pthread_cond_timedwait (pthread_cond_t *__restrict __cond,
const struct timespec *__restrict __abstime)
__nonnull ((1, 2, 3));

+#ifdef __USE_GNU
+/* 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 the clock to use. 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));
+#endif
+
/* Functions for handling condition variable attributes. */

/* Initialize condition variable attribute ATTR. */
diff --git a/sysdeps/unix/sysv/linux/arm/libc.abilist b/sysdeps/unix/sysv/linux/arm/libc.abilist
index d2a206a8df..b0ecb71d3b 100644
--- a/sysdeps/unix/sysv/linux/arm/libc.abilist
+++ b/sysdeps/unix/sysv/linux/arm/libc.abilist
@@ -101,6 +101,7 @@ GLIBC_2.25 strfroml F
GLIBC_2.26 GLIBC_2.26 A
GLIBC_2.26 preadv2 F
GLIBC_2.26 preadv64v2 F
+GLIBC_2.26 pthread_cond_timedwaitonclock_np F
GLIBC_2.26 pwritev2 F
GLIBC_2.26 pwritev64v2 F
GLIBC_2.26 reallocarray F
diff --git a/sysdeps/unix/sysv/linux/arm/libpthread.abilist b/sysdeps/unix/sysv/linux/arm/libpthread.abilist
index 91545c1542..32b3331c19 100644
--- a/sysdeps/unix/sysv/linux/arm/libpthread.abilist
+++ b/sysdeps/unix/sysv/linux/arm/libpthread.abilist
@@ -9,6 +9,7 @@ GLIBC_2.12 pthread_setname_np F
GLIBC_2.18 GLIBC_2.18 A
GLIBC_2.18 pthread_getattr_default_np F
GLIBC_2.18 pthread_setattr_default_np F
+GLIBC_2.26 pthread_cond_timedwaitonclock_np F
GLIBC_2.4 GLIBC_2.4 A
GLIBC_2.4 _IO_flockfile F
GLIBC_2.4 _IO_ftrylockfile F
diff --git a/sysdeps/unix/sysv/linux/i386/libc.abilist b/sysdeps/unix/sysv/linux/i386/libc.abilist
index 2ff1998ac9..1bcf512fa1 100644
--- a/sysdeps/unix/sysv/linux/i386/libc.abilist
+++ b/sysdeps/unix/sysv/linux/i386/libc.abilist
@@ -2023,6 +2023,7 @@ GLIBC_2.25 strfroml F
GLIBC_2.26 GLIBC_2.26 A
GLIBC_2.26 preadv2 F
GLIBC_2.26 preadv64v2 F
+GLIBC_2.26 pthread_cond_timedwaitonclock_np F
GLIBC_2.26 pwritev2 F
GLIBC_2.26 pwritev64v2 F
GLIBC_2.26 reallocarray F
diff --git a/sysdeps/unix/sysv/linux/i386/libpthread.abilist b/sysdeps/unix/sysv/linux/i386/libpthread.abilist
index 8f9c3254be..3ae0e37475 100644
--- a/sysdeps/unix/sysv/linux/i386/libpthread.abilist
+++ b/sysdeps/unix/sysv/linux/i386/libpthread.abilist
@@ -216,6 +216,7 @@ GLIBC_2.2.3 GLIBC_2.2.3 A
GLIBC_2.2.3 pthread_getattr_np F
GLIBC_2.2.6 GLIBC_2.2.6 A
GLIBC_2.2.6 __nanosleep F
+GLIBC_2.26 pthread_cond_timedwaitonclock_np F
GLIBC_2.3.2 GLIBC_2.3.2 A
GLIBC_2.3.2 pthread_cond_broadcast F
GLIBC_2.3.2 pthread_cond_destroy F
diff --git a/sysdeps/unix/sysv/linux/m68k/coldfire/libpthread.abilist b/sysdeps/unix/sysv/linux/m68k/coldfire/libpthread.abilist
index 91545c1542..32b3331c19 100644
--- a/sysdeps/unix/sysv/linux/m68k/coldfire/libpthread.abilist
+++ b/sysdeps/unix/sysv/linux/m68k/coldfire/libpthread.abilist
@@ -9,6 +9,7 @@ GLIBC_2.12 pthread_setname_np F
GLIBC_2.18 GLIBC_2.18 A
GLIBC_2.18 pthread_getattr_default_np F
GLIBC_2.18 pthread_setattr_default_np F
+GLIBC_2.26 pthread_cond_timedwaitonclock_np F
GLIBC_2.4 GLIBC_2.4 A
GLIBC_2.4 _IO_flockfile F
GLIBC_2.4 _IO_ftrylockfile F
diff --git a/sysdeps/unix/sysv/linux/m68k/m680x0/libpthread.abilist b/sysdeps/unix/sysv/linux/m68k/m680x0/libpthread.abilist
index 8f9c3254be..3ae0e37475 100644
--- a/sysdeps/unix/sysv/linux/m68k/m680x0/libpthread.abilist
+++ b/sysdeps/unix/sysv/linux/m68k/m680x0/libpthread.abilist
@@ -216,6 +216,7 @@ GLIBC_2.2.3 GLIBC_2.2.3 A
GLIBC_2.2.3 pthread_getattr_np F
GLIBC_2.2.6 GLIBC_2.2.6 A
GLIBC_2.2.6 __nanosleep F
+GLIBC_2.26 pthread_cond_timedwaitonclock_np F
GLIBC_2.3.2 GLIBC_2.3.2 A
GLIBC_2.3.2 pthread_cond_broadcast F
GLIBC_2.3.2 pthread_cond_destroy F
diff --git a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
index 513524d932..13f13e04db 100644
--- a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
@@ -1864,6 +1864,7 @@ GLIBC_2.25 strfroml F
GLIBC_2.26 GLIBC_2.26 A
GLIBC_2.26 preadv2 F
GLIBC_2.26 preadv64v2 F
+GLIBC_2.26 pthread_cond_timedwaitonclock_np F
GLIBC_2.26 pwritev2 F
GLIBC_2.26 pwritev64v2 F
GLIBC_2.26 reallocarray F
diff --git a/sysdeps/unix/sysv/linux/x86_64/64/libpthread.abilist b/sysdeps/unix/sysv/linux/x86_64/64/libpthread.abilist
index 85365c057c..accb1d8d6f 100644
--- a/sysdeps/unix/sysv/linux/x86_64/64/libpthread.abilist
+++ b/sysdeps/unix/sysv/linux/x86_64/64/libpthread.abilist
@@ -203,6 +203,8 @@ GLIBC_2.2.5 waitpid F
GLIBC_2.2.5 write F
GLIBC_2.2.6 GLIBC_2.2.6 A
GLIBC_2.2.6 __nanosleep F
+GLIBC_2.26 GLIBC_2.26 A
+GLIBC_2.26 pthread_cond_timedwaitonclock_np F
GLIBC_2.3.2 GLIBC_2.3.2 A
GLIBC_2.3.2 pthread_cond_broadcast F
GLIBC_2.3.2 pthread_cond_destroy F
diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
index 0c557e9f43..db930f6307 100644
--- a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
@@ -2107,6 +2107,7 @@ GLIBC_2.25 strfroml F
GLIBC_2.26 GLIBC_2.26 A
GLIBC_2.26 preadv2 F
GLIBC_2.26 preadv64v2 F
+GLIBC_2.26 pthread_cond_timedwaitonclock_np F
GLIBC_2.26 pwritev2 F
GLIBC_2.26 pwritev64v2 F
GLIBC_2.26 reallocarray F
diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/libpthread.abilist b/sysdeps/unix/sysv/linux/x86_64/x32/libpthread.abilist
index 6cd0fc3487..d89762bd33 100644
--- a/sysdeps/unix/sysv/linux/x86_64/x32/libpthread.abilist
+++ b/sysdeps/unix/sysv/linux/x86_64/x32/libpthread.abilist
@@ -224,3 +224,4 @@ GLIBC_2.16 write F
GLIBC_2.18 GLIBC_2.18 A
GLIBC_2.18 pthread_getattr_default_np F
GLIBC_2.18 pthread_setattr_default_np F
+GLIBC_2.26 pthread_cond_timedwaitonclock_np F

[Adhemerval Zanella]

Post by Mike Crowe
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 condition
variable 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.
Let's invent 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.
Earlier versions of this patch were proposed in 2015 but further progress
stalled waiting for the assembly implementation of pthread_cond_timedwait.S
to be removed by Torvald Riegel. This happened in
ed19993b5b0d05d62cc883571519a67dae481a14.
* https://sourceware.org/ml/libc-alpha/2015-07/msg00193.html
* https://sourceware.org/ml/libc-alpha/2015-08/msg00186.html
* https://sourceware.org/ml/libc-alpha/2015-08/msg00230.html
Some of the abilist updates for other architectures appear to have gone
missing during the rebase. If this change is reviewed favourably then I
shall try to include them.

I think it is a reasonable addition, however I have some concerns if this
would just another bad discussed ABI to fix and specific issues that will
be replaced by another better or simple solution (as your last comment in
BZ#41861 [1]). It also concerns that it was not brought to Austin Group
attention as suggested by Szabolcs [2].

Torvald, do you think a new API to specific a clock is a reasonable addition
for GLIBC? Rich, any thought about this new API (any possible musl inclusion
in nearby future in case of glibc acceptance)? Do you think we should
approach Austin Group first? The issue with C++ seems to a QoI one that is
currently defined afaik as implementation-defined, so it would be an
enhancement

The patch itself has some minor issues about formatting that I outlined
below.

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41861
[2] https://sourceware.org/ml/libc-alpha/2015-07/msg00194.html

Post by Mike Crowe
---
ChangeLog | 48 +++++
manual/threads.texi | 21 +++
nptl/Makefile | 4 +-
nptl/Versions | 7 +
nptl/forward.c | 5 +
nptl/nptl-init.c | 1 +
nptl/pthreadP.h | 4 +
nptl/pthread_cond_wait.c | 28 ++-
nptl/tst-cond11-onclock.c | 209 +++++++++++++++++++++
nptl/tst-cond5-onclock.c | 114 +++++++++++
sysdeps/nptl/pthread-functions.h | 4 +
sysdeps/nptl/pthread.h | 15 ++
sysdeps/unix/sysv/linux/arm/libc.abilist | 1 +
sysdeps/unix/sysv/linux/arm/libpthread.abilist | 1 +
sysdeps/unix/sysv/linux/i386/libc.abilist | 1 +
sysdeps/unix/sysv/linux/i386/libpthread.abilist | 1 +
.../sysv/linux/m68k/coldfire/libpthread.abilist | 1 +
.../unix/sysv/linux/m68k/m680x0/libpthread.abilist | 1 +
sysdeps/unix/sysv/linux/x86_64/64/libc.abilist | 1 +
.../unix/sysv/linux/x86_64/64/libpthread.abilist | 2 +
sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist | 1 +
.../unix/sysv/linux/x86_64/x32/libpthread.abilist | 1 +
22 files changed, 466 insertions(+), 5 deletions(-)
create mode 100644 nptl/tst-cond11-onclock.c
create mode 100644 nptl/tst-cond5-onclock.c
diff --git a/ChangeLog b/ChangeLog
index 46a70ff7ce..385deb472f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,51 @@
+
+ * sysdeps/nptl/pthread.h: Add pthread_cond_timedwaitonclock_np
+ declaration
+ (__pthread_cond_wait_common): Add clockid parameter to determine
+ which clock to use.
+ (__pthread_cond_wait): Add dummy clockid parameter when calling
+ __pthread_cond_wait_common.
+ (__pthread_cond_timedwait): Read default clock set during
+ pthread_cond_init to supply as clockid parameter in call to
+ __pthread_cond_wait_common.
+ (__pthread_cond_timedwaitonclock_np): New function that supports
+ specifying clock to use at call time rather than initialisation
+ time.
+ (pthread_cond_timdwaitonclock_np): Weak alias for
+ __pthread_cond_timedwaitonclock_np
+ * nptl/tst-cond5-onclock.c: New file to test
+ pthread_cond_timedwaitonclock_np
+ * nptl/tst-cond11-onclock.c: New file to test
+ pthread_cond_timedwaitonclock_np
+ * nptl/Makefile: Add tst-cond5-onclock and tst-cond11-onclock
+ * nptl/Versions (GLIBC_2.26): Add pthread_cond_timedwaitonclock_np
+ to both libc and libpthread
+ * nptl/pthread_functions.h: Add
+ ptr___pthread_cond_timedwaitonclock_np
+ * nptl/forward.c (pthread_functions): Forward
+ pthread_cond_timedwaitonclock_np from libc to libpthread3
+ * nptl/nptl-init.c: Forwarding function initialisation for
+ pthread_cond_timedwaitonclock_np
+ * nptl/pthreadP.h (__pthread_cond_timedwait_np): Add
+ * manual/threads.texi: Add documentation for
+ pthread_cond_timedwaitonclock_np
+ * sysdeps/unix/sysv/linux/aarch64/libc.abilist: Add
+ pthread_cond_timedwaitonclock_np
+ * sysdeps/unix/sysv/linux/arm/libc.abilist: Likewise
+ * sysdeps/unix/sysv/linux/arm/libpthread.abilist: Likewise
+ * sysdeps/unix/sysv/linux/i386/libc.abilist: Likewise
+ * sysdeps/unix/sysv/linux/i386/libpthread.abilist: Likewise
+ * sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist: Likewise
+ * sysdeps/unix/sysv/linux/m68k/coldfire/libpthread.abilist: Likewise
+ * sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist: Likewise
+ * sysdeps/unix/sysv/linux/m68k/m680x0/libpthread.abilist: Likewise
+ * sysdeps/unix/sysv/linux/x86_64/64/libc.abilist: Likewise
+ * sysdeps/unix/sysv/linux/x86_64/64/libpthread.abilist: Likewise
+ * sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist: Likewise
+ * sysdeps/unix/sysv/linux/x86_64/x32/libpthread.abilist: Likewise
+
* intl/loadmsgcat.c: Remove alloca support.
diff --git a/manual/threads.texi b/manual/threads.texi
index 769d974d50..8b3652c859 100644
--- a/manual/threads.texi
+++ b/manual/threads.texi
@@ -124,6 +124,27 @@ The system does not have sufficient memory.
@end table
@end deftypefun

I think we can't omit the C++11 requirement on manual.

Post by Mike Crowe
+
@c pthread_atfork
@c pthread_attr_destroy
diff --git a/nptl/Makefile b/nptl/Makefile
index 853da72e74..8cf661dc64 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -235,8 +235,8 @@ tests = tst-attr1 tst-attr2 tst-attr3 tst-default-attr \
tst-mutexpi5a tst-mutexpi6 tst-mutexpi7 tst-mutexpi7a \
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 \

Line too long, split in multiple ones.

Post by Mike Crowe
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 0ae5def464..1cf8c2fbad 100644
--- a/nptl/Versions
+++ b/nptl/Versions
@@ -28,6 +28,9 @@ libc {
pthread_cond_wait; pthread_cond_signal;
pthread_cond_broadcast; pthread_cond_timedwait;
}
+ GLIBC_2.26 {
+ pthread_cond_timedwaitonclock_np;
+ }

The libc addition seems wrong, whhy this is required?

Post by Mike Crowe
GLIBC_PRIVATE {
__libc_alloca_cutoff;
# Internal libc interface to libpthread
@@ -265,6 +268,10 @@ libpthread {
GLIBC_2.22 {
}
+ GLIBC_2.26 {
+ pthread_cond_timedwaitonclock_np;
+ }
+
GLIBC_PRIVATE {
__pthread_initialize_minimal;
__pthread_clock_gettime; __pthread_clock_settime;
diff --git a/nptl/forward.c b/nptl/forward.c
index ac96765f29..c5fcb2f066 100644
--- a/nptl/forward.c
+++ b/nptl/forward.c
@@ -164,6 +164,11 @@ FORWARD (__pthread_cond_timedwait,
const struct timespec *abstime), (cond, mutex, abstime), 0)
versioned_symbol (libc, __pthread_cond_timedwait, pthread_cond_timedwait,
GLIBC_2_3_2);
+FORWARD (__pthread_cond_timedwaitonclock_np,
+ (pthread_cond_t *cond, pthread_mutex_t *mutex, clockid_t clockid,
+ const struct timespec *abstime), (cond, mutex, clockid, abstime),
+ 0)
+weak_alias (__pthread_cond_timedwaitonclock_np, pthread_cond_timedwaitonclock_np);
FORWARD (pthread_equal, (pthread_t thread1, pthread_t thread2),
diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c
index 29216077a2..b913515fe9 100644
--- a/nptl/nptl-init.c
+++ b/nptl/nptl-init.c
@@ -106,6 +106,7 @@ static const struct pthread_functions pthread_functions =
.ptr___pthread_cond_signal = __pthread_cond_signal,
.ptr___pthread_cond_wait = __pthread_cond_wait,
.ptr___pthread_cond_timedwait = __pthread_cond_timedwait,
+ .ptr___pthread_cond_timedwaitonclock_np = __pthread_cond_timedwaitonclock_np,
# if SHLIB_COMPAT(libpthread, GLIBC_2_0, GLIBC_2_3_2)
.ptr___pthread_cond_broadcast_2_0 = __pthread_cond_broadcast_2_0,
.ptr___pthread_cond_destroy_2_0 = __pthread_cond_destroy_2_0,
diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
index 7fc1e50f78..c7e7587479 100644
--- a/nptl/pthreadP.h
+++ b/nptl/pthreadP.h
@@ -488,6 +488,10 @@ extern int __pthread_cond_wait (pthread_cond_t *cond, pthread_mutex_t *mutex);
extern int __pthread_cond_timedwait (pthread_cond_t *cond,
pthread_mutex_t *mutex,
const struct timespec *abstime);
+extern int __pthread_cond_timedwaitonclock_np (pthread_cond_t *cond,
+ pthread_mutex_t *mutex,
+ clockid_t clockid,
+ const struct timespec *abstime);
extern int __pthread_condattr_destroy (pthread_condattr_t *attr);
extern int __pthread_condattr_init (pthread_condattr_t *attr);
extern int __pthread_key_create (pthread_key_t *key, void (*destr) (void *));
diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c
index 7812b94a3a..d0166b86d5 100644
--- a/nptl/pthread_cond_wait.c
+++ b/nptl/pthread_cond_wait.c
@@ -378,6 +378,7 @@ __condvar_cleanup_waiting (void *arg)
*/
static __always_inline int
__pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
+ clockid_t clockid,
const struct timespec *abstime)
{
const int maxspin = 0;
@@ -510,7 +511,7 @@ __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
if (__glibc_unlikely (abstime->tv_sec < 0))
err = ETIMEDOUT;
- else if ((flags & __PTHREAD_COND_CLOCK_MONOTONIC_MASK) != 0)
+ else if (clockid == CLOCK_MONOTONIC)
{
/* CLOCK_MONOTONIC is requested. */
struct timespec rt;
@@ -652,7 +653,7 @@ __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
int__PTHREAD_COND_CLOCK_MONOTONIC_MASK
__pthread_cond_wait (pthread_cond_t *cond, pthread_mutex_t *mutex)
{
- return __pthread_cond_wait_common (cond, mutex, NULL);
+ return __pthread_cond_wait_common (cond, mutex, CLOCK_REALTIME, NULL);
}

I would prefer to just use '0' and states clockid_t is unused due 'abstime' being NULL.

Post by Mike Crowe
/* See __pthread_cond_wait_common. */
@@ -664,10 +665,31 @@ __pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
it can assume that abstime is not NULL. */
if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
return EINVAL;
- return __pthread_cond_wait_common (cond, mutex, abstime);
+
+ clockid_t clockid = ((cond->__data.__wrefs & __PTHREAD_COND_CLOCK_MONOTONIC_MASK) != 0) ? CLOCK_MONOTONIC : CLOCK_REALTIME;
+ return __pthread_cond_wait_common (cond, mutex, clockid, abstime);
+}

Line to long and I think it is required to state why a plain load to
__wrefs in this specific case is suffice for synchronization. In fact
I think it would be better to just use a relaxed MO:

/* Relaxed MO is suffice because clock ID bit is only modified
in condition creation. */
unsigned int flags = atomic_load_relaxed (&cond->__data.__wrefs);
clockid_t clock = (flags & __PTHREAD_COND_CLOCK_MONOTONIC_MASK)
? CLOCK_MONOTONIC : CLOCK_REALTIME;
return __pthread_cond_wait_common (cond, mutex, clock, abstime);

Post by Mike Crowe
+
+/* See __pthread_cond_wait_common. */
+int
+__pthread_cond_timedwaitonclock_np (pthread_cond_t *cond, pthread_mutex_t *mutex,
+ clockid_t clockid,
+ const struct timespec *abstime)
+{
+ /* Check parameter validity. This should also tell the compiler that
+ it can assume that abstime is not NULL. */
+ if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
+ return EINVAL;
+
+ /* We only support CLOCK_REALTIME and CLOCK_MONOTONIC */
+ if (clockid != CLOCK_REALTIME && clockid != CLOCK_MONOTONIC)
+ return EINVAL;

I think we shold also add the test present on pthread_condattr_setclock
(Which will in fact turn to always 'true' due 'futex_supports_exact_relative_timeouts'
being always true in Linux) for consistency.

/* If we do not support waiting using CLOCK_MONOTONIC, return an error. */
if (clock_id == CLOCK_MONOTONIC
&& !futex_supports_exact_relative_timeouts())
return ENOTSUP;

Post by Mike Crowe
+
+ return __pthread_cond_wait_common (cond, mutex, clockid, abstime);
}
versioned_symbol (libpthread, __pthread_cond_wait, pthread_cond_wait,
GLIBC_2_3_2);
versioned_symbol (libpthread, __pthread_cond_timedwait, pthread_cond_timedwait,
GLIBC_2_3_2);
+weak_alias (__pthread_cond_timedwaitonclock_np, pthread_cond_timedwaitonclock_np);
diff --git a/nptl/tst-cond11-onclock.c b/nptl/tst-cond11-onclock.c
new file mode 100644
index 0000000000..439cd404de
--- /dev/null
+++ b/nptl/tst-cond11-onclock.c
@@ -0,0 +1,209 @@
+/* Test pthread_cond_timedwaitonclock_np, based on tst-cond11.c
+ Copyright (C) 2003-2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ 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"

We create tests using libsupport now, support/README, support/README-testing.c
have some examples. It is simplifies a lot the error reporting and code
on tests.

Post by Mike Crowe
diff --git a/nptl/tst-cond5-onclock.c b/nptl/tst-cond5-onclock.c
new file mode 100644
index 0000000000..adb3342781
--- /dev/null
+++ b/nptl/tst-cond5-onclock.c
@@ -0,0 +1,114 @@
+/* Test pthread_cond_timedwaitonclock_np, based on tst-cond5.c
+ Copyright (C) 2002-2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ 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-functions.h b/sysdeps/nptl/pthread-functions.h
index 4006fc6c25..8225fdbfca 100644
--- a/sysdeps/nptl/pthread-functions.h
+++ b/sysdeps/nptl/pthread-functions.h
@@ -55,6 +55,10 @@ struct pthread_functions
int (*ptr___pthread_cond_wait) (pthread_cond_t *, pthread_mutex_t *);
int (*ptr___pthread_cond_timedwait) (pthread_cond_t *, pthread_mutex_t *,
const struct timespec *);
+ int (*ptr___pthread_cond_timedwaitonclock_np) (pthread_cond_t *,
+ pthread_mutex_t *,
+ clockid_t,
+ const struct timespec *);
int (*ptr___pthread_cond_broadcast_2_0) (pthread_cond_2_0_t *);
int (*ptr___pthread_cond_destroy_2_0) (pthread_cond_2_0_t *);
int (*ptr___pthread_cond_init_2_0) (pthread_cond_2_0_t *,
diff --git a/sysdeps/nptl/pthread.h b/sysdeps/nptl/pthread.h
index 632ea7bc36..e95df8f592 100644
--- a/sysdeps/nptl/pthread.h
+++ b/sysdeps/nptl/pthread.h
@@ -1003,6 +1003,21 @@ extern int pthread_cond_timedwait (pthread_cond_t *__restrict __cond,
const struct timespec *__restrict __abstime)
__nonnull ((1, 2, 3));
+#ifdef __USE_GNU
+/* 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 the clock to use. 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));
+#endif
+
/* Functions for handling condition variable attributes. */
/* Initialize condition variable attribute ATTR. */
diff --git a/sysdeps/unix/sysv/linux/arm/libc.abilist b/sysdeps/unix/sysv/linux/arm/libc.abilist
index d2a206a8df..b0ecb71d3b 100644
--- a/sysdeps/unix/sysv/linux/arm/libc.abilist
+++ b/sysdeps/unix/sysv/linux/arm/libc.abilist
@@ -101,6 +101,7 @@ GLIBC_2.25 strfroml F
GLIBC_2.26 GLIBC_2.26 A
GLIBC_2.26 preadv2 F
GLIBC_2.26 preadv64v2 F
+GLIBC_2.26 pthread_cond_timedwaitonclock_np F
GLIBC_2.26 pwritev2 F
GLIBC_2.26 pwritev64v2 F
GLIBC_2.26 reallocarray F
diff --git a/sysdeps/unix/sysv/linux/arm/libpthread.abilist b/sysdeps/unix/sysv/linux/arm/libpthread.abilist
index 91545c1542..32b3331c19 100644
--- a/sysdeps/unix/sysv/linux/arm/libpthread.abilist
+++ b/sysdeps/unix/sysv/linux/arm/libpthread.abilist
@@ -9,6 +9,7 @@ GLIBC_2.12 pthread_setname_np F
GLIBC_2.18 GLIBC_2.18 A
GLIBC_2.18 pthread_getattr_default_np F
GLIBC_2.18 pthread_setattr_default_np F
+GLIBC_2.26 pthread_cond_timedwaitonclock_np F
GLIBC_2.4 GLIBC_2.4 A
GLIBC_2.4 _IO_flockfile F
GLIBC_2.4 _IO_ftrylockfile F
diff --git a/sysdeps/unix/sysv/linux/i386/libc.abilist b/sysdeps/unix/sysv/linux/i386/libc.abilist
index 2ff1998ac9..1bcf512fa1 100644
--- a/sysdeps/unix/sysv/linux/i386/libc.abilist
+++ b/sysdeps/unix/sysv/linux/i386/libc.abilist
@@ -2023,6 +2023,7 @@ GLIBC_2.25 strfroml F
GLIBC_2.26 GLIBC_2.26 A
GLIBC_2.26 preadv2 F
GLIBC_2.26 preadv64v2 F
+GLIBC_2.26 pthread_cond_timedwaitonclock_np F
GLIBC_2.26 pwritev2 F
GLIBC_2.26 pwritev64v2 F
GLIBC_2.26 reallocarray F
diff --git a/sysdeps/unix/sysv/linux/i386/libpthread.abilist b/sysdeps/unix/sysv/linux/i386/libpthread.abilist
index 8f9c3254be..3ae0e37475 100644
--- a/sysdeps/unix/sysv/linux/i386/libpthread.abilist
+++ b/sysdeps/unix/sysv/linux/i386/libpthread.abilist
@@ -216,6 +216,7 @@ GLIBC_2.2.3 GLIBC_2.2.3 A
GLIBC_2.2.3 pthread_getattr_np F
GLIBC_2.2.6 GLIBC_2.2.6 A
GLIBC_2.2.6 __nanosleep F
+GLIBC_2.26 pthread_cond_timedwaitonclock_np F
GLIBC_2.3.2 GLIBC_2.3.2 A
GLIBC_2.3.2 pthread_cond_broadcast F
GLIBC_2.3.2 pthread_cond_destroy F
diff --git a/sysdeps/unix/sysv/linux/m68k/coldfire/libpthread.abilist b/sysdeps/unix/sysv/linux/m68k/coldfire/libpthread.abilist
index 91545c1542..32b3331c19 100644
--- a/sysdeps/unix/sysv/linux/m68k/coldfire/libpthread.abilist
+++ b/sysdeps/unix/sysv/linux/m68k/coldfire/libpthread.abilist
@@ -9,6 +9,7 @@ GLIBC_2.12 pthread_setname_np F
GLIBC_2.18 GLIBC_2.18 A
GLIBC_2.18 pthread_getattr_default_np F
GLIBC_2.18 pthread_setattr_default_np F
+GLIBC_2.26 pthread_cond_timedwaitonclock_np F
GLIBC_2.4 GLIBC_2.4 A
GLIBC_2.4 _IO_flockfile F
GLIBC_2.4 _IO_ftrylockfile F
diff --git a/sysdeps/unix/sysv/linux/m68k/m680x0/libpthread.abilist b/sysdeps/unix/sysv/linux/m68k/m680x0/libpthread.abilist
index 8f9c3254be..3ae0e37475 100644
--- a/sysdeps/unix/sysv/linux/m68k/m680x0/libpthread.abilist
+++ b/sysdeps/unix/sysv/linux/m68k/m680x0/libpthread.abilist
@@ -216,6 +216,7 @@ GLIBC_2.2.3 GLIBC_2.2.3 A
GLIBC_2.2.3 pthread_getattr_np F
GLIBC_2.2.6 GLIBC_2.2.6 A
GLIBC_2.2.6 __nanosleep F
+GLIBC_2.26 pthread_cond_timedwaitonclock_np F
GLIBC_2.3.2 GLIBC_2.3.2 A
GLIBC_2.3.2 pthread_cond_broadcast F
GLIBC_2.3.2 pthread_cond_destroy F
diff --git a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
index 513524d932..13f13e04db 100644
--- a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
@@ -1864,6 +1864,7 @@ GLIBC_2.25 strfroml F
GLIBC_2.26 GLIBC_2.26 A
GLIBC_2.26 preadv2 F
GLIBC_2.26 preadv64v2 F
+GLIBC_2.26 pthread_cond_timedwaitonclock_np F
GLIBC_2.26 pwritev2 F
GLIBC_2.26 pwritev64v2 F
GLIBC_2.26 reallocarray F
diff --git a/sysdeps/unix/sysv/linux/x86_64/64/libpthread.abilist b/sysdeps/unix/sysv/linux/x86_64/64/libpthread.abilist
index 85365c057c..accb1d8d6f 100644
--- a/sysdeps/unix/sysv/linux/x86_64/64/libpthread.abilist
+++ b/sysdeps/unix/sysv/linux/x86_64/64/libpthread.abilist
@@ -203,6 +203,8 @@ GLIBC_2.2.5 waitpid F
GLIBC_2.2.5 write F
GLIBC_2.2.6 GLIBC_2.2.6 A
GLIBC_2.2.6 __nanosleep F
+GLIBC_2.26 GLIBC_2.26 A
+GLIBC_2.26 pthread_cond_timedwaitonclock_np F
GLIBC_2.3.2 GLIBC_2.3.2 A
GLIBC_2.3.2 pthread_cond_broadcast F
GLIBC_2.3.2 pthread_cond_destroy F
diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
index 0c557e9f43..db930f6307 100644
--- a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
@@ -2107,6 +2107,7 @@ GLIBC_2.25 strfroml F
GLIBC_2.26 GLIBC_2.26 A
GLIBC_2.26 preadv2 F
GLIBC_2.26 preadv64v2 F
+GLIBC_2.26 pthread_cond_timedwaitonclock_np F
GLIBC_2.26 pwritev2 F
GLIBC_2.26 pwritev64v2 F
GLIBC_2.26 reallocarray F
diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/libpthread.abilist b/sysdeps/unix/sysv/linux/x86_64/x32/libpthread.abilist
index 6cd0fc3487..d89762bd33 100644
--- a/sysdeps/unix/sysv/linux/x86_64/x32/libpthread.abilist
+++ b/sysdeps/unix/sysv/linux/x86_64/x32/libpthread.abilist
@@ -224,3 +224,4 @@ GLIBC_2.16 write F
GLIBC_2.18 GLIBC_2.18 A
GLIBC_2.18 pthread_getattr_default_np F
GLIBC_2.18 pthread_setattr_default_np F
+GLIBC_2.26 pthread_cond_timedwaitonclock_np F

[Mike Crowe]

Apologies for the slow reply. I seem to have somehow missed your response
on libc-alpha. :(

Post by Adhemerval Zanella

Post by Mike Crowe
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 condition
variable 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.
Let's invent 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.
Earlier versions of this patch were proposed in 2015 but further progress
stalled waiting for the assembly implementation of pthread_cond_timedwait.S
to be removed by Torvald Riegel. This happened in
ed19993b5b0d05d62cc883571519a67dae481a14.
* https://sourceware.org/ml/libc-alpha/2015-07/msg00193.html
* https://sourceware.org/ml/libc-alpha/2015-08/msg00186.html
* https://sourceware.org/ml/libc-alpha/2015-08/msg00230.html
Some of the abilist updates for other architectures appear to have gone
missing during the rebase. If this change is reviewed favourably then I
shall try to include them.

I think it is a reasonable addition, however I have some concerns if this
would just another bad discussed ABI to fix and specific issues that will
be replaced by another better or simple solution (as your last comment in
BZ#41861 [1]).

Comment 14 of BZ#41861 proposed two solutions, but I believe that they both
have downsides:

1. Write a new condition variable implementation in libstdc++ on top of
futex(2). Since I wrote that comment I've become aware that libstdc++ has
gained an atomic_futex implementation that could be used. It currently
falls back to using std::condition_variable when futex isn't available so
that would need to be changed before it could be used as the basis for
std::condition_variable itself.

2. Make std::condition_variable use CLOCK_MONOTONIC by default and convert
to that from CLOCK_REALTIME when passed std::chrono::system_clock. I'm
surprised this wasn't done several years ago, since I think it is
preferable to the current situation. However, it's not perfect since a wait
on std::chrono::system_clock will not react to changes made to the clock
during the wait. The current implementation results in FUTEX_CLOCK_REALTIME
being used which does react to changes to the system clock during the wait,
so this solution would change the behaviour of such code.

Post by Adhemerval Zanella
It also concerns that it was not brought to Austin Group
attention as suggested by Szabolcs [2].

I shall try to work out how I should go about doing that.

Post by Adhemerval Zanella
Torvald, do you think a new API to specific a clock is a reasonable addition
for GLIBC? Rich, any thought about this new API (any possible musl inclusion
in nearby future in case of glibc acceptance)? Do you think we should
approach Austin Group first? The issue with C++ seems to a QoI one that is
currently defined afaik as implementation-defined, so it would be an
enhancement

The lack of reliable waits using a steady clock is a problem for embedded
systems in particular because the system clock may be set at any time. In
the past we used to not even attempt to set the system clock to the real
time because so many libraries would misbehave when it changed. But,
gradually, most code has been fixed to use CLOCK_MONOTONIC so we now do so.
But we're left using our own std::condition_variable variant now (that
basically implements solution 2 above) in order to avoid the problem in C++
code.

Post by Adhemerval Zanella
The patch itself has some minor issues about formatting that I outlined
below.

Thanks for the review. I shall try to incorporate your suggestions and post
a new version.

Mike.

[Mike Crowe]

Post by Adhemerval Zanella

Post by Mike Crowe
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 condition
variable 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.
Let's invent 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.
Earlier versions of this patch were proposed in 2015 but further progress
stalled waiting for the assembly implementation of pthread_cond_timedwait.S
to be removed by Torvald Riegel. This happened in
ed19993b5b0d05d62cc883571519a67dae481a14.
* https://sourceware.org/ml/libc-alpha/2015-07/msg00193.html
* https://sourceware.org/ml/libc-alpha/2015-08/msg00186.html
* https://sourceware.org/ml/libc-alpha/2015-08/msg00230.html
Some of the abilist updates for other architectures appear to have gone
missing during the rebase. If this change is reviewed favourably then I
shall try to include them.

I think it is a reasonable addition, however I have some concerns if this
would just another bad discussed ABI to fix and specific issues that will
be replaced by another better or simple solution (as your last comment in
BZ#41861 [1]). It also concerns that it was not brought to Austin Group
attention as suggested by Szabolcs [2].

I've now submitted the suggestion. It can be viewed at
http://austingroupbugs.net/view.php?id=1164

[Snipped uncontentious parts of patch and suggestions for brevity.]

Post by Adhemerval Zanella

Post by Mike Crowe
diff --git a/nptl/Versions b/nptl/Versions
index 0ae5def464..1cf8c2fbad 100644
--- a/nptl/Versions
+++ b/nptl/Versions
@@ -28,6 +28,9 @@ libc {
pthread_cond_wait; pthread_cond_signal;
pthread_cond_broadcast; pthread_cond_timedwait;
}
+ GLIBC_2.26 {
+ pthread_cond_timedwaitonclock_np;
+ }

The libc addition seems wrong, whhy this is required?

pthread_cond_timedwait is in libc, so I followed suite and added
pthread_cond_timedwaitonclock_np too. I must admit that I can't quite see
how the dummy empty implementations of the pthread functions end up in
libc, but they apparently do according to:

https://stackoverflow.com/questions/11161462/why-glibc-and-pthread-library-both-defined-same-apis

Maybe this means that I've missed something.

Post by Adhemerval Zanella

Post by Mike Crowe
/* See __pthread_cond_wait_common. */
@@ -664,10 +665,31 @@ __pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
it can assume that abstime is not NULL. */
if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
return EINVAL;
- return __pthread_cond_wait_common (cond, mutex, abstime);
+
+ clockid_t clockid = ((cond->__data.__wrefs & __PTHREAD_COND_CLOCK_MONOTONIC_MASK) != 0) ? CLOCK_MONOTONIC : CLOCK_REALTIME;
+ return __pthread_cond_wait_common (cond, mutex, clockid, abstime);
+}

Line to long and I think it is required to state why a plain load to
__wrefs in this specific case is suffice for synchronization. In fact
/* Relaxed MO is suffice because clock ID bit is only modified
in condition creation. */
unsigned int flags = atomic_load_relaxed (&cond->__data.__wrefs);
clockid_t clock = (flags & __PTHREAD_COND_CLOCK_MONOTONIC_MASK)
? CLOCK_MONOTONIC : CLOCK_REALTIME;
return __pthread_cond_wait_common (cond, mutex, clock, abstime);

OK. I too was worried about that load, but persuaded myself that it was
safe for the same reasons. I think you're correct that it should be made
explicit and explained.

Thanks again for your review.

Mike.

[Adhemerval Zanella]

Post by Mike Crowe

Post by Adhemerval Zanella

Post by Mike Crowe
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 condition
variable 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.
Let's invent 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.
Earlier versions of this patch were proposed in 2015 but further progress
stalled waiting for the assembly implementation of pthread_cond_timedwait.S
to be removed by Torvald Riegel. This happened in
ed19993b5b0d05d62cc883571519a67dae481a14.
* https://sourceware.org/ml/libc-alpha/2015-07/msg00193.html
* https://sourceware.org/ml/libc-alpha/2015-08/msg00186.html
* https://sourceware.org/ml/libc-alpha/2015-08/msg00230.html
Some of the abilist updates for other architectures appear to have gone
missing during the rebase. If this change is reviewed favourably then I
shall try to include them.

I think it is a reasonable addition, however I have some concerns if this
would just another bad discussed ABI to fix and specific issues that will
be replaced by another better or simple solution (as your last comment in
BZ#41861 [1]). It also concerns that it was not brought to Austin Group
attention as suggested by Szabolcs [2].

I've now submitted the suggestion. It can be viewed at
http://austingroupbugs.net/view.php?id=1164
[Snipped uncontentious parts of patch and suggestions for brevity.]

Thanks, I think this is the best way forward, although it might require
some time to get Austin group any reply. If and once we get a positive
indication for API inclusion, we speed up by adding as a GNU extension
for upcoming releases and later adding in the default API.

Post by Mike Crowe

Post by Adhemerval Zanella

Post by Mike Crowe
diff --git a/nptl/Versions b/nptl/Versions
index 0ae5def464..1cf8c2fbad 100644
--- a/nptl/Versions
+++ b/nptl/Versions
@@ -28,6 +28,9 @@ libc {
pthread_cond_wait; pthread_cond_signal;
pthread_cond_broadcast; pthread_cond_timedwait;
}
+ GLIBC_2.26 {
+ pthread_cond_timedwaitonclock_np;
+ }

The libc addition seems wrong, whhy this is required?

pthread_cond_timedwait is in libc, so I followed suite and added
pthread_cond_timedwaitonclock_np too. I must admit that I can't quite see
how the dummy empty implementations of the pthread functions end up in
https://stackoverflow.com/questions/11161462/why-glibc-and-pthread-library-both-defined-same-apis
Maybe this means that I've missed something.

This is an GLIB implementation detail, where some libraries which
are expected to be usable without explicit linking to libpthread use
some pthread functions (for instance aio from librt).

In the single thread case libc.so uses wrappers that may or not call the
libpthread.so implementation (nptl/forward.c) if libpthread.so.0 is
loaded (by dlopen for instance).

So since there is no current use of pthread_cond_timedwaitonclock_np,
there is no requirement to add it on libc.so.

Post by Mike Crowe

Post by Adhemerval Zanella

Post by Mike Crowe
/* See __pthread_cond_wait_common. */
@@ -664,10 +665,31 @@ __pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
it can assume that abstime is not NULL. */
if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
return EINVAL;
- return __pthread_cond_wait_common (cond, mutex, abstime);
+
+ clockid_t clockid = ((cond->__data.__wrefs & __PTHREAD_COND_CLOCK_MONOTONIC_MASK) != 0) ? CLOCK_MONOTONIC : CLOCK_REALTIME;
+ return __pthread_cond_wait_common (cond, mutex, clockid, abstime);
+}

Line to long and I think it is required to state why a plain load to
__wrefs in this specific case is suffice for synchronization. In fact
/* Relaxed MO is suffice because clock ID bit is only modified
in condition creation. */
unsigned int flags = atomic_load_relaxed (&cond->__data.__wrefs);
clockid_t clock = (flags & __PTHREAD_COND_CLOCK_MONOTONIC_MASK)
? CLOCK_MONOTONIC : CLOCK_REALTIME;
return __pthread_cond_wait_common (cond, mutex, clock, abstime);

OK. I too was worried about that load, but persuaded myself that it was
safe for the same reasons. I think you're correct that it should be made
explicit and explained.
Thanks again for your review.
Mike.

I think you can add a comment stating that since it is undefined behaviour
to call pthread_mutex_timedwait with a cond or mutex argument which has not
been initialized, the __wrefs bits used for clockid is not expected to be
set concurrently and so it should be safe to use a relaxed load in this
case.