Skip to content

Close async TCP sockets when connection setup is interrupted - #51

Merged
romis2012 merged 1 commit into
romis2012:masterfrom
mayankdhingra02:fix-connect-socket-cleanup
Oct 31, 2026
Merged

romis2012 merged 1 commit into
romis2012:masterfrom
mayankdhingra02:fix-connect-socket-cleanup

Conversation

@mayankdhingra02

Copy link
Copy Markdown
Contributor

Cancellation or a timeout while establishing an asyncio/Trio TCP connection bypasses the existing OSError cleanup, leaving the newly allocated socket open. A failed local-address bind has the same ownership gap.

Keep socket setup, binding and connection establishment inside the cleanup guard, close on any unsuccessful exit, and re-raise the original exception. Successfully returned sockets remain open for the caller. The stream-based v2 and AnyIO helpers are unchanged.

Validation on macOS:

  • Eight regression/control cases: six fail on the original implementation; all eight pass with the fix. They cover cancellation, timeout, binding failures and usable successful connections.
  • An actual loopback Proxy.connect() task cancellation, without socket/loop mocks, leaves an open descriptor on the original code and a closed descriptor with the fix.
  • Full suites pass on Python 3.9, 3.10, 3.11, 3.12, 3.13, 3.14 and free-threaded 3.14: 403 tests per runtime, including sync, asyncio, Trio, AnyIO and v2 APIs. Ruff checks/formatting and mypy pass on all seven runtimes.
  • The four asyncio cases also pass with optional Trio absent. Wheel and source distribution builds pass.

Linux and Windows remain for CI.

Signed-off-by: Mayank Dhingra <mayankdhingra02@gmail.com>
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.31%. Comparing base (1427381) to head (9807fa6).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #51      +/-   ##
==========================================
- Coverage   94.36%   94.31%   -0.05%     
==========================================
  Files          55       55              
  Lines        1562     1566       +4     
==========================================
+ Hits         1474     1477       +3     
- Misses         88       89       +1     
Flag Coverage Δ
unit 94.31% <100.00%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@romis2012
romis2012 merged commit 60629db into romis2012:master Oct 31, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants