[Python-checkins] bpo-30768: Recompute timeout on interrupted lock (GH-4103)

Tue Oct 24 19:53:36 EDT 2017

https://github.com/python/cpython/commit/850a18e03e8f8309bc8c39adc6e7d51a4568cd9a
commit: 850a18e03e8f8309bc8c39adc6e7d51a4568cd9a
branch: master
author: Victor Stinner <victor.stinner at gmail.com>
committer: GitHub <noreply at github.com>
date: 2017年10月24日T16:53:32-07:00
summary:
bpo-30768: Recompute timeout on interrupted lock (GH-4103)
Fix the pthread+semaphore implementation of
PyThread_acquire_lock_timed() when called with timeout > 0 and
intr_flag=0: recompute the timeout if sem_timedwait() is interrupted
by a signal (EINTR).
See also the PEP 475.
The pthread implementation of PyThread_acquire_lock() now fails with
a fatal error if the timeout is larger than PY_TIMEOUT_MAX, as done
in the Windows implementation.
The check prevents any risk of overflow in PyThread_acquire_lock().
Add also PY_DWORD_MAX constant.
files:
A Misc/NEWS.d/next/Library/2017-10-24-12-00-16.bpo-30768.Om8Yj_.rst
M Include/pyport.h
M Include/pythread.h
M Modules/_threadmodule.c
M Modules/_winapi.c
M Modules/clinic/_winapi.c.h
M Modules/posixmodule.c
M Python/thread_nt.h
M Python/thread_pthread.h

diff --git a/Include/pyport.h b/Include/pyport.h
index 2742e477fd4..0e82543ac78 100644
--- a/Include/pyport.h
+++ b/Include/pyport.h
@@ -787,6 +787,9 @@ extern _invalid_parameter_handler _Py_silent_invalid_parameter_handler;
 #include <android/api-level.h>
 #endif
 
+/* Maximum value of the Windows DWORD type */
+#define PY_DWORD_MAX 4294967295U
+
 /* This macro used to tell whether Python was built with multithreading
 * enabled. Now multithreading is always enabled, but keep the macro
 * for compatibility.
diff --git a/Include/pythread.h b/Include/pythread.h
index d6674685f28..eb61033b2d9 100644
--- a/Include/pythread.h
+++ b/Include/pythread.h
@@ -42,16 +42,23 @@ PyAPI_FUNC(int) PyThread_acquire_lock(PyThread_type_lock, int);
 and floating-point numbers allowed.
 */
 #define PY_TIMEOUT_T long long
-#define PY_TIMEOUT_MAX PY_LLONG_MAX
 
-/* In the NT API, the timeout is a DWORD and is expressed in milliseconds */
-#if defined (NT_THREADS)
-#if 0xFFFFFFFFLL * 1000 < PY_TIMEOUT_MAX
-#undef PY_TIMEOUT_MAX
-#define PY_TIMEOUT_MAX (0xFFFFFFFFLL * 1000)
-#endif
+#if defined(_POSIX_THREADS)
+ /* PyThread_acquire_lock_timed() uses _PyTime_FromNanoseconds(us * 1000),
+ convert microseconds to nanoseconds. */
+# define PY_TIMEOUT_MAX (PY_LLONG_MAX / 1000)
+#elif defined (NT_THREADS)
+ /* In the NT API, the timeout is a DWORD and is expressed in milliseconds */
+# if 0xFFFFFFFFLL * 1000 < PY_LLONG_MAX
+# define PY_TIMEOUT_MAX (0xFFFFFFFFLL * 1000)
+# else
+# define PY_TIMEOUT_MAX PY_LLONG_MAX
+# endif
+#else
+# define PY_TIMEOUT_MAX PY_LLONG_MAX
 #endif
 
+
 /* If microseconds == 0, the call is non-blocking: it returns immediately
 even when the lock can't be acquired.
 If microseconds > 0, the call waits up to the specified duration.
diff --git a/Misc/NEWS.d/next/Library/2017-10-24-12-00-16.bpo-30768.Om8Yj_.rst b/Misc/NEWS.d/next/Library/2017-10-24-12-00-16.bpo-30768.Om8Yj_.rst
new file mode 100644
index 00000000000..77bff97a7ee
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2017-10-24-12-00-16.bpo-30768.Om8Yj_.rst
@@ -0,0 +1,3 @@
+Fix the pthread+semaphore implementation of PyThread_acquire_lock_timed() when
+called with timeout > 0 and intr_flag=0: recompute the timeout if
+sem_timedwait() is interrupted by a signal (EINTR). See also the :pep:`475`.
diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c
index 72df78f9ccc..99611eebcb7 100644
--- a/Modules/_threadmodule.c
+++ b/Modules/_threadmodule.c
@@ -1363,9 +1363,11 @@ PyInit__thread(void)
 if (m == NULL)
 return NULL;
 
- timeout_max = PY_TIMEOUT_MAX / 1000000;
- time_max = floor(_PyTime_AsSecondsDouble(_PyTime_MAX));
+ timeout_max = (double)PY_TIMEOUT_MAX * 1e-6;
+ time_max = _PyTime_AsSecondsDouble(_PyTime_MAX);
 timeout_max = Py_MIN(timeout_max, time_max);
+ /* Round towards minus infinity */
+ timeout_max = floor(timeout_max);
 
 v = PyFloat_FromDouble(timeout_max);
 if (!v)
diff --git a/Modules/_winapi.c b/Modules/_winapi.c
index 00a26d515e0..7e8d4e38464 100644
--- a/Modules/_winapi.c
+++ b/Modules/_winapi.c
@@ -61,8 +61,6 @@
 
 #define T_HANDLE T_POINTER
 
-#define DWORD_MAX 4294967295U
-
 /* Grab CancelIoEx dynamically from kernel32 */
 static int has_CancelIoEx = -1;
 static BOOL (CALLBACK *Py_CancelIoEx)(HANDLE, LPOVERLAPPED);
@@ -184,11 +182,11 @@ class DWORD_return_converter(CReturnConverter):
 
 def render(self, function, data):
 self.declare(data)
- self.err_occurred_if("_return_value == DWORD_MAX", data)
+ self.err_occurred_if("_return_value == PY_DWORD_MAX", data)
 data.return_conversion.append(
 'return_value = Py_BuildValue("k", _return_value);\n')
 [python start generated code]*/
-/*[python end generated code: output=da39a3ee5e6b4b0d input=94819e72d2c6d558]*/
+/*[python end generated code: output=da39a3ee5e6b4b0d input=4527052fe06e5823]*/
 
 #include "clinic/_winapi.c.h"
 
@@ -1009,7 +1007,7 @@ _winapi_GetExitCodeProcess_impl(PyObject *module, HANDLE process)
 
 if (! result) {
 PyErr_SetFromWindowsErr(GetLastError());
- exit_code = DWORD_MAX;
+ exit_code = PY_DWORD_MAX;
 }
 
 return exit_code;
@@ -1466,7 +1464,7 @@ _winapi_WriteFile_impl(PyObject *module, HANDLE handle, PyObject *buffer,
 }
 
 Py_BEGIN_ALLOW_THREADS
- len = (DWORD)Py_MIN(buf->len, DWORD_MAX);
+ len = (DWORD)Py_MIN(buf->len, PY_DWORD_MAX);
 ret = WriteFile(handle, buf->buf, len, &written,
 overlapped ? &overlapped->overlapped : NULL);
 Py_END_ALLOW_THREADS
diff --git a/Modules/clinic/_winapi.c.h b/Modules/clinic/_winapi.c.h
index 9e1fbe1fdb0..01bba360714 100644
--- a/Modules/clinic/_winapi.c.h
+++ b/Modules/clinic/_winapi.c.h
@@ -460,7 +460,7 @@ _winapi_GetExitCodeProcess(PyObject *module, PyObject *arg)
 goto exit;
 }
 _return_value = _winapi_GetExitCodeProcess_impl(module, process);
- if ((_return_value == DWORD_MAX) && PyErr_Occurred()) {
+ if ((_return_value == PY_DWORD_MAX) && PyErr_Occurred()) {
 goto exit;
 }
 return_value = Py_BuildValue("k", _return_value);
@@ -487,7 +487,7 @@ _winapi_GetLastError(PyObject *module, PyObject *Py_UNUSED(ignored))
 DWORD _return_value;
 
 _return_value = _winapi_GetLastError_impl(module);
- if ((_return_value == DWORD_MAX) && PyErr_Occurred()) {
+ if ((_return_value == PY_DWORD_MAX) && PyErr_Occurred()) {
 goto exit;
 }
 return_value = Py_BuildValue("k", _return_value);
@@ -889,4 +889,4 @@ _winapi_WriteFile(PyObject *module, PyObject **args, Py_ssize_t nargs, PyObject
 exit:
 return return_value;
 }
-/*[clinic end generated code: output=afa6bd61eb0f18d2 input=a9049054013a1b77]*/
+/*[clinic end generated code: output=fba2ad7bf1a87e4a input=a9049054013a1b77]*/
diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
index c7d8b00e6f9..661fa1312ad 100644
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -390,8 +390,6 @@ static int win32_can_symlink = 0;
 #endif
 #endif
 
-#define DWORD_MAX 4294967295U
-
 #ifdef MS_WINDOWS
 #define INITFUNC PyInit_nt
 #define MODNAME "nt"
@@ -3817,7 +3815,7 @@ os__getvolumepathname_impl(PyObject *module, PyObject *path)
 /* Volume path should be shorter than entire path */
 buflen = Py_MAX(buflen, MAX_PATH);
 
- if (buflen > DWORD_MAX) {
+ if (buflen > PY_DWORD_MAX) {
 PyErr_SetString(PyExc_OverflowError, "path too long");
 return NULL;
 }
diff --git a/Python/thread_nt.h b/Python/thread_nt.h
index bae8bcc3566..46df3466928 100644
--- a/Python/thread_nt.h
+++ b/Python/thread_nt.h
@@ -283,12 +283,13 @@ PyThread_acquire_lock_timed(PyThread_type_lock aLock,
 milliseconds = microseconds / 1000;
 if (microseconds % 1000 > 0)
 ++milliseconds;
- if ((DWORD) milliseconds != milliseconds)
- Py_FatalError("Timeout too large for a DWORD, "
- "please check PY_TIMEOUT_MAX");
+ if (milliseconds > PY_DWORD_MAX) {
+ Py_FatalError("Timeout larger than PY_TIMEOUT_MAX");
+ }
 }
- else
+ else {
 milliseconds = INFINITE;
+ }
 
 dprintf(("%lu: PyThread_acquire_lock_timed(%p, %lld) called\n",
 PyThread_get_thread_ident(), aLock, microseconds));
diff --git a/Python/thread_pthread.h b/Python/thread_pthread.h
index c5b7f3256fa..13cffa3bf3e 100644
--- a/Python/thread_pthread.h
+++ b/Python/thread_pthread.h
@@ -318,23 +318,66 @@ 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;
+ _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 > 0)
+ if (microseconds > PY_TIMEOUT_MAX) {
+ Py_FatalError("Timeout larger than PY_TIMEOUT_MAX");
+ }
+
+ if (microseconds > 0) {
 MICROSECONDS_TO_TIMESPEC(microseconds, ts);
- do {
- if (microseconds > 0)
+
+ 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;
+ }
+ }
+
+ while (1) {
+ if (microseconds > 0) {
 status = fix_status(sem_timedwait(thelock, &ts));
- else if (microseconds == 0)
+ }
+ else if (microseconds == 0) {
 status = fix_status(sem_trywait(thelock));
- else
+ }
+ else {
 status = fix_status(sem_wait(thelock));
+ }
+
 /* Retry if interrupted by a signal, unless the caller wants to be
 notified. */
- } while (!intr_flag && status == EINTR);
+ if (intr_flag || status != EINTR) {
+ break;
+ }
+
+ if (microseconds > 0) {
+ /* wait interrupted by a signal (EINTR): recompute the timeout */
+ _PyTime_t dt = deadline - _PyTime_GetMonotonicClock();
+ if (dt < 0) {
+ status = ETIMEDOUT;
+ break;
+ }
+ else if (dt > 0) {
+ _PyTime_t realtime_deadline = _PyTime_GetSystemClock() + dt;
+ if (_PyTime_AsTimespec(realtime_deadline, &ts) < 0) {
+ /* Cannot occur thanks to (microseconds > PY_TIMEOUT_MAX)
+ check done above */
+ Py_UNREACHABLE();
+ }
+ /* 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)) {
