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

Author: adr26

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;

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.

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
 }
 
 

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

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

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