[Python-checkins] bpo-41710: PyThread_acquire_lock_timed() clamps the timout (GH-28643)
vstinner
webhook-mailer at python.org
Thu Sep 30 04:17:03 EDT 2021
https://github.com/python/cpython/commit/37b8294d6295ca12553fd7c98778be71d24f4b24
commit: 37b8294d6295ca12553fd7c98778be71d24f4b24
branch: main
author: Victor Stinner <vstinner at python.org>
committer: vstinner <vstinner at 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");
}
More information about the Python-checkins
mailing list