From: Victor Stinner Date: Fri, 1 Oct 2021 16:22:49 +0000 (+0200) Subject: bpo-41710: PyThread_acquire_lock_timed() uses sem_clockwait() (GH-28671) X-Git-Tag: v3.10.1~237 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=6df8c327532627d6a99991993c52e8e4a9b34968;p=thirdparty%2FPython%2Fcpython.git 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 index 000000000000..d8a4f9507c18 --- /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 a45d842ffe73..35b9810aa377 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 15801175a3dd..753f9564693b 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 a75fdf918c7e..41a3679d3dc5 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 49407ab62b41..0559274b4768 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