PYTHON-3923 - Handle socket closures in tests to avoid ResourceWarning#2805
Open
NoahStapp wants to merge 8 commits into
Open
PYTHON-3923 - Handle socket closures in tests to avoid ResourceWarning#2805NoahStapp wants to merge 8 commits into
NoahStapp wants to merge 8 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to reduce (and now surface) ResourceWarning failures during the test suite by ensuring sockets/clients are properly closed in failure/cancellation paths and by removing warning filters that previously masked leaks.
Changes:
- Close the global (async/sync) test client during package/session teardown.
- Remove
ResourceWarningfilters frompyproject.tomlso unclosed sockets/transports fail tests. - Add additional socket/connection cleanup paths during async connection establishment to prevent leaks when exceptions/cancellations occur.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/asynchronous/init.py | Close the shared async test client during async_teardown. |
| test/init.py | Close the shared sync test client during teardown. |
| pyproject.toml | Remove ResourceWarning filters so leaks are not suppressed. |
| pymongo/asynchronous/pool.py | Add an outer exception handler to close connections when cancellation interrupts after socket creation/handshake steps. |
| pymongo/synchronous/pool.py | Mirror of the pool connection cleanup restructuring in the sync pool. |
| pymongo/pool_shared.py | Ensure raw sockets are closed if not successfully returned/adopted by asyncio transports during async connect/setup. |
Comment on lines
+1097
to
+1104
| # Catch cancellations that interrupt outside the inner try block above | ||
| except BaseException: | ||
| if not conn.closed: | ||
| try: | ||
| await conn.close_conn(ConnectionClosedReason.ERROR) | ||
| except BaseException: # noqa: S110 | ||
| pass | ||
| raise |
Comment on lines
+235
to
+238
| finally: | ||
| # Always close the socket if it wasn't returned to avoid leaks. | ||
| if not sock_returned: | ||
| sock.close() |
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-3923
Changes in this PR
Improve cleanup of created sockets in failure cases to reduce the number of thrown
ResourceWarningduring tests. Remove theResourceWarningfilter so tests fail on encountering them.Test Plan
Regressions will be detected by existing tests failing with
ResourceWarning.Checklist
Checklist for Author
[ ] Did you update the changelog (if necessary)?[ ] Is any followup work tracked in a JIRA ticket? If so, add link(s).Checklist for Reviewer