-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Fix #2189: Wakeup pending waiters #2329
Conversation
aiohttp/connector.py
Outdated
# signal to waiter | ||
waiters = self._waiters[key] | ||
if waiters: | ||
waiter = waiters.pop(0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is required, this could cause IndexError on line 379
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe you should just call _release_waiter
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, figured it out after writing tests.
Please wait for update -- polishing test suite
I don't think you can use |
Well, let's keep complicated synchronization logic for sake of speed. |
…bs/aiohttp into wakeup-connection-waiter-on-error
@fafhrd91 unfortunately |
Nevermind, new implementation passes all tests |
Codecov Report
@@ Coverage Diff @@
## master #2329 +/- ##
==========================================
- Coverage 97.24% 97.23% -0.01%
==========================================
Files 39 39
Lines 8217 8224 +7
Branches 1440 1442 +2
==========================================
+ Hits 7991 7997 +6
Misses 98 98
- Partials 128 129 +1
Continue to review full report at Codecov.
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a [new issue] for related bugs. |
Fix for #2189
@fafhrd91 please review.
Sure, I'll add tests if you agree in general.
BTW logic with waiters is too complicated.
Better to rewrite it with
asyncio.Condition
.