gh-104530: Enable native Win32 condition variables by default (GH-104531)

adr26 · web-flow · commit b3f0b698daf2 · 2024-02-02T13:50:51.000Z
diff --git a/Include/internal/pycore_condvar.h b/Include/internal/pycore_condvar.h
@@ -35,14 +35,14 @@
 #include <windows.h>              // CRITICAL_SECTION
 
 /* options */
-/* non-emulated condition variables are provided for those that want
- * to target Windows Vista.  Modify this macro to enable them.
+/* emulated condition variables are provided for those that want
+ * to target Windows XP or earlier. Modify this macro to enable them.
  */
 #ifndef _PY_EMULATED_WIN_CV
-#define _PY_EMULATED_WIN_CV 1  /* use emulated condition variables */
+#define _PY_EMULATED_WIN_CV 0  /* use non-emulated condition variables */
 #endif
 
-/* fall back to emulation if not targeting Vista */
+/* fall back to emulation if targeting earlier than Vista */
 #if !defined NTDDI_VISTA || NTDDI_VERSION < NTDDI_VISTA
 #undef _PY_EMULATED_WIN_CV
 #define _PY_EMULATED_WIN_CV 1
@@ -77,7 +77,7 @@ typedef struct _PyCOND_T
 
 #else /* !_PY_EMULATED_WIN_CV */
 
-/* Use native Win7 primitives if build target is Win7 or higher */
+/* Use native Windows primitives if build target is Vista or higher */
 
 /* SRWLOCK is faster and better than CriticalSection */
 typedef SRWLOCK PyMUTEX_T;
diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-05-16-06-52-34.gh-issue-104530.mJnA0W.rst b/Misc/NEWS.d/next/Core and Builtins/2023-05-16-06-52-34.gh-issue-104530.mJnA0W.rst
@@ -0,0 +1 @@
+Use native Win32 condition variables.
diff --git a/Python/ceval_gil.c b/Python/ceval_gil.c
@@ -610,8 +610,16 @@ PyEval_SaveThread(void)
 void
 PyEval_RestoreThread(PyThreadState *tstate)
 {
+#ifdef MS_WINDOWS
+    int err = GetLastError();
+#endif
+
     _Py_EnsureTstateNotNULL(tstate);
     _PyThreadState_Attach(tstate);
+
+#ifdef MS_WINDOWS
+    SetLastError(err);
+#endif
 }
 
 
diff --git a/Python/condvar.h b/Python/condvar.h
@@ -260,13 +260,13 @@ PyMUTEX_UNLOCK(PyMUTEX_T *cs)
     return 0;
 }
 
-
 Py_LOCAL_INLINE(int)
 PyCOND_INIT(PyCOND_T *cv)
 {
     InitializeConditionVariable(cv);
     return 0;
 }
+
 Py_LOCAL_INLINE(int)
 PyCOND_FINI(PyCOND_T *cv)
 {
@@ -279,27 +279,32 @@ PyCOND_WAIT(PyCOND_T *cv, PyMUTEX_T *cs)
     return SleepConditionVariableSRW(cv, cs, INFINITE, 0) ? 0 : -1;
 }
 
-/* This implementation makes no distinction about timeouts.  Signal
- * 2 to indicate that we don't know.
- */
+/* return 0 for success, 1 on timeout, -1 on error */
 Py_LOCAL_INLINE(int)
 PyCOND_TIMEDWAIT(PyCOND_T *cv, PyMUTEX_T *cs, long long us)
 {
-    return SleepConditionVariableSRW(cv, cs, (DWORD)(us/1000), 0) ? 2 : -1;
+    BOOL success = SleepConditionVariableSRW(cv, cs, (DWORD)(us/1000), 0);
+    if (!success) {
+        if (GetLastError() == ERROR_TIMEOUT) {
+            return 1;
+        }
+        return -1;
+    }
+    return 0;
 }
 
 Py_LOCAL_INLINE(int)
 PyCOND_SIGNAL(PyCOND_T *cv)
 {
-     WakeConditionVariable(cv);
-     return 0;
+    WakeConditionVariable(cv);
+    return 0;
 }
 
 Py_LOCAL_INLINE(int)
 PyCOND_BROADCAST(PyCOND_T *cv)
 {
-     WakeAllConditionVariable(cv);
-     return 0;
+    WakeAllConditionVariable(cv);
+    return 0;
 }
 
 
diff --git a/Python/pystate.c b/Python/pystate.c
@@ -2488,7 +2488,17 @@ PyGILState_Check(void)
         return 0;
     }
 
-    return (tstate == gilstate_tss_get(runtime));
+#ifdef MS_WINDOWS
+    int err = GetLastError();
+#endif
+
+    PyThreadState *tcur = gilstate_tss_get(runtime);
+
+#ifdef MS_WINDOWS
+    SetLastError(err);
+#endif
+
+    return (tstate == tcur);
 }
 
 PyGILState_STATE
diff --git a/Python/thread_nt.h b/Python/thread_nt.h
@@ -444,16 +444,7 @@ PyThread_set_key_value(int key, void *value)
 void *
 PyThread_get_key_value(int key)
 {
-    /* because TLS is used in the Py_END_ALLOW_THREAD macro,
-     * it is necessary to preserve the windows error state, because
-     * it is assumed to be preserved across the call to the macro.
-     * Ideally, the macro should be fixed, but it is simpler to
-     * do it here.
-     */
-    DWORD error = GetLastError();
-    void *result = TlsGetValue(key);
-    SetLastError(error);
-    return result;
+    return TlsGetValue(key);
 }
 
 void
@@ -525,14 +516,5 @@ void *
 PyThread_tss_get(Py_tss_t *key)
 {
     assert(key != NULL);
-    /* because TSS is used in the Py_END_ALLOW_THREAD macro,
-     * it is necessary to preserve the windows error state, because
-     * it is assumed to be preserved across the call to the macro.
-     * Ideally, the macro should be fixed, but it is simpler to
-     * do it here.
-     */
-    DWORD error = GetLastError();
-    void *result = TlsGetValue(key->_key);
-    SetLastError(error);
-    return result;
+    return TlsGetValue(key->_key);
 }

Original file line number	Diff line number	Diff line change
`@@ -260,13 +260,13 @@ PyMUTEX_UNLOCK(PyMUTEX_T *cs)`
`260`	`260`	`return 0;`
`261`	`261`	`}`
`262`	`262`
`263`		`-`
`264`	`263`	`Py_LOCAL_INLINE(int)`
`265`	`264`	`PyCOND_INIT(PyCOND_T *cv)`
`266`	`265`	`{`
`267`	`266`	`InitializeConditionVariable(cv);`
`268`	`267`	`return 0;`
`269`	`268`	`}`
	`269`	`+`
`270`	`270`	`Py_LOCAL_INLINE(int)`
`271`	`271`	`PyCOND_FINI(PyCOND_T *cv)`
`272`	`272`	`{`
`@@ -279,27 +279,32 @@ PyCOND_WAIT(PyCOND_T cv, PyMUTEX_T cs)`
`279`	`279`	`return SleepConditionVariableSRW(cv, cs, INFINITE, 0) ? 0 : -1;`
`280`	`280`	`}`
`281`	`281`
`282`		`-/* This implementation makes no distinction about timeouts. Signal`
`283`		`- * 2 to indicate that we don't know.`
`284`		`- */`
	`282`	`+/* return 0 for success, 1 on timeout, -1 on error */`
`285`	`283`	`Py_LOCAL_INLINE(int)`
`286`	`284`	`PyCOND_TIMEDWAIT(PyCOND_T cv, PyMUTEX_T cs, long long us)`
`287`	`285`	`{`
`288`		`- return SleepConditionVariableSRW(cv, cs, (DWORD)(us/1000), 0) ? 2 : -1;`
	`286`	`+ BOOL success = SleepConditionVariableSRW(cv, cs, (DWORD)(us/1000), 0);`
	`287`	`+ if (!success) {`
	`288`	`+ if (GetLastError() == ERROR_TIMEOUT) {`
	`289`	`+ return 1;`
	`290`	`+ }`
	`291`	`+ return -1;`
	`292`	`+ }`
	`293`	`+ return 0;`
`289`	`294`	`}`
`290`	`295`
`291`	`296`	`Py_LOCAL_INLINE(int)`
`292`	`297`	`PyCOND_SIGNAL(PyCOND_T *cv)`
`293`	`298`	`{`
`294`		`- WakeConditionVariable(cv);`
`295`		`- return 0;`
	`299`	`+ WakeConditionVariable(cv);`
	`300`	`+ return 0;`
`296`	`301`	`}`
`297`	`302`
`298`	`303`	`Py_LOCAL_INLINE(int)`
`299`	`304`	`PyCOND_BROADCAST(PyCOND_T *cv)`
`300`	`305`	`{`
`301`		`- WakeAllConditionVariable(cv);`
`302`		`- return 0;`
	`306`	`+ WakeAllConditionVariable(cv);`
	`307`	`+ return 0;`
`303`	`308`	`}`
`304`	`309`
`305`	`310`