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-35409: Ignore GeneratorExit in async_gen_athrow_throw #14755

Merged
merged 1 commit into from
Nov 19, 2019

Conversation

vxgmichel
Copy link
Contributor

@vxgmichel vxgmichel commented Jul 13, 2019

Ignore GeneratorExit exceptions when throwing an exception into the aclose coroutine of an asynchronous generator.

https://bugs.python.org/issue35409

Automerge-Triggered-By: @asvetlov

@vxgmichel vxgmichel requested a review from 1st1 as a code owner July 13, 2019 16:04
mozillazg pushed a commit to mozillazg/pypy that referenced this pull request Jul 14, 2019
@vxgmichel
Copy link
Contributor Author

Corresponding PR on pypy: https://bitbucket.org/pypy/pypy/pull-requests/655

Copy link
Member

@1st1 1st1 left a comment

Choose a reason for hiding this comment

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

Can you double check if regular generators have the same bahavior before we merge?

@vxgmichel
Copy link
Contributor Author

@1st1

Can you double check if regular generators have the same behavior before we merge?

Well, I don't think this behavior applies to regular generators. The closest thing I could think of is:

>>> def gen():
...     try:
...         yield
...     finally:
...         yield
... 
>>> g = gen()
>>> g.send(None)
>>> g.close()

which produces:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
RuntimeError: generator ignored GeneratorExit

Also note that the chunk of code added to async_gen_athrow_throw is directly taken from async_gen_athrow_send. A bit of refactoring might be required, although I don't feel comfortable enough to do it myself. A similar refactoring has been done in pypy: see the handle_error method which is used by both the send and throw methods of the athrow coroutine.

@vxgmichel
Copy link
Contributor Author

Just pinging, as it turns out to be a blocking issue for the aiostream library :)

@1st1 1st1 requested a review from asvetlov October 4, 2019 14:57
@1st1
Copy link
Member

1st1 commented Oct 4, 2019

Andrew, can you take a look at this too?

@asvetlov
Copy link
Contributor

asvetlov commented Oct 4, 2019

Is it urgent?
I'll have time next week only after finishing my business trip.

@1st1
Copy link
Member

1st1 commented Oct 4, 2019

I guess since we missed the RC1 for this it's no longer urgent. But please look at this as soon as you can. I'd like to hear your opinion.

@vxgmichel
Copy link
Contributor Author

Hi all, I'm just pinging again as I would really like to have this issue fixed :)

@asvetlov
Copy link
Contributor

LGTM

@miss-islington
Copy link
Contributor

Sorry, I can't merge this PR. Reason: Base branch was modified. Review and try the merge again..

@miss-islington miss-islington merged commit 8e0de2a into python:master Nov 19, 2019
@miss-islington
Copy link
Contributor

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

@miss-islington
Copy link
Contributor

I'm having trouble backporting to 3.8. Reason: 'Error 110 while writing to socket. Connection timed out.'. Please retry by removing and re-adding the needs backport to 3.8 label.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 19, 2019
…4755)

Ignore `GeneratorExit` exceptions when throwing an exception into the `aclose` coroutine of an asynchronous generator.

https://bugs.python.org/issue35409
(cherry picked from commit 8e0de2a)

Co-authored-by: Vincent Michel <vxgmichel@gmail.com>
@bedevere-bot
Copy link

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

@asvetlov asvetlov added needs backport to 3.8 only security fixes and removed needs backport to 3.8 only security fixes labels Nov 19, 2019
@miss-islington
Copy link
Contributor

Thanks @vxgmichel for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 19, 2019
…4755)

Ignore `GeneratorExit` exceptions when throwing an exception into the `aclose` coroutine of an asynchronous generator.

https://bugs.python.org/issue35409
(cherry picked from commit 8e0de2a)

Co-authored-by: Vincent Michel <vxgmichel@gmail.com>
@bedevere-bot bedevere-bot removed the needs backport to 3.8 only security fixes label Nov 19, 2019
@bedevere-bot
Copy link

GH-17258 is a backport of this pull request to the 3.8 branch.

@asvetlov
Copy link
Contributor

Thanks!

miss-islington added a commit that referenced this pull request Nov 19, 2019
Ignore `GeneratorExit` exceptions when throwing an exception into the `aclose` coroutine of an asynchronous generator.

https://bugs.python.org/issue35409
(cherry picked from commit 8e0de2a)

Co-authored-by: Vincent Michel <vxgmichel@gmail.com>
miss-islington added a commit that referenced this pull request Nov 19, 2019
Ignore `GeneratorExit` exceptions when throwing an exception into the `aclose` coroutine of an asynchronous generator.

https://bugs.python.org/issue35409
(cherry picked from commit 8e0de2a)

Co-authored-by: Vincent Michel <vxgmichel@gmail.com>
jacobneiltaylor pushed a commit to jacobneiltaylor/cpython that referenced this pull request Dec 5, 2019
…4755)

Ignore `GeneratorExit` exceptions when throwing an exception into the `aclose` coroutine of an asynchronous generator.





https://bugs.python.org/issue35409
shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Jan 31, 2020
…4755)

Ignore `GeneratorExit` exceptions when throwing an exception into the `aclose` coroutine of an asynchronous generator.





https://bugs.python.org/issue35409
tekknolagi pushed a commit to facebookarchive/skybison that referenced this pull request Aug 26, 2021
Summary: Apply the changes from python/cpython#14755 to Pyro

Based on Facebook D27943651
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants