-
-
Notifications
You must be signed in to change notification settings - Fork 718
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
Ensure connectionpool does not leave comms if closed mid connect #4951
Conversation
I retriggered the run to see if the register_backend_entrypoint issue persists |
The register_backend_entrypoint issue seems to be unrelated (I believe) but is still persistent, see #4961 |
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.
This seems ok to me
# We might still have tasks haning in the semaphore. This will let them | ||
# run into an exception and raise a commclosed | ||
while self._n_connecting: | ||
await asyncio.sleep(0.005) |
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.
Just checking, the finally
block is triggered when we cancel
?
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.
Yes. That's covered in the test, ensured by assert pool._connecting
in line 651
+1 from me |
Co-authored-by: Matthew Rocklin <mrocklin@gmail.com>
66b9a16
to
14c10af
Compare
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.
Thanks @fjetter! This is in
There is a chance for the
ConnectionPool
to leave connections open even after it was closed.This came up during #4734