From 6df8c327532627d6a99991993c52e8e4a9b34968 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 1 Oct 2021 18:22:49 +0200 Subject: 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. --- .../2021-09-30-23-00-18.bpo-41710.svuloZ.rst | 5 ++ Python/thread_pthread.h | 53 +++++++++++++++++----- configure | 2 +- configure.ac | 2 +- pyconfig.h.in | 3 ++ 5 files changed, 51 insertions(+), 14 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2021-09-30-23-00-18.bpo-41710.svuloZ.rst 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 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 a45d842..35b9810 100644 --- 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 1580117..753f956 100755 --- 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 a75fdf9..41a3679 100644 --- 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 49407ab..0559274 100644 --- 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 -- cgit v0.12