Fatal error if timeout > PY_TIMEOUT_MAX

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.

Author: vstinner

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.

Include/pythread.h

@@ -42,21 +42,22 @@ PyAPI_FUNC(int) PyThread_acquire_lock(PyThread_type_lock, int);
    and floating-point numbers allowed.
 */
 #define PY_TIMEOUT_T long long
+
 #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
 
-/* 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
-#endif
 
 /* If microseconds == 0, the call is non-blocking: it returns immediately
    even when the lock can't be acquired.

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

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

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

Python/thread_pthread.h

@@ -324,13 +324,16 @@ PyThread_acquire_lock_timed(PyThread_type_lock lock, PY_TIMEOUT_T microseconds,
     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");
+    }
+
     if (microseconds > 0) {
         MICROSECONDS_TO_TIMESPEC(microseconds, ts);
 
         if (!intr_flag) {
-            /* the caller must ensures that microseconds <= PY_TIMEOUT_MAX
-               and so microseconds * 1000 cannot overflow. PY_TIMEOUT_MAX
-               is defined to prevent this specific overflow. */
+            /* cannot overflow thanks to (microseconds > PY_TIMEOUT_MAX)
+               check done above */
             _PyTime_t timeout = _PyTime_FromNanoseconds(microseconds * 1000);
             deadline = _PyTime_GetMonotonicClock() + timeout;
         }
@@ -363,8 +366,9 @@ PyThread_acquire_lock_timed(PyThread_type_lock lock, PY_TIMEOUT_T microseconds,
             else if (dt > 0) {
                 _PyTime_t realtime_deadline = _PyTime_GetSystemClock() + dt;
                 if (_PyTime_AsTimespec(realtime_deadline, &ts) < 0) {
-                    success = PY_LOCK_FAILURE;
-                    goto exit;
+                    /* 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). */