https://github.com/python/cpython/commit/37b8294d6295ca12553fd7c98778be71d24... commit: 37b8294d6295ca12553fd7c98778be71d24f4b24 branch: main author: Victor Stinner <vstinner@python.org> committer: vstinner <vstinner@python.org> date: 2021-09-30T10:16:51+02:00 summary: bpo-41710: PyThread_acquire_lock_timed() clamps the timout (GH-28643) PyThread_acquire_lock_timed() now clamps the timeout into the [_PyTime_MIN; _PyTime_MAX] range (_PyTime_t type) if it is too large, rather than calling Py_FatalError() which aborts the process. PyThread_acquire_lock_timed() no longer uses MICROSECONDS_TO_TIMESPEC() to compute sem_timedwait() argument, but _PyTime_GetSystemClock() and _PyTime_AsTimespec_truncate(). Fix _thread.TIMEOUT_MAX value on Windows: the maximum timeout is 0x7FFFFFFF milliseconds (around 24.9 days), not 0xFFFFFFFF milliseconds (around 49.7 days). Set PY_TIMEOUT_MAX to 0x7FFFFFFF milliseconds, rather than 0xFFFFFFFF milliseconds. Fix PY_TIMEOUT_MAX overflow test: replace (us >= PY_TIMEOUT_MAX) with (us > PY_TIMEOUT_MAX). files: A Misc/NEWS.d/next/C API/2021-09-30-03-14-35.bpo-41710.DDWJKx.rst A Misc/NEWS.d/next/Library/2021-09-30-08-57-50.bpo-41710.JMsPAW.rst M Include/cpython/pytime.h M Include/pythread.h M Modules/_queuemodule.c M Modules/_threadmodule.c M Modules/faulthandler.c M Python/thread_nt.h M Python/thread_pthread.h diff --git a/Include/cpython/pytime.h b/Include/cpython/pytime.h index b5a351349f85b..db3adfab6a9ab 100644 --- a/Include/cpython/pytime.h +++ b/Include/cpython/pytime.h @@ -14,7 +14,9 @@ extern "C" { store a duration, and so indirectly a date (related to another date, like UNIX epoch). */ typedef int64_t _PyTime_t; +// _PyTime_MIN nanoseconds is around -292.3 years #define _PyTime_MIN INT64_MIN +// _PyTime_MAX nanoseconds is around +292.3 years #define _PyTime_MAX INT64_MAX #define _SIZEOF_PYTIME_T 8 diff --git a/Include/pythread.h b/Include/pythread.h index bb9d86412218a..cf4cc9a7473f1 100644 --- a/Include/pythread.h +++ b/Include/pythread.h @@ -61,9 +61,11 @@ PyAPI_FUNC(int) _PyThread_at_fork_reinit(PyThread_type_lock *lock); convert microseconds to nanoseconds. */ # define PY_TIMEOUT_MAX (LLONG_MAX / 1000) #elif defined (NT_THREADS) - /* In the NT API, the timeout is a DWORD and is expressed in milliseconds */ -# if 0xFFFFFFFFLL * 1000 < LLONG_MAX -# define PY_TIMEOUT_MAX (0xFFFFFFFFLL * 1000) + /* In the NT API, the timeout is a DWORD and is expressed in milliseconds, + * a positive number between 0 and 0x7FFFFFFF (see WaitForSingleObject() + * documentation). */ +# if 0x7FFFFFFFLL * 1000 < LLONG_MAX +# define PY_TIMEOUT_MAX (0x7FFFFFFFLL * 1000) # else # define PY_TIMEOUT_MAX LLONG_MAX # endif diff --git a/Misc/NEWS.d/next/C API/2021-09-30-03-14-35.bpo-41710.DDWJKx.rst b/Misc/NEWS.d/next/C API/2021-09-30-03-14-35.bpo-41710.DDWJKx.rst new file mode 100644 index 0000000000000..902c7cc8f2b99 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2021-09-30-03-14-35.bpo-41710.DDWJKx.rst @@ -0,0 +1,2 @@ +The PyThread_acquire_lock_timed() function now clamps the timeout if it is +too large, rather than aborting the process. Patch by Victor Stinner. diff --git a/Misc/NEWS.d/next/Library/2021-09-30-08-57-50.bpo-41710.JMsPAW.rst b/Misc/NEWS.d/next/Library/2021-09-30-08-57-50.bpo-41710.JMsPAW.rst new file mode 100644 index 0000000000000..516214a619e8e --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-09-30-08-57-50.bpo-41710.JMsPAW.rst @@ -0,0 +1,3 @@ +Fix :data:`_thread.TIMEOUT_MAX` value on Windows: the maximum timeout is +0x7FFFFFFF milliseconds (around 24.9 days), not 0xFFFFFFFF milliseconds (around +49.7 days). diff --git a/Modules/_queuemodule.c b/Modules/_queuemodule.c index 58772d7f0f523..b6769e6b7764e 100644 --- a/Modules/_queuemodule.c +++ b/Modules/_queuemodule.c @@ -223,7 +223,7 @@ _queue_SimpleQueue_get_impl(simplequeueobject *self, PyTypeObject *cls, } microseconds = _PyTime_AsMicroseconds(timeout_val, _PyTime_ROUND_CEILING); - if (microseconds >= PY_TIMEOUT_MAX) { + if (microseconds > PY_TIMEOUT_MAX) { PyErr_SetString(PyExc_OverflowError, "timeout value is too large"); return NULL; diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 5b5d2c5b03ec3..fa7e6d0e09d18 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -164,7 +164,7 @@ lock_acquire_parse_args(PyObject *args, PyObject *kwds, _PyTime_t microseconds; microseconds = _PyTime_AsMicroseconds(*timeout, _PyTime_ROUND_TIMEOUT); - if (microseconds >= PY_TIMEOUT_MAX) { + if (microseconds > PY_TIMEOUT_MAX) { PyErr_SetString(PyExc_OverflowError, "timeout value is too large"); return -1; diff --git a/Modules/faulthandler.c b/Modules/faulthandler.c index 350f4cf6b8edf..868b4f4f42de6 100644 --- a/Modules/faulthandler.c +++ b/Modules/faulthandler.c @@ -710,7 +710,7 @@ faulthandler_dump_traceback_later(PyObject *self, return NULL; } /* Limit to LONG_MAX seconds for format_timeout() */ - if (timeout_us >= PY_TIMEOUT_MAX || timeout_us / SEC_TO_US >= LONG_MAX) { + if (timeout_us > PY_TIMEOUT_MAX || timeout_us / SEC_TO_US > LONG_MAX) { PyErr_SetString(PyExc_OverflowError, "timeout value is too large"); return NULL; diff --git a/Python/thread_nt.h b/Python/thread_nt.h index f8c098ca5238a..0beb3d3af2fd3 100644 --- a/Python/thread_nt.h +++ b/Python/thread_nt.h @@ -292,6 +292,10 @@ PyThread_free_lock(PyThread_type_lock aLock) FreeNonRecursiveMutex(aLock) ; } +// WaitForSingleObject() documentation: "The time-out value needs to be a +// positive number between 0 and 0x7FFFFFFF." INFINITE is equal to 0xFFFFFFFF. +const DWORD TIMEOUT_MS_MAX = 0x7FFFFFFF; + /* * Return 1 on success if the lock was acquired * @@ -309,10 +313,20 @@ PyThread_acquire_lock_timed(PyThread_type_lock aLock, if (microseconds >= 0) { milliseconds = microseconds / 1000; - if (microseconds % 1000 > 0) - ++milliseconds; - if (milliseconds > PY_DWORD_MAX) { - Py_FatalError("Timeout larger than PY_TIMEOUT_MAX"); + // Round milliseconds away from zero + if (microseconds % 1000 > 0) { + milliseconds++; + } + if (milliseconds > (PY_TIMEOUT_T)TIMEOUT_MS_MAX) { + // bpo-41710: PyThread_acquire_lock_timed() cannot report timeout + // overflow to the caller, so clamp the timeout to + // [0, TIMEOUT_MS_MAX] milliseconds. + // + // TIMEOUT_MS_MAX milliseconds is around 24.9 days. + // + // _thread.Lock.acquire() and _thread.RLock.acquire() raise an + // OverflowError if microseconds is greater than PY_TIMEOUT_MAX. + milliseconds = TIMEOUT_MS_MAX; } } else { diff --git a/Python/thread_pthread.h b/Python/thread_pthread.h index 7f04151ca91fd..3815ffae20c01 100644 --- a/Python/thread_pthread.h +++ b/Python/thread_pthread.h @@ -433,33 +433,47 @@ PyThread_acquire_lock_timed(PyThread_type_lock lock, PY_TIMEOUT_T microseconds, PyLockStatus success; sem_t *thelock = (sem_t *)lock; int status, error = 0; - struct timespec ts; - _PyTime_t deadline = 0; (void) error; /* silence unused-but-set-variable warning */ dprintf(("PyThread_acquire_lock_timed(%p, %lld, %d) called\n", lock, microseconds, intr_flag)); - if (microseconds > PY_TIMEOUT_MAX) { - Py_FatalError("Timeout larger than PY_TIMEOUT_MAX"); + _PyTime_t timeout; + if (microseconds >= 0) { + _PyTime_t ns; + if (microseconds <= _PyTime_MAX / 1000) { + ns = microseconds * 1000; + } + else { + // bpo-41710: PyThread_acquire_lock_timed() cannot report timeout + // overflow to the caller, so clamp the timeout to + // [_PyTime_MIN, _PyTime_MAX]. + // + // _PyTime_MAX nanoseconds is around 292.3 years. + // + // _thread.Lock.acquire() and _thread.RLock.acquire() raise an + // OverflowError if microseconds is greater than PY_TIMEOUT_MAX. + ns = _PyTime_MAX; + } + timeout = _PyTime_FromNanoseconds(ns); + } + else { + timeout = _PyTime_FromNanoseconds(-1); } - if (microseconds > 0) { - MICROSECONDS_TO_TIMESPEC(microseconds, ts); - - if (!intr_flag) { - /* cannot overflow thanks to (microseconds > PY_TIMEOUT_MAX) - check done above */ - _PyTime_t timeout = _PyTime_FromNanoseconds(microseconds * 1000); - deadline = _PyTime_GetMonotonicClock() + timeout; - } + _PyTime_t deadline = 0; + if (timeout > 0 && !intr_flag) { + deadline = _PyTime_GetMonotonicClock() + timeout; } while (1) { - if (microseconds > 0) { + if (timeout > 0) { + _PyTime_t t = _PyTime_GetSystemClock() + timeout; + struct timespec ts; + _PyTime_AsTimespec_clamp(t, &ts); status = fix_status(sem_timedwait(thelock, &ts)); } - else if (microseconds == 0) { + else if (timeout == 0) { status = fix_status(sem_trywait(thelock)); } else { @@ -472,32 +486,23 @@ PyThread_acquire_lock_timed(PyThread_type_lock lock, PY_TIMEOUT_T microseconds, break; } - if (microseconds > 0) { + if (timeout > 0) { /* wait interrupted by a signal (EINTR): recompute the timeout */ - _PyTime_t dt = deadline - _PyTime_GetMonotonicClock(); - if (dt < 0) { + _PyTime_t timeout = deadline - _PyTime_GetMonotonicClock(); + if (timeout < 0) { status = ETIMEDOUT; break; } - else if (dt > 0) { - _PyTime_t realtime_deadline = _PyTime_GetSystemClock() + dt; - _PyTime_AsTimespec_clamp(realtime_deadline, &ts); - /* no need to update microseconds value, the code only care - if (microseconds > 0 or (microseconds == 0). */ - } - else { - microseconds = 0; - } } } /* Don't check the status if we're stopping because of an interrupt. */ if (!(intr_flag && status == EINTR)) { - if (microseconds > 0) { + if (timeout > 0) { if (status != ETIMEDOUT) CHECK_STATUS("sem_timedwait"); } - else if (microseconds == 0) { + else if (timeout == 0) { if (status != EAGAIN) CHECK_STATUS("sem_trywait"); }