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

Asynchronous comprehensions don't work in asyncio REPL #83743

Closed
jack1142 mannequin opened this issue Feb 5, 2020 · 28 comments
Closed

Asynchronous comprehensions don't work in asyncio REPL #83743

jack1142 mannequin opened this issue Feb 5, 2020 · 28 comments
Labels
3.8 only security fixes 3.9 only security fixes release-blocker topic-asyncio type-bug An unexpected behavior, bug, or error

Comments

@jack1142
Copy link
Mannequin

jack1142 mannequin commented Feb 5, 2020

BPO 39562
Nosy @asvetlov, @ambv, @serhiy-storchaka, @Carreau, @asottile, @hroncok, @pablogsal, @miss-islington, @isidentical, @jack1142
PRs
  • bpo-39562: Allow executing asynchronous comprehensions in the asyncio REPL #18968
  • bpo-39965: Correctly raise SyntaxError if await is used outside async functions when PyCF_ALLOW_TOP_LEVEL_AWAIT is set #19010
  • [3.8] bpo-39562: Allow executing asynchronous comprehensions in the asyncio REPL (GH-18968) #19070
  • bpo-39562: Prevent collision of future and compiler flags #19230
  • [3.8] bpo-39562: Prevent collision of future and compiler flags (GH-19230) #19835
  • [3.8] bpo-39562: Correctly updated the version section in the what's new document #19838
  • 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 = <Date 2020-07-06.20:18:05.903>
    created_at = <Date 2020-02-05.17:28:25.689>
    labels = ['type-bug', '3.8', '3.9', 'release-blocker', 'expert-asyncio']
    title = "Asynchronous comprehensions don't work in asyncio REPL"
    updated_at = <Date 2021-09-07.20:14:54.080>
    user = 'https://github.com/jack1142'

    bugs.python.org fields:

    activity = <Date 2021-09-07.20:14:54.080>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2020-07-06.20:18:05.903>
    closer = 'pablogsal'
    components = ['asyncio']
    creation = <Date 2020-02-05.17:28:25.689>
    creator = 'jack1142'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 39562
    keywords = ['patch']
    message_count = 28.0
    messages = ['361443', '361445', '364600', '364601', '365311', '365312', '365313', '365347', '365881', '366838', '366841', '366843', '366849', '366851', '367015', '367018', '367734', '367847', '367855', '367890', '367892', '367897', '368334', '368835', '373131', '373159', '373168', '400234']
    nosy_count = 10.0
    nosy_names = ['asvetlov', 'lukasz.langa', 'serhiy.storchaka', 'mbussonn', 'Anthony Sottile', 'hroncok', 'pablogsal', 'miss-islington', 'BTaskaya', 'jack1142']
    pr_nums = ['18968', '19010', '19070', '19230', '19835', '19838']
    priority = 'release blocker'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue39562'
    versions = ['Python 3.8', 'Python 3.9']

    @jack1142
    Copy link
    Mannequin Author

    jack1142 mannequin commented Feb 5, 2020

    asyncio REPL doesn't allow using asynchronous comprehensions outside of async func. Same behavior can also be observed when using ast.PyCF_ALLOW_TOP_LEVEL_AWAIT flag in compile()

    Example with `async for`:
    >>> async def async_gen():
    ...     for x in range(5):
    ...         yield await asyncio.sleep(1, x)
    ... 
    >>> [x async for x in async_gen()]
      File "<console>", line 0
    SyntaxError: asynchronous comprehension outside of an asynchronous function
    
    
    Example with `await`:
    >>> [await asyncio.sleep(1, x) for x in range(5)]
      File "<console>", line 0
    SyntaxError: asynchronous comprehension outside of an asynchronous function

    @jack1142 jack1142 mannequin added 3.8 only security fixes 3.9 only security fixes topic-asyncio type-bug An unexpected behavior, bug, or error labels Feb 5, 2020
    @jack1142
    Copy link
    Mannequin Author

    jack1142 mannequin commented Feb 5, 2020

    I also noticed that putting await before the async comprehension will make the code inside the comprehension run (though after it runs, it will fail on awaiting list):

    >>> await [await asyncio.sleep(1, print(x)) for x in range(5)] 
    0
    1
    2
    3
    4
    Traceback (most recent call last):
      File "C:\Python38\lib\concurrent\futures\_base.py", line 439, in result
        return self.__get_result()
      File "C:\Python38\lib\concurrent\futures\_base.py", line 388, in __get_result
        raise self._exception
      File "<console>", line 1, in <module>
    TypeError: object list can't be used in 'await' expression

    @pablogsal
    Copy link
    Member

    New changeset 9052f7a by Batuhan Taşkaya in branch 'master':
    bpo-39562: Allow executing asynchronous comprehensions in the asyncio REPL (GH-18968)
    9052f7a

    @miss-islington
    Copy link
    Contributor

    New changeset ec8a973 by Miss Islington (bot) in branch '3.8':
    bpo-39562: Allow executing asynchronous comprehensions in the asyncio REPL (GH-18968)
    ec8a973

    @vstinner
    Copy link
    Member

    This change caused python-gmpy2 tests to fail:
    https://bugzilla.redhat.com/show_bug.cgi?id=1817710

    Reproducer:
    ---

    from __future__ import print_function, division
    import doctest
    
    filename = "doctest.txt"
    with open(filename, "w") as fp:
        print("""
    Test

    ====

        >>> all(x == 1 for x in [1, 1, 1])
        True
        """.strip(), file=fp)
    result = doctest.testfile(filename, globs=globals())
    print("result:", result)

    @isidentical
    Copy link
    Sponsor Member

    Both PyCF_ALLOW_TOP_LEVEL_AWAIT and CO_FUTURE_DIVISION has same value.

    @isidentical
    Copy link
    Sponsor Member

    Sending a patch that would prevent this collision.

    @serhiy-storchaka
    Copy link
    Member

    If CO_FUTURE_DIVISION conflicts with PyCF_ALLOW_TOP_LEVEL_AWAIT, does not CO_ITERABLE_COROUTINE conflict with PyCF_SOURCE_IS_UTF8 and CO_ASYNC_GENERATOR with PyCF_DONT_IMPLY_DEDENT?

    @isidentical
    Copy link
    Sponsor Member

    #define IS_COMPILER_FLAG_ENABLED(c, flag) printf("%s: %d\n", #flag, c->c_flags->cf_flags & flag)

    If CO_FUTURE_DIVISION conflicts with PyCF_ALLOW_TOP_LEVEL_AWAIT, does not CO_ITERABLE_COROUTINE conflict with PyCF_SOURCE_IS_UTF8 and CO_ASYNC_GENERATOR with PyCF_DONT_IMPLY_DEDENT?

    Yes, they do.

    Compiling without anything
    PyCF_SOURCE_IS_UTF8: 256
    CO_ITERABLE_COROUTINE: 256
    PyCF_DONT_IMPLY_DEDENT: 0
    CO_ASYNC_GENERATOR: 0
    Compiling with CO_ASYNC_GENERATOR
    PyCF_SOURCE_IS_UTF8: 256
    CO_ITERABLE_COROUTINE: 256
    PyCF_DONT_IMPLY_DEDENT: 512
    CO_ASYNC_GENERATOR: 512

    This result is a side affect of merging future flags with compiler flags. Even if we access from cf_flags (or the other way around, ff_features) it doesnt change anything because we are merging both flags before we start the process.

    Two ways of escaping this is changing flags to not to conlict with each other or not merging. Not merging is out of this box because it will break user level compile function (it takes both flags in a single parameter, flags). The most reasonable solution I thought was making this flags not to conflict with each other.

    @vstinner
    Copy link
    Member

    Is there an update of this *release blocker* issue? Should we revert the commit 9052f7a?

    @isidentical
    Copy link
    Sponsor Member

    Is there an update of this *release blocker* issue? Should we revert the commit 9052f7a?

    I dont think we can gain much by reverting 9052f7a because it doesn't introduce a new issue, it just adds "another" path to notice it.

    Test:
    >>> await __import__('asyncio').sleep(1)

    Expected (and the behavior of 3.6):

    (.venv) [  6:09PM ]  [ isidentical@threeheadedgiant:~ ]
     $ python3.6 z.py       
    **********************************************************************
    File "doctest.txt", line 4, in doctest.txt
    Failed example:
        await __import__('asyncio').sleep(1)
    Exception raised:
        Traceback (most recent call last):
          File "/usr/lib/python3.6/doctest.py", line 1330, in __run
            compileflags, 1), test.globs)
          File "<doctest doctest.txt[0]>", line 1
            await __import__('asyncio').sleep(1)
                           ^
        SyntaxError: invalid syntax
    **********************************************************************
    1 items had failures:
       1 of   1 in doctest.txt
    ***Test Failed*** 1 failures.
    result: TestResults(failed=1, attempted=1

    Current master (+ reverted GH 18968):
    (.venv) [ 6:09PM ] [ isidentical@threeheadedgiant:~ ]
    $ ./cpython/python z.py
    /home/isidentical/cpython/Lib/doctest.py:1336: RuntimeWarning: coroutine '<module>' was never awaited
    exec(compile(example.source, filename, "single",
    RuntimeWarning: Enable tracemalloc to get the object allocation traceback
    result: TestResults(failed=0, attempted=1)

    @isidentical
    Copy link
    Sponsor Member

    GH 19230:
    (.venv) [  6:17PM ]  [ isidentical@threeheadedgiant:~ ]
     $ ./cpython/python z.py
    **********************************************************************
    File "/home/isidentical/doctest.txt", line 4, in doctest.txt
    Failed example:
        await __import__('asyncio').sleep(1)
    Exception raised:
        Traceback (most recent call last):
          File "/home/isidentical/cpython/Lib/doctest.py", line 1336, in __run
            exec(compile(example.source, filename, "single",
          File "<doctest doctest.txt[0]>", line 1
        SyntaxError: 'await' outside function
    **********************************************************************
    1 items had failures:
       1 of   1 in doctest.txt
    ***Test Failed*** 1 failures.
    result: TestResults(failed=1, attempted=1)

    @vstinner
    Copy link
    Member

    Either the regression should be fixed, or the commits which introduced the regression should be reverted.

    I'm talking about python-gmpy2 doctest which pass on Python 3.8: msg365311.

    @pablogsal
    Copy link
    Member

    Either the regression should be fixed, or the commits which introduced the regression should be reverted.

    If you do that we will have now two problems:

    • The asyncio repr will not work correctly.
    • The flags in compile still have a clash.

    The solution is to merge #19230 or some version of it, which should fix the issue in python-gmpy2 doctest .

    @vstinner
    Copy link
    Member

    New changeset 4454057 by Batuhan Taşkaya in branch 'master':
    bpo-39562: Prevent collision of future and compiler flags (GH-19230)
    4454057

    @vstinner
    Copy link
    Member

    bpo-39562: Prevent collision of future and compiler flags (GH-19230)
    4454057

    I tested manually: this change fix my msg365311 reproducer. Thanks!

    In Python 3.8, PyCF_ALLOW_TOP_LEVEL_AWAIT = CO_FUTURE_DIVISION = 0x2000. The commit 9052f7a was backported to 3.8:

    New changeset ec8a973 by Miss Islington (bot) in branch '3.8':
    bpo-39562: Allow executing asynchronous comprehensions in the asyncio REPL (GH-18968)
    ec8a973

    And now 3.8 branch also has the bug: msg365311 reproducer fails as well in 3.8.

    What should be done? Revert ec8a973? Change constants value in Python 3.8.3? (backport 4454057 to 3.8)

    IMHO reverting the commit ec8a973 (fix async comprehension in asyncio REPL) is the safest option.

    First, I understood that the asyncio REPL was experimental in Python 3.8: https://bugs.python.org/issue37028

    But it's not documented as provisional or experimental in What's New in Python 3.8:
    https://docs.python.org/dev/whatsnew/3.8.html#asyncio

    But I'm also fine with backporting the fix 4454057 (change constants) to 3.8.

    @pablogsal
    Copy link
    Member

    I think we should backport the fix 4454057 (change constants) to 3.8 given that the likelihood of users using the actual hardcoded value of the constants instead of the constants themselves is very low. Also, given the collision, it would be fixing a bug present still in 3.8.

    If we revert 9052f7a we would have two bugs in 3.8: collision of constants and asyncio repr not working properly.

    @miss-islington
    Copy link
    Contributor

    New changeset 5055c27 by Pablo Galindo in branch '3.8':
    [3.8] bpo-39562: Prevent collision of future and compiler flags (GH-19230) (GH-19835)
    5055c27

    @pablogsal
    Copy link
    Member

    New changeset 71e6122 by Pablo Galindo in branch '3.8':
    bpo-39562: Correctly updated the version section in the what's new document (GH-19838)
    71e6122

    @hroncok
    Copy link
    Mannequin

    hroncok mannequin commented May 1, 2020

    Fedora packagers report that this problem is now showing up in 3.8.3rc1. What can be done to ensure that 3.8.3 final will contain the fix?

    @pablogsal
    Copy link
    Member

    The merge is in 3.8 master, so we need to make sure that Łukasz includes this on the 3.8.3 release. Victor sent an email to python-dev already about this issue. If you want to make absolutely sure this happens, maybe send an email directly to Łukasz.

    @hroncok
    Copy link
    Mannequin

    hroncok mannequin commented May 1, 2020

    I just did.

    @vstinner
    Copy link
    Member

    vstinner commented May 7, 2020

    I reopen the bug to ensure that the fix will land into Python 3.8.3. The issue is marked as release blocker.

    @vstinner vstinner reopened this May 7, 2020
    @vstinner vstinner reopened this May 7, 2020
    @vstinner
    Copy link
    Member

    I tested manually the just released Python 3.8.3 with msg365311: I confirm that it's fixed. Thanks!

    @Carreau
    Copy link
    Mannequin

    Carreau mannequin commented Jul 6, 2020

    This make non-await list comprehension coroutine-code-objects as well:

    https://bugs.python.org/issue41218

        import ast
        import inspect
        cell = '[x for x in l]'
        code = compile(cell, "<>", "exec", flags=getattr(ast,'PyCF_ALLOW_TOP_LEVEL_AWAIT', 0x0))
    inspect.CO_COROUTINE & code.co_flags == inspect.CO_COROUTINE  # this is now TRUE. 
    

    This leads to weird things in Jupyter/IPython when we try to detect wether a block of code is, or os not async.

    @Carreau
    Copy link
    Mannequin

    Carreau mannequin commented Jul 6, 2020

    Pablo, I see you reopened and marked as blocker,

    As the changes here has been released maybe you want to reclose and marked https://bugs.python.org/issue41218 as blocker it should also be fixed by
    #21357

    @pablogsal
    Copy link
    Member

    As the changes here has been released maybe you want to reclose and marked https://bugs.python.org/issue41218 as blocker it should also be fixed by
    #21357

    Thanks for the heads up, Matthias!

    Yes, I think is the best thing to do. For some reason, I thought this was not still released :(

    @asottile
    Copy link
    Mannequin

    asottile mannequin commented Aug 24, 2021

    this maybe shouldn't have been backported to 3.8.x as the change in compiler flags appears to break pyc files in subtle ways: https://stackoverflow.com/q/68910329/812183

    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

    5 participants