PYTHON-5867 - Close sockets on interruption or cancellation during async connection creation#2858
Draft
NoahStapp wants to merge 3 commits into
Draft
PYTHON-5867 - Close sockets on interruption or cancellation during async connection creation#2858NoahStapp wants to merge 3 commits into
NoahStapp wants to merge 3 commits into
Conversation
…ync connection creation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a resource-leak scenario in PyMongo’s async connection setup by ensuring sockets/transports are closed when connection creation is interrupted (e.g., task cancellation), which is especially important for event-loop reliability and preventing leaked file descriptors.
Changes:
- Add cancellation/interruption-safe cleanup (
except BaseException: close(); raise) in async socket creation and TLS configuration paths. - Ensure transports are aborted / sockets are closed when failures occur during async protocol interface setup.
- Add a destructor (
__del__) toAsyncNetworkingInterfaceto synchronously close the underlying raw socket if the connection is orphaned (e.g., loop already closed).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
pymongo/pool_shared.py |
Adds broad-exception cleanup to prevent socket leaks during async connection creation/configuration and protocol setup. |
pymongo/network_layer.py |
Adds AsyncNetworkingInterface.__del__ to defensively close raw sockets when async connections are orphaned. |
Comment on lines
+437
to
+448
| def sock(self) -> socket.socket: | ||
| return self.conn[0].get_extra_info("socket") | ||
|
|
||
| def __del__(self) -> None: | ||
| # Synchronously release the raw socket in case the event loop is already closed | ||
| # or this connection was orphaned. | ||
| # Safe even if asyncio has already closed the socket. | ||
| try: | ||
| if self.sock is not None: | ||
| self.sock.close() | ||
| except Exception: # noqa: S110 | ||
| pass |
Comment on lines
+185
to
188
| except BaseException: | ||
| # Protect against cancellation or interruption where the raw socket would otherwise leak | ||
| sock.close() | ||
| raise |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Comment on lines
+140
to
+164
| real_socket_cls = _socket.socket | ||
|
|
||
| def tracking_socket(*args, **kwargs): | ||
| s = real_socket_cls(*args, **kwargs) | ||
| created_sockets.append(s) | ||
| return s | ||
|
|
||
| loop = asyncio.get_running_loop() | ||
| started = asyncio.Event() | ||
| block_forever = asyncio.Event() | ||
|
|
||
| async def slow_sock_connect(sock, addr): | ||
| started.set() | ||
| await block_forever.wait() | ||
|
|
||
| with ( | ||
| patch.object(_socket, "socket", tracking_socket), | ||
| patch.object(loop, "sock_connect", slow_sock_connect), | ||
| ): | ||
| task = asyncio.create_task(pool_shared._async_create_connection(address, options)) | ||
| await asyncio.wait_for(started.wait(), timeout=5) | ||
| task.cancel() | ||
| with self.assertRaises(asyncio.CancelledError): | ||
| await task | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PYTHON-5867
Changes in this PR
Close sockets opened as part of async connection creation when an interruption or cancellation occurs.
Test Plan
Fix existing PyPy test failure.
Checklist
Checklist for Author
Checklist for Reviewer