New submission from Jeff Robbins <je...@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: je...@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 <rep...@bugs.python.org>
<https://bugs.python.org/issue35662>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to