GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||
GeezerVison is showing github.com — gh-98703: Fix asyncio proactor_events calling _call_connection_lost multiple times by Fidget-Spinner · Pull Request #98704 · python/cpython Conversation
|
|
@kumaraditya303 I don't know why but this started showing on our tests after #98572. I can't tell if that PR introduced a bug or it just exposed something wrong with our implementation. IMO, it seems it's something wrong with our implementation. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Seriously though this has uncovered another race in our implementation. The code does not guard against concurrent closing of the stream which leads to this failure. The Here's a better fix: diff --git a/Lib/asyncio/proactor_events.py b/Lib/asyncio/proactor_events.py index 2685a3376c..c6aab408fc 100644 --- a/Lib/asyncio/proactor_events.py +++ b/Lib/asyncio/proactor_events.py @@ -152,6 +152,8 @@ def _force_close(self, exc): self._loop.call_soon(self._call_connection_lost, exc) def _call_connection_lost(self, exc): + if self._called_connection_lost: + return try: self._protocol.connection_lost(exc) finally: Feel free to update this PR with the patch above, I have verified this. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Co-Authored-By: Kumar Aditya <[email protected]>
Awesome! Thank you for the fix. I was trying to find something more elegant but I'm a noob at asyncio internals :). Your fix is way nicer. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Co-Authored-By: Kumar Aditya <[email protected]>
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
LGTM thanks for the PR!
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I created #98730 to add tests for this. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Ironic.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks @Fidget-Spinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Sorry @Fidget-Spinner, I had trouble checking out the |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
GH-98753 is a backport of this pull request to the 3.10 branch. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…lost multiple times (pythonGH-98704) Co-authored-by: Kumar Aditya <[email protected]> (cherry picked from commit 8a75542) Co-authored-by: Ken Jin <[email protected]>
|
Thanks @Fidget-Spinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
GH-98754 is a backport of this pull request to the 3.11 branch. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…lost multiple times (pythonGH-98704) Co-authored-by: Kumar Aditya <[email protected]> (cherry picked from commit 8a75542) Co-authored-by: Ken Jin <[email protected]>
…ultiple times (GH-98704) Co-authored-by: Kumar Aditya <[email protected]> (cherry picked from commit 8a75542) Co-authored-by: Ken Jin <[email protected]>
…ultiple times (GH-98704) Co-authored-by: Kumar Aditya <[email protected]> (cherry picked from commit 8a75542) Co-authored-by: Ken Jin <[email protected]>
…lost multiple times (pythonGH-98704) Co-authored-by: Kumar Aditya <[email protected]>
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.