Skip to content

pycore_condvar.h: remove Windows conditonal variable emulation, use Windows native conditional variable #89464

Closed
Closed
pycore_condvar.h: remove Windows conditonal variable emulation, use Windows native conditional variable#89464
Labels
3.11only security fixesstdlibStandard Library Python modules in the Lib/ directory

Description

@vstinner
vstinner
opened on Sep 27, 2021
BPO 45301
Nosy @pfmoore, @vstinner, @tjguk, @zware, @eryksun, @zooba

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2021-09-27.14:26:03.506>
labels = ['library', '3.11']
title = 'pycore_condvar.h: remove Windows conditonal variable emulation, use Windows native conditional variable'
updated_at = <Date 2021-09-28.06:30:07.878>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2021-09-28.06:30:07.878>
actor = 'eryksun'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2021-09-27.14:26:03.506>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 45301
keywords = []
message_count = 6.0
messages = ['402720', '402721', '402725', '402732', '402741', '402757']
nosy_count = 6.0
nosy_names = ['paul.moore', 'vstinner', 'tim.golden', 'zach.ware', 'eryksun', 'steve.dower']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue45301'
versions = ['Python 3.11']

Activity

  1. vstinner commented on Sep 27, 2021

    @vstinner
    vstinner
    on Sep 27, 2021
    MemberAuthor

    I recently worked on time.sleep() enhancement (bpo-21302) and threading bugfixes (bpo-45274, bpo-1596321). I saw one more time that Python emulates conditional variables to support Windows XP and older. But Python 3.11 requires Windows 8.1 or newer. IMO it's time to remove _PY_EMULATED_WIN_CV code path from pycore_condvar.h.

  2. vstinner
    added
    3.11only security fixes
    stdlibStandard Library Python modules in the Lib/ directory
    on Sep 27, 2021

  3. vstinner commented on Sep 27, 2021

    @vstinner
    vstinner
    on Sep 27, 2021
    MemberAuthor

    See also bpo-29971 "Lock.acquire() not interruptible on Windows".

  4. eryksun commented on Sep 27, 2021

    @eryksun
    eryksun
    on Sep 27, 2021
    Contributor

    IMO it's time to remove _PY_EMULATED_WIN_CV code path from
    pycore_condvar.h.

    SleepConditionVariableSRW() can't be interrupted in PyCOND_WAIT() and PyCOND_TIMEDWAIT(). Maybe a hybrid solution could be adopted. Use native condition variables for the GIL, where performance matters and the ability to interrupt the wait doesn't matter. Otherwise use the semaphore implementation, for which the wait implementation can be modified to use WaitForMultipleObjects(), and include the Ctrl+C event.

  5. zooba commented on Sep 27, 2021

    @zooba
    zooba
    on Sep 27, 2021
    Member

    SleepConditionVariableSRW() can't be interrupted in PyCOND_WAIT() and PyCOND_TIMEDWAIT()

    This was my immediate reaction as well.

    Unfortunately, we keep seeing that all waits need to be interruptible, so either a WaitForMultipleObjects or a slow spinlock.

  6. vstinner commented on Sep 27, 2021

    @vstinner
    vstinner
    on Sep 27, 2021
    MemberAuthor

    Eryk:

    SleepConditionVariableSRW() can't be interrupted in PyCOND_WAIT() and PyCOND_TIMEDWAIT().

    Oh. A comment on StackOverlow says:

    "The WaitForXxx functions accept parameters of the generic HANDLE type, which represents a handle to a kernel object. Condition variables are user-mode objects, not kernel objects, so you cannot use them with these functions, since they work only with kernel objects."

    https://stackoverflow.com/questions/37522108/can-you-really-wait-on-condition-variable-with-waitfor-objects

    I only created this issue because of this comment in pycore_condvar.h:
    ---

    /* non-emulated condition variables are provided for those that want
 * to target Windows Vista.  Modify this macro to enable them.
 */
#ifndef _PY_EMULATED_WIN_CV
#define _PY_EMULATED_WIN_CV 1  /* use emulated condition variables */
#endif
  7. vstinner
    changed the title [-]pycore_condvar.h: remove Windows conditonal variable emulation[/-] [+]pycore_condvar.h: remove Windows conditonal variable emulation, use Windows native conditional variable[/+] on Sep 27, 2021
  8. vstinner
    changed the title [-]pycore_condvar.h: remove Windows conditonal variable emulation[/-] [+]pycore_condvar.h: remove Windows conditonal variable emulation, use Windows native conditional variable[/+] on Sep 27, 2021

  9. eryksun commented on Sep 28, 2021

    @eryksun
    eryksun
    on Sep 28, 2021
    Contributor

    FYI, waiting for a condition variable can cause a thread to enter a wait state that's interruptible, in theory, but the mechanism is different since condition variables and SRW locks are pointer-sized values in user space, instead of NT objects in kernel space. The current implementation is based on the system call NtWaitForAlertByThreadId(address, timeout), which enters the "WrAlertByThreadId" wait state. The address parameter is that of the SRW lock. The kernel sets this as the 'object' for the wait, but the wait is actually satisfied by alerting the thread directly via NtAlertThreadByThreadId(tid). ISTM, they could have added a function that calls the latter to cancel a wait on a given thread. That would have been useful for Ctrl+C since the handler executes on a new thread.

  10. ezio-melotti
    transferred this issue fromon Apr 10, 2022

  11. vstinner commented on Nov 3, 2022

    @vstinner
    vstinner
    on Nov 3, 2022
    MemberAuthor

    Sadly, I don't have the bandwidth to work on this issue, I just close it.

    First, I understood that it was just deadcode. Nope. It's way more complicated than what I expected!

  12. vstinner
    closed this as completedon Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    3.11only security fixesstdlibStandard Library Python modules in the Lib/ directory

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions