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

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.
(cherry picked from commit 6df8c32753)

Co-authored-by: Victor Stinner <vstinner@python.org>
This commit is contained in:
Miss Islington (bot) 2021-10-01 09:51:15 -07:00 committed by GitHub
parent 8822526caa
commit 0e1aeab5d7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 51 additions and 14 deletions

View file

@ -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.

View file

@ -89,12 +89,17 @@
* mutexes and condition variables: * mutexes and condition variables:
*/ */
#if (defined(_POSIX_SEMAPHORES) && !defined(HAVE_BROKEN_POSIX_SEMAPHORES) && \ #if (defined(_POSIX_SEMAPHORES) && !defined(HAVE_BROKEN_POSIX_SEMAPHORES) && \
defined(HAVE_SEM_TIMEDWAIT)) (defined(HAVE_SEM_TIMEDWAIT) || defined(HAVE_SEM_CLOCKWAIT)))
# define USE_SEMAPHORES # define USE_SEMAPHORES
#else #else
# undef USE_SEMAPHORES # undef USE_SEMAPHORES
#endif #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() /* On platforms that don't use standard POSIX threads pthread_sigmask()
* isn't present. DEC threads uses sigprocmask() instead as do most * isn't present. DEC threads uses sigprocmask() instead as do most
@ -120,16 +125,23 @@ do { \
ts.tv_nsec = tv.tv_usec * 1000; \ ts.tv_nsec = tv.tv_usec * 1000; \
} while(0) } 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 * 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. // NULL when pthread_condattr_setclock(CLOCK_MONOTONIC) is not supported.
static pthread_condattr_t *condattr_monotonic = NULL; static pthread_condattr_t *condattr_monotonic = NULL;
@ -151,16 +163,13 @@ _PyThread_cond_init(PyCOND_T *cond)
return pthread_cond_init(cond, condattr_monotonic); return pthread_cond_init(cond, condattr_monotonic);
} }
void void
_PyThread_cond_after(long long us, struct timespec *abs) _PyThread_cond_after(long long us, struct timespec *abs)
{ {
#ifdef CONDATTR_MONOTONIC #ifdef CONDATTR_MONOTONIC
if (condattr_monotonic) { if (condattr_monotonic) {
clock_gettime(CLOCK_MONOTONIC, abs); monotonic_abs_timeout(us, abs);
abs->tv_sec += us / 1000000;
abs->tv_nsec += (us % 1000000) * 1000;
abs->tv_sec += abs->tv_nsec / 1000000000;
abs->tv_nsec %= 1000000000;
return; return;
} }
#endif #endif
@ -431,7 +440,9 @@ PyThread_acquire_lock_timed(PyThread_type_lock lock, PY_TIMEOUT_T microseconds,
sem_t *thelock = (sem_t *)lock; sem_t *thelock = (sem_t *)lock;
int status, error = 0; int status, error = 0;
struct timespec ts; struct timespec ts;
#ifndef HAVE_SEM_CLOCKWAIT
_PyTime_t deadline = 0; _PyTime_t deadline = 0;
#endif
(void) error; /* silence unused-but-set-variable warning */ (void) error; /* silence unused-but-set-variable warning */
dprintf(("PyThread_acquire_lock_timed(%p, %lld, %d) called\n", dprintf(("PyThread_acquire_lock_timed(%p, %lld, %d) called\n",
@ -442,6 +453,9 @@ PyThread_acquire_lock_timed(PyThread_type_lock lock, PY_TIMEOUT_T microseconds,
} }
if (microseconds > 0) { if (microseconds > 0) {
#ifdef HAVE_SEM_CLOCKWAIT
monotonic_abs_timeout(microseconds, &ts);
#else
MICROSECONDS_TO_TIMESPEC(microseconds, ts); MICROSECONDS_TO_TIMESPEC(microseconds, ts);
if (!intr_flag) { if (!intr_flag) {
@ -450,11 +464,17 @@ PyThread_acquire_lock_timed(PyThread_type_lock lock, PY_TIMEOUT_T microseconds,
_PyTime_t timeout = _PyTime_FromNanoseconds(microseconds * 1000); _PyTime_t timeout = _PyTime_FromNanoseconds(microseconds * 1000);
deadline = _PyTime_GetMonotonicClock() + timeout; deadline = _PyTime_GetMonotonicClock() + timeout;
} }
#endif
} }
while (1) { while (1) {
if (microseconds > 0) { if (microseconds > 0) {
#ifdef HAVE_SEM_CLOCKWAIT
status = fix_status(sem_clockwait(thelock, CLOCK_MONOTONIC,
&ts));
#else
status = fix_status(sem_timedwait(thelock, &ts)); status = fix_status(sem_timedwait(thelock, &ts));
#endif
} }
else if (microseconds == 0) { else if (microseconds == 0) {
status = fix_status(sem_trywait(thelock)); status = fix_status(sem_trywait(thelock));
@ -469,6 +489,9 @@ PyThread_acquire_lock_timed(PyThread_type_lock lock, PY_TIMEOUT_T microseconds,
break; break;
} }
// sem_clockwait() uses an absolute timeout, there is no need
// to recompute the relative timeout.
#ifndef HAVE_SEM_CLOCKWAIT
if (microseconds > 0) { if (microseconds > 0) {
/* wait interrupted by a signal (EINTR): recompute the timeout */ /* wait interrupted by a signal (EINTR): recompute the timeout */
_PyTime_t dt = deadline - _PyTime_GetMonotonicClock(); _PyTime_t dt = deadline - _PyTime_GetMonotonicClock();
@ -490,13 +513,19 @@ PyThread_acquire_lock_timed(PyThread_type_lock lock, PY_TIMEOUT_T microseconds,
microseconds = 0; microseconds = 0;
} }
} }
#endif
} }
/* Don't check the status if we're stopping because of an interrupt. */ /* Don't check the status if we're stopping because of an interrupt. */
if (!(intr_flag && status == EINTR)) { if (!(intr_flag && status == EINTR)) {
if (microseconds > 0) { if (microseconds > 0) {
if (status != ETIMEDOUT) if (status != ETIMEDOUT) {
#ifdef HAVE_SEM_CLOCKWAIT
CHECK_STATUS("sem_clockwait");
#else
CHECK_STATUS("sem_timedwait"); CHECK_STATUS("sem_timedwait");
#endif
}
} }
else if (microseconds == 0) { else if (microseconds == 0) {
if (status != EAGAIN) if (status != EAGAIN)

2
configure vendored
View file

@ -11723,7 +11723,7 @@ for ac_func in alarm accept4 setitimer getitimer bind_textdomain_codeset chown \
posix_fallocate posix_fadvise posix_spawn posix_spawnp pread preadv preadv2 \ posix_fallocate posix_fadvise posix_spawn posix_spawnp pread preadv preadv2 \
pthread_condattr_setclock pthread_init pthread_kill pwrite pwritev pwritev2 \ pthread_condattr_setclock pthread_init pthread_kill pwrite pwritev pwritev2 \
readlink readlinkat readv realpath renameat \ 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 \ setgid sethostname \
setlocale setregid setreuid setresuid setresgid setsid setpgid setpgrp setpriority setuid setvbuf \ setlocale setregid setreuid setresuid setresgid setsid setpgid setpgrp setpriority setuid setvbuf \
sched_get_priority_max sched_setaffinity sched_setscheduler sched_setparam \ sched_get_priority_max sched_setaffinity sched_setscheduler sched_setparam \

View file

@ -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 \ posix_fallocate posix_fadvise posix_spawn posix_spawnp pread preadv preadv2 \
pthread_condattr_setclock pthread_init pthread_kill pwrite pwritev pwritev2 \ pthread_condattr_setclock pthread_init pthread_kill pwrite pwritev pwritev2 \
readlink readlinkat readv realpath renameat \ 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 \ setgid sethostname \
setlocale setregid setreuid setresuid setresgid setsid setpgid setpgrp setpriority setuid setvbuf \ setlocale setregid setreuid setresuid setresgid setsid setpgid setpgrp setpriority setuid setvbuf \
sched_get_priority_max sched_setaffinity sched_setscheduler sched_setparam \ sched_get_priority_max sched_setaffinity sched_setscheduler sched_setparam \

View file

@ -887,6 +887,9 @@
/* Define to 1 if you have the `sched_setscheduler' function. */ /* Define to 1 if you have the `sched_setscheduler' function. */
#undef HAVE_SCHED_SETSCHEDULER #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. */ /* Define to 1 if you have the `sem_getvalue' function. */
#undef HAVE_SEM_GETVALUE #undef HAVE_SEM_GETVALUE