[New-bugs-announce] [issue35662] Windows #define _PY_EMULATED_WIN_CV 0 bug

[Jeff Robbins]

New submission from Jeff Robbins <jeffr at livedata.com>:

Python 3.x defaults to using emulated condition variables on Windows.  I tested a build with native Windows condition variables (#define _PY_EMULATED_WIN_CV 0), and found a serious issue.

The problem is in condvar.h, in this routine:


/* This implementation makes no distinction about timeouts.  Signal
 * 2 to indicate that we don't know.
 */
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;
}


The issue is that `SleepConditionVariableSRW` returns FALSE in the timeout case.  PyCOND_TIMEDWAIT returns -1 in that case. 

But... COND_TIMED_WAIT, which calls PyCOND_TIMEDWAIT, in ceval_gil.h, fatals(!) on a negative return value


#define COND_TIMED_WAIT(cond, mut, microseconds, timeout_result) \
    { \
        int r = PyCOND_TIMEDWAIT(&(cond), &(mut), (microseconds)); \
        if (r < 0) \
            Py_FatalError("PyCOND_WAIT(" #cond ") failed"); \



I'd like to suggest that we use the documented behavior of the OS API call already being used (SleepConditionVariableSRW https://docs.microsoft.com/en-us/windows/desktop/api/synchapi/nf-synchapi-sleepconditionvariablesrw) and return 0 on regular success and 1 on timeout, like in the _POSIX_THREADS case.  

"""
Return Value
If the function succeeds, the return value is nonzero.

If the function fails, the return value is zero. To get extended error information, call GetLastError.

If the timeout expires the function returns FALSE and GetLastError returns ERROR_TIMEOUT.
"""

I've tested this rewrite -- the main difference is in the FALSE case, check GetLastError() for ERROR_TIMEOUT and then *do not* treat this as a fatal error.
 


/*
 * PyCOND_TIMEDWAIT, in addition to returning negative on error,
 * thus returns 0 on regular success, 1 on timeout
*/
Py_LOCAL_INLINE(int)
PyCOND_TIMEDWAIT(PyCOND_T *cv, PyMUTEX_T *cs, long long us)
{
    BOOL result = SleepConditionVariableSRW(cv, cs, (DWORD)(us / 1000), 0);

    if (result)
        return 0;

    if (GetLastError() == ERROR_TIMEOUT)
        return 1;

    return -1;
}


I've attached the test I ran to reproduce the crash.

----------
components: Windows
files: thread_test2.py
messages: 333036
nosy: jeffr at livedata.com, paul.moore, steve.dower, tim.golden, zach.ware
priority: normal
severity: normal
status: open
title: Windows #define _PY_EMULATED_WIN_CV 0 bug
type: crash
versions: Python 3.7
Added file: https://bugs.python.org/file48024/thread_test2.py

_______________________________________
Python tracker <report at bugs.python.org>
<https://bugs.python.org/issue35662>
_______________________________________