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

bpo-33792: Add selector and proactor windows policies #7487

Merged
merged 1 commit into from
Jun 8, 2018

Conversation

1st1
Copy link
Member

@1st1 1st1 commented Jun 7, 2018

@1st1
Copy link
Member Author

1st1 commented Jun 7, 2018

There's no adequate documentation on asyncio policies that lists all of them. I'll try to work on that in a separate PR.

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

The new classes should be documented around https://docs.python.org/dev/library/asyncio-eventloops.html#event-loop-policies-and-the-default-policy

But it can be done later, right now event DefaultEventLoopPolicy is not properly documented... It's just mentionned one.

Please backport the change to 3.7 :-)

@vstinner
Copy link
Member

vstinner commented Jun 7, 2018

Oh wow, AppVeyor failed with a weird error!?

FAIL: test_env_var_debug (test.test_asyncio.test_base_events.BaseEventLoopTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\projects\cpython\lib\test\test_asyncio\test_base_events.py", line 771, in test_env_var_debug
    sts, stdout, stderr = assert_python_ok('-E', '-c', code)
  File "C:\projects\cpython\lib\test\support\script_helper.py", line 157, in assert_python_ok
    return _assert_python(True, *args, **env_vars)
  File "C:\projects\cpython\lib\test\support\script_helper.py", line 143, in _assert_python
    res.fail(cmd_line)
  File "C:\projects\cpython\lib\test\support\script_helper.py", line 84, in fail
    err))
AssertionError: Process return code is 3221225477
command line: ['C:\\projects\\cpython\\PCbuild\\win32\\python.exe', '-X', 'faulthandler', '-I', '-E', '-c', 'import asyncio\nloop = asyncio.get_event_loop()\nprint(loop.get_debug())']
stdout:
---
---
stderr:
---
Windows fatal exception: access violation
Current thread 0x00000184 (most recent call first):
  File "C:\projects\cpython\lib\asyncio\events.py", line 41 in __init__
  File "C:\projects\cpython\lib\asyncio\selector_events.py", line 260 in _add_reader
  File "C:\projects\cpython\lib\asyncio\selector_events.py", line 117 in _make_self_pipe
  File "C:\projects\cpython\lib\asyncio\selector_events.py", line 66 in __init__
  File "C:\projects\cpython\lib\asyncio\events.py", line 660 in new_event_loop
  File "C:\projects\cpython\lib\asyncio\events.py", line 640 in get_event_loop
  File "<string>", line 2 in <module>
---

@vstinner
Copy link
Member

vstinner commented Jun 7, 2018

There's no adequate documentation on asyncio policies that lists all of them. I'll try to work on that in a separate PR.

Ok, works for me.

@1st1 1st1 closed this Jun 7, 2018
@1st1 1st1 reopened this Jun 7, 2018
@1st1
Copy link
Member Author

1st1 commented Jun 7, 2018

Oh wow, AppVeyor failed with a weird error!?

Not sure if it's related; restarted the CI.

@1st1
Copy link
Member Author

1st1 commented Jun 7, 2018

@ned-deily @vstinner The CI crash on Windows buildbots is real and I have no idea what's going on. It seems to be completely unrelated to asyncio.

Here's a screenshot from my Win VM:
screen shot 2018-06-07 at 6 28 08 pm

If we comment out

class WindowsProactorEventLoopPolicy(events.BaseDefaultEventLoopPolicy):
    _loop_factory = ProactorEventLoop

then everything works fine. The problem is that test_env_var_debug doesn't even use or invoke WindowsProactorEventLoopPolicy in any way.

@ned-deily
Copy link
Member

Would it help to pull in some of the Windows experts? (I’ll be off line for the next few hours.)

@1st1
Copy link
Member Author

1st1 commented Jun 7, 2018

Victor was also able to reproduce the bug. But the CI is green now...

The crash is specific to 32bit builds and seems to be originating from

class Handle:
    """Object returned by callback registration methods."""

    __slots__ = ('_callback', '_args', '_cancelled', '_loop',
                 '_source_traceback', '_repr', '__weakref__',
                 '_context')

    def __init__(self, callback, args, loop, context=None):
        if context is None:
            context = contextvars.copy_current()   # << this line

If we add a simple dict() before "# << this line" the crash disappears :)

contextvars.copy_current() can be replaced with contextvars.Context(), which means that the bug isn't related to the interpreter state or something.

@vstinner
Copy link
Member

vstinner commented Jun 8, 2018

The crash is https://bugs.python.org/issue33803 and it's unrelated to this change. It's just that the change made the bug more likely.

@1st1 1st1 merged commit 8f40429 into python:master Jun 8, 2018
@miss-islington
Copy link
Contributor

Thanks @1st1 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

@1st1 1st1 deleted the policy branch June 8, 2018 00:45
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 8, 2018
(cherry picked from commit 8f40429)

Co-authored-by: Yury Selivanov <yury@magic.io>
@bedevere-bot
Copy link

GH-7508 is a backport of this pull request to the 3.7 branch.

miss-islington added a commit that referenced this pull request Jun 8, 2018
(cherry picked from commit 8f40429)

Co-authored-by: Yury Selivanov <yury@magic.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants