bpo-41710: PyThread_acquire_lock_timed() uses sem_clockwait() (GH-28671)

On Unix, if the sem_clockwait() function is available in the C
library (glibc 2.30 and newer), the threading.Lock.acquire() method
now uses the monotonic clock (time.CLOCK_MONOTONIC) for the timeout,
rather than using the system clock (time.CLOCK_REALTIME), to not be
affected by system clock changes.

configure now checks if the sem_clockwait() function is available.

diff --git a/Misc/NEWS.d/next/Library/2021-09-30-23-00-18.bpo-41710.svuloZ.rst b/Misc/NEWS.d/next/Library/2021-09-30-23-00-18.bpo-41710.svuloZ.rst
new file mode 100644 (file)
index 0000000..d8a4f95
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2021-09-30-23-00-18.bpo-41710.svuloZ.rst
@@ -0,0 +1,5 @@
+On Unix, if the ``sem_clockwait()`` function is available in the C library
+(glibc 2.30 and newer), the :meth:`threading.Lock.acquire` method now uses the
+monotonic clock (:data:`time.CLOCK_MONOTONIC`) for the timeout, rather than
+using the system clock (:data:`time.CLOCK_REALTIME`), to not be affected by
+system clock changes. Patch by Victor Stinner.

diff --git a/Python/thread_pthread.h b/Python/thread_pthread.h
index a45d842ffe73d2417e38b3f0b04d4265d32220ab..35b9810aa377f25dfedf505400e5cba916d0423f 100644 (file)
--- a/Python/thread_pthread.h
+++ b/Python/thread_pthread.h
@@ -92,12 +92,17 @@
  * mutexes and condition variables:
  */
 #if (defined(_POSIX_SEMAPHORES) && !defined(HAVE_BROKEN_POSIX_SEMAPHORES) && \
-     defined(HAVE_SEM_TIMEDWAIT))
+     (defined(HAVE_SEM_TIMEDWAIT) || defined(HAVE_SEM_CLOCKWAIT)))
 #  define USE_SEMAPHORES
 #else
 #  undef USE_SEMAPHORES
 #endif
 
+#if defined(HAVE_PTHREAD_CONDATTR_SETCLOCK) && defined(HAVE_CLOCK_GETTIME) && defined(CLOCK_MONOTONIC)
+// monotonic is supported statically.  It doesn't mean it works on runtime.
+#define CONDATTR_MONOTONIC
+#endif
+
 
 /* On platforms that don't use standard POSIX threads pthread_sigmask()
  * isn't present.  DEC threads uses sigprocmask() instead as do most
@@ -123,16 +128,23 @@ do { \
     ts.tv_nsec = tv.tv_usec * 1000; \
 } while(0)
 
+#if defined(CONDATTR_MONOTONIC) || defined(HAVE_SEM_CLOCKWAIT)
+static void
+monotonic_abs_timeout(long long us, struct timespec *abs)
+{
+    clock_gettime(CLOCK_MONOTONIC, abs);
+    abs->tv_sec  += us / 1000000;
+    abs->tv_nsec += (us % 1000000) * 1000;
+    abs->tv_sec  += abs->tv_nsec / 1000000000;
+    abs->tv_nsec %= 1000000000;
+}
+#endif
+
 
 /*
  * pthread_cond support
  */
 
-#if defined(HAVE_PTHREAD_CONDATTR_SETCLOCK) && defined(HAVE_CLOCK_GETTIME) && defined(CLOCK_MONOTONIC)
-// monotonic is supported statically.  It doesn't mean it works on runtime.
-#define CONDATTR_MONOTONIC
-#endif
-
 // NULL when pthread_condattr_setclock(CLOCK_MONOTONIC) is not supported.
 static pthread_condattr_t *condattr_monotonic = NULL;
 
@@ -154,16 +166,13 @@ _PyThread_cond_init(PyCOND_T *cond)
     return pthread_cond_init(cond, condattr_monotonic);
 }
 
+
 void
 _PyThread_cond_after(long long us, struct timespec *abs)
 {
 #ifdef CONDATTR_MONOTONIC
     if (condattr_monotonic) {
-        clock_gettime(CLOCK_MONOTONIC, abs);
-        abs->tv_sec  += us / 1000000;
-        abs->tv_nsec += (us % 1000000) * 1000;
-        abs->tv_sec  += abs->tv_nsec / 1000000000;
-        abs->tv_nsec %= 1000000000;
+        monotonic_abs_timeout(us, abs);
         return;
     }
 #endif
@@ -434,7 +443,9 @@ PyThread_acquire_lock_timed(PyThread_type_lock lock, PY_TIMEOUT_T microseconds,
     sem_t *thelock = (sem_t *)lock;
     int status, error = 0;
     struct timespec ts;
+#ifndef HAVE_SEM_CLOCKWAIT
     _PyTime_t deadline = 0;
+#endif
 
     (void) error; /* silence unused-but-set-variable warning */
     dprintf(("PyThread_acquire_lock_timed(%p, %lld, %d) called\n",
@@ -445,6 +456,9 @@ PyThread_acquire_lock_timed(PyThread_type_lock lock, PY_TIMEOUT_T microseconds,
     }
 
     if (microseconds > 0) {
+#ifdef HAVE_SEM_CLOCKWAIT
+        monotonic_abs_timeout(microseconds, &ts);
+#else
         MICROSECONDS_TO_TIMESPEC(microseconds, ts);
 
         if (!intr_flag) {
@@ -453,11 +467,17 @@ PyThread_acquire_lock_timed(PyThread_type_lock lock, PY_TIMEOUT_T microseconds,
             _PyTime_t timeout = _PyTime_FromNanoseconds(microseconds * 1000);
             deadline = _PyTime_GetMonotonicClock() + timeout;
         }
+#endif
     }
 
     while (1) {
         if (microseconds > 0) {
+#ifdef HAVE_SEM_CLOCKWAIT
+            status = fix_status(sem_clockwait(thelock, CLOCK_MONOTONIC,
+                                              &ts));
+#else
             status = fix_status(sem_timedwait(thelock, &ts));
+#endif
         }
         else if (microseconds == 0) {
             status = fix_status(sem_trywait(thelock));
@@ -472,6 +492,9 @@ PyThread_acquire_lock_timed(PyThread_type_lock lock, PY_TIMEOUT_T microseconds,
             break;
         }
 
+        // sem_clockwait() uses an absolute timeout, there is no need
+        // to recompute the relative timeout.
+#ifndef HAVE_SEM_CLOCKWAIT
         if (microseconds > 0) {
             /* wait interrupted by a signal (EINTR): recompute the timeout */
             _PyTime_t dt = deadline - _PyTime_GetMonotonicClock();
@@ -493,13 +516,19 @@ PyThread_acquire_lock_timed(PyThread_type_lock lock, PY_TIMEOUT_T microseconds,
                 microseconds = 0;
             }
         }
+#endif
     }
 
     /* Don't check the status if we're stopping because of an interrupt.  */
     if (!(intr_flag && status == EINTR)) {
         if (microseconds > 0) {
-            if (status != ETIMEDOUT)
+            if (status != ETIMEDOUT) {
+#ifdef HAVE_SEM_CLOCKWAIT
+                CHECK_STATUS("sem_clockwait");
+#else
                 CHECK_STATUS("sem_timedwait");
+#endif
+            }
         }
         else if (microseconds == 0) {
             if (status != EAGAIN)

diff --git a/configure b/configure
index 15801175a3dd5cc3ea219cfe33a469ed45e652f8..753f9564693bce636fc7f2f5b0b399715aff88ad 100755 (executable)
--- a/configure
+++ b/configure
@@ -11738,7 +11738,7 @@ for ac_func in alarm accept4 setitimer getitimer bind_textdomain_codeset chown \
  posix_fallocate posix_fadvise posix_spawn posix_spawnp pread preadv preadv2 \
  pthread_condattr_setclock pthread_init pthread_kill pwrite pwritev pwritev2 \
  readlink readlinkat readv realpath renameat \
- sem_open sem_timedwait sem_getvalue sem_unlink sendfile setegid seteuid \
+ sem_open sem_timedwait sem_clockwait sem_getvalue sem_unlink sendfile setegid seteuid \
  setgid sethostname \
  setlocale setregid setreuid setresuid setresgid setsid setpgid setpgrp setpriority setuid setvbuf \
  sched_get_priority_max sched_setaffinity sched_setscheduler sched_setparam \

diff --git a/configure.ac b/configure.ac
index a75fdf918c7ea8250a346b672856ece523f6b5f4..41a3679d3dc5587de23dee7240fe04280899830c 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -3707,7 +3707,7 @@ AC_CHECK_FUNCS(alarm accept4 setitimer getitimer bind_textdomain_codeset chown \
  posix_fallocate posix_fadvise posix_spawn posix_spawnp pread preadv preadv2 \
  pthread_condattr_setclock pthread_init pthread_kill pwrite pwritev pwritev2 \
  readlink readlinkat readv realpath renameat \
- sem_open sem_timedwait sem_getvalue sem_unlink sendfile setegid seteuid \
+ sem_open sem_timedwait sem_clockwait sem_getvalue sem_unlink sendfile setegid seteuid \
  setgid sethostname \
  setlocale setregid setreuid setresuid setresgid setsid setpgid setpgrp setpriority setuid setvbuf \
  sched_get_priority_max sched_setaffinity sched_setscheduler sched_setparam \

diff --git a/pyconfig.h.in b/pyconfig.h.in
index 49407ab62b417d7ccb679e1c5f738d33c52bf95b..0559274b4768c43dfe720aec586c7f66590560ac 100644 (file)
--- a/pyconfig.h.in
+++ b/pyconfig.h.in
@@ -902,6 +902,9 @@
 /* Define to 1 if you have the `sched_setscheduler' function. */
 #undef HAVE_SCHED_SETSCHEDULER
 
+/* Define to 1 if you have the `sem_clockwait' function. */
+#undef HAVE_SEM_CLOCKWAIT
+
 /* Define to 1 if you have the `sem_getvalue' function. */
 #undef HAVE_SEM_GETVALUE