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.
This commit is contained in:
Victor Stinner 2017-10-24 16:53:32 -07:00 committed by GitHub
parent 3557b05c5a
commit 850a18e03e
9 changed files with 86 additions and 31 deletions

View File

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

View File

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

View File

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

View File

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

View File

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

View File

@ -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]*/

View File

@ -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;
}

View File

@ -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));

View File

@ -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)) {