Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

asyncio.add_signal_handler call fails if not on main thread #78860

Closed
jnwatson mannequin opened this issue Sep 14, 2018 · 16 comments
Closed

asyncio.add_signal_handler call fails if not on main thread #78860

jnwatson mannequin opened this issue Sep 14, 2018 · 16 comments
Assignees
Labels
3.8 only security fixes 3.9 only security fixes release-blocker topic-asyncio type-bug An unexpected behavior, bug, or error

Comments

@jnwatson
Copy link
Mannequin

jnwatson mannequin commented Sep 14, 2018

BPO 34679
Nosy @vstinner, @ned-deily, @asvetlov, @ambv, @1st1, @jnwatson, @miss-islington, @tirkarthi, @epiphyte, @aeros, @bslatkin
PRs
  • bpo-34679: Add conditional to BaseProactorEventLoop init #15477
  • bpo-34679: Restore instantiation Windows IOCP event loop from non-main thread #15492
  • [3.8] bpo-34679: Restore instantiation Windows IOCP event loop from non-main thread (GH-15492) #15512
  • bpo-34679: ProactorEventLoop only uses set_wakeup_fd() in main thread #16901
  • [3.8] bpo-34679: ProactorEventLoop only uses set_wakeup_fd() in main thread (GH-16901) #16902
  • 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 = 'https://github.com/asvetlov'
    closed_at = <Date 2019-08-26.09:53:15.031>
    created_at = <Date 2018-09-14.15:27:45.465>
    labels = ['type-bug', '3.8', '3.9', 'release-blocker', 'expert-asyncio']
    title = 'asyncio.add_signal_handler call fails if not on main thread'
    updated_at = <Date 2019-10-23.15:44:03.600>
    user = 'https://github.com/jnwatson'

    bugs.python.org fields:

    activity = <Date 2019-10-23.15:44:03.600>
    actor = 'miss-islington'
    assignee = 'asvetlov'
    closed = True
    closed_date = <Date 2019-08-26.09:53:15.031>
    closer = 'asvetlov'
    components = ['asyncio']
    creation = <Date 2018-09-14.15:27:45.465>
    creator = 'jnwatson'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 34679
    keywords = ['patch', 'needs review']
    message_count = 16.0
    messages = ['325350', '326426', '347525', '347546', '350288', '350298', '350304', '350399', '350401', '350463', '350464', '350472', '350515', '350518', '355230', '355233']
    nosy_count = 11.0
    nosy_names = ['vstinner', 'ned.deily', 'asvetlov', 'lukasz.langa', 'yselivanov', 'jnwatson', 'miss-islington', 'xtreak', 'epiphyte', 'aeros', 'haxor']
    pr_nums = ['15477', '15492', '15512', '16901', '16902']
    priority = 'release blocker'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue34679'
    versions = ['Python 3.8', 'Python 3.9']

    @jnwatson
    Copy link
    Mannequin Author

    jnwatson mannequin commented Sep 14, 2018

    Summary: essentially asyncio.add_signal_handler doesn't work when called off the main thread. One might consider this a documentation failure.

    (Note: there's also a bigger issue with cleanup, which I'll submit separately)

    Exception in thread Thread-1:
    Traceback (most recent call last):
      File "/home/nic/.pyenv/versions/3.6.4/lib/python3.6/asyncio/unix_events.py", line 91, in add_signal_handler
        signal.set_wakeup_fd(self._csock.fileno())
    ValueError: set_wakeup_fd only works in main thread
    
    During handling of the above exception, another exception occurred:
    
    Traceback (most recent call last):
      File "/home/nic/.pyenv/versions/3.6.4/lib/python3.6/threading.py", line 916, in _bootstrap_inner
        self.run()
      File "/home/nic/.pyenv/versions/3.6.4/lib/python3.6/threading.py", line 864, in run
        self._target(*self._args, **self._kwargs)
      File "/home/nic/tmp/signal_event_loop_bug.py", line 14, in do_loop
        loop.add_signal_handler(signal.SIGINT, mysighandler)
      File "/home/nic/.pyenv/versions/3.6.4/lib/python3.6/asyncio/unix_events.py", line 93, in add_signal_handler
        raise RuntimeError(str(exc))
    RuntimeError: set_wakeup_fd only works in main thread

    Code:

    import asyncio
    import signal
    import threading
    
    def mysighandler():
        pass
    
    def do_loop():
        loop = asyncio.new_event_loop()
        # This will fail with RuntimeError: set_wakeup_fd only works in main thread
        loop.add_signal_handler(signal.SIGINT, mysighandler)
        loop.close()
    
    t = threading.Thread(target=do_loop)
    t.start()
    t.join()

    @jnwatson jnwatson mannequin added 3.7 (EOL) end of life topic-asyncio type-bug An unexpected behavior, bug, or error labels Sep 14, 2018
    @asvetlov
    Copy link
    Contributor

    In my opinion, we should don't change the behavior.
    Signal handlers should be processed in main thread at the end, that's how Python works for regular synchronous API.

    Mentioning it in the doc makes sense.

    Would you make a pull request?

    @asvetlov asvetlov added the 3.8 only security fixes label Sep 26, 2018
    @bslatkin
    Copy link
    Mannequin

    bslatkin mannequin commented Jul 9, 2019

    I believe the scope of this bug may be larger than it originally seemed.

    Now that ProactorEventLoop is the default for Python 3.8 (https://bugs.python.org/issue34687), I'm seeing this same problem on Windows when you try to call asyncio.new_event_loop() from within a thread. It breaks with the ProactorEventLoop (snippet #1 below). It works fine with the SelectorEventLoop (snippet #2 below).

    Am I wrong to expect to be able to create a unique event loop for each thread from within the thread itself?

    I worked around this problem by manually forcing the event loop policy (https://bugs.python.org/issue33792).

    === Snippet #1 (breaks) ===

    import asyncio
    from threading import Thread
    
    def my_func():
        asyncio.new_event_loop()
    
    t = Thread(target=my_func)
    t.start()
    t.join()

    === Output from snippet #1 ===

    PS G:\> python .\repro.py
    Exception in thread Thread-1:
    Traceback (most recent call last):
      File "C:\Users\User\AppData\Local\Programs\Python\Python38\lib\threading.py", line 932, in _bootstrap_inner
        self.run()
      File "C:\Users\User\AppData\Local\Programs\Python\Python38\lib\threading.py", line 870, in run
        self._target(*self._args, **self._kwargs)
      File ".\repro.py", line 6, in my_func
        asyncio.new_event_loop()
      File "C:\Users\User\AppData\Local\Programs\Python\Python38\lib\asyncio\events.py", line 758, in new_event_loop
        return get_event_loop_policy().new_event_loop()
      File "C:\Users\User\AppData\Local\Programs\Python\Python38\lib\asyncio\events.py", line 656, in new_event_loop
        return self._loop_factory()
      File "C:\Users\User\AppData\Local\Programs\Python\Python38\lib\asyncio\windows_events.py", line 310, in __init__
        super().__init__(proactor)
      File "C:\Users\User\AppData\Local\Programs\Python\Python38\lib\asyncio\proactor_events.py", line 630, in __init__
        signal.set_wakeup_fd(self_no)
    ValueError: set_wakeup_fd only works in main thread

    === Snippet #2 (works) ===

    import asyncio
    from threading import Thread
    
    # Work-around from https://bugs.python.org/issue34679
    policy = asyncio.get_event_loop_policy()
    policy._loop_factory = asyncio.SelectorEventLoop
    
    def my_func():
        asyncio.new_event_loop()
    
    t = Thread(target=my_func)
    t.start()
    t.join()

    === More details ===

    My version of Python:

    3.8.0b2 (tags/v3.8.0b2:21dd01d, Jul 4 2019, 16:00:09) [MSC v.1916 64 bit (AMD64)]

    My version of Windows (it's a developer VM from https://developer.microsoft.com/en-us/windows/downloads/virtual-machines):

    PS G:\> [System.Environment]::OSVersion.Version

    Major Minor Build Revision
    ----- ----- ----- --------
    10 0 17763 0

    @asvetlov
    Copy link
    Contributor

    asvetlov commented Jul 9, 2019

    Good point.
    For proactor event loop set_wakeup_fd() is used for Ctrl+C handling.
    Skipping this call for non-main thread in proactor implementation makes sense.

    @asvetlov asvetlov self-assigned this Jul 9, 2019
    @ambv
    Copy link
    Contributor

    ambv commented Aug 23, 2019

    Please fix this ASAP, last 3.8 beta is scheduled for Monday.

    @1st1
    Copy link
    Member

    1st1 commented Aug 23, 2019

    Andrew, can you fix ctrl-c/windows issue? I'm basically afk until monday.

    And we're not going to do anything about add_signal_handler in 3.8.

    @bslatkin
    Copy link
    Mannequin

    bslatkin mannequin commented Aug 23, 2019

    Maybe we should just roll back https://bugs.python.org/issue34687 to fix both issues? Otherwise asyncio will be broken on Windows in 3.8. Is general API stability more important than performance?

    @aeros
    Copy link
    Contributor

    aeros commented Aug 24, 2019

    Skipping this call for non-main thread in proactor implementation makes sense.

    How do we identify whether or not set_wakeup_fd() is being called from a non-main thread?

    @aeros
    Copy link
    Contributor

    aeros commented Aug 24, 2019

    How do we identify whether or not set_wakeup_fd() is being called from a non-main thread?

    Never mind, I think I found the answer to my own question and tested a patch locally, I'll open a PR.

    @asvetlov
    Copy link
    Contributor

    The issue is related to Python 3.8 and master only. 3.6-3.7 are not affected

    @asvetlov asvetlov added 3.9 only security fixes and removed 3.7 (EOL) end of life labels Aug 25, 2019
    @asvetlov
    Copy link
    Contributor

    Kyle, thanks for the fix.
    I have basically the same change in my PR but with test and news note.

    @aeros
    Copy link
    Contributor

    aeros commented Aug 25, 2019

    Kyle, thanks for the fix.
    I have basically the same change in my PR but with test and news note.

    No problem, that works for me. I was mostly just trying to help with resolving some of the release blockers for 3.8b4.

    @asvetlov
    Copy link
    Contributor

    New changeset 1c06009 by Andrew Svetlov in branch 'master':
    bpo-34679: Restore instantiation Windows IOCP event loop from non-main thread (bpo-15492)
    1c06009

    @miss-islington
    Copy link
    Contributor

    New changeset 69d22b8 by Miss Islington (bot) in branch '3.8':
    bpo-34679: Restore instantiation Windows IOCP event loop from non-main thread (GH-15492)
    69d22b8

    @vstinner
    Copy link
    Member

    New changeset 1b53a24 by Victor Stinner in branch 'master':
    bpo-34679: ProactorEventLoop only uses set_wakeup_fd() in main thread (GH-16901)
    1b53a24

    @miss-islington
    Copy link
    Contributor

    New changeset cbf474c by Miss Skeleton (bot) in branch '3.8':
    bpo-34679: ProactorEventLoop only uses set_wakeup_fd() in main thread (GH-16901)
    cbf474c

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    cderici added a commit to cderici/python-libjuju that referenced this issue Sep 5, 2023
    This test (test_explicit_loop_threaded) is added 7 years ago in juju#63
    juju#63 in the good old days of
    custom loops in libjuju. Nowadays everything's handled by the asyncio,
    so I don't think anyone's using this library with a ThreadPoolExecutor
    anymore.
    
    The signal handlers are breaking because of a known issue from Python
    3.8 python/cpython#78860.
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes 3.9 only security fixes release-blocker topic-asyncio type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants