Skip to content

Conversation

@bdraco
Copy link
Member

@bdraco bdraco commented May 28, 2025

What do these changes do?

This PR fixes an issue where BaseConnector.close() would return immediately without waiting for connections to actually close. This is a partial backport of #3733 that includes only the bug fix without any breaking API changes.

The fix ensures that close() properly waits for all connection closures to complete by:

  • Collecting all proto.closed futures from connections being closed
  • Using asyncio.gather() to wait for all closures to complete
  • Optimizing for Python 3.12+ by using eager task execution when available

Are there changes in behavior for the user?

No breaking changes. The close() method will now properly wait for connections to close as expected, which is the correct behavior. The method continues to return an awaitable that emits a deprecation warning if not awaited.

Note: When using the connector as a non-async context manager (deprecated pattern), the issue of not waiting for connections to close still remains. This is because __exit__ calls _close() synchronously and cannot await the connection closures. Users should use the async context manager pattern (async with) instead. We cannot remove the sync context manager in 3.x as that would be a breaking change.

Related issue number

Partial backport of #3733
fixes #1925 fixes #3736

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • If you provide code modification, please add yourself to CONTRIBUTORS.txt
  • Add a new news fragment into the CHANGES/ folde

Thie is a partial backport of #3733 without the breaking
changes

fixes #1925
@bdraco bdraco changed the title Wait for connections to close in connector.close() [3.13] Wait for connections to close in connector.close() May 28, 2025
@bdraco bdraco changed the title [3.13] Wait for connections to close in connector.close() [3.13] Fix connector not waiting for connections to close May 28, 2025
@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label May 28, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented May 28, 2025

CodSpeed Performance Report

Merging #11074 will not alter performance

Comparing backport_close_fix (9998fb1) with 3.13 (05d39fe)

Summary

✅ 59 untouched benchmarks

@codecov
Copy link

codecov bot commented May 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.24%. Comparing base (05d39fe) to head (9998fb1).
⚠️ Report is 224 commits behind head on 3.13.

Additional details and impacted files
@@           Coverage Diff           @@
##             3.13   #11074   +/-   ##
=======================================
  Coverage   98.24%   98.24%           
=======================================
  Files         130      130           
  Lines       41612    41664   +52     
  Branches     2286     2291    +5     
=======================================
+ Hits        40880    40933   +53     
  Misses        557      557           
+ Partials      175      174    -1     
Flag Coverage Δ
CI-GHA 98.13% <98.82%> (+<0.01%) ⬆️
OS-Linux 97.87% <98.82%> (-0.01%) ⬇️
OS-Windows 95.47% <98.82%> (+0.01%) ⬆️
OS-macOS 97.08% <98.82%> (-0.01%) ⬇️
Py-3.10.11 96.97% <97.64%> (+<0.01%) ⬆️
Py-3.10.17 97.47% <97.64%> (-0.02%) ⬇️
Py-3.11.12 97.56% <97.64%> (+0.02%) ⬆️
Py-3.11.9 97.05% <97.64%> (-0.01%) ⬇️
Py-3.12.10 97.92% <97.64%> (-0.01%) ⬇️
Py-3.13.3 97.92% <97.64%> (+0.01%) ⬆️
Py-3.9.13 96.86% <97.64%> (+<0.01%) ⬆️
Py-3.9.22 97.37% <97.64%> (-0.02%) ⬇️
Py-pypy7.3.16 83.17% <97.64%> (-1.57%) ⬇️
VM-macos 97.08% <98.82%> (-0.01%) ⬇️
VM-ubuntu 97.87% <98.82%> (-0.01%) ⬇️
VM-windows 95.47% <98.82%> (+0.01%) ⬆️

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

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

@bdraco bdraco changed the title [3.13] Fix connector not waiting for connections to close [PR #3733/6b0bc4ef backport][3.13] Fix connector not waiting for connections to close May 28, 2025
@bdraco bdraco marked this pull request as ready for review May 29, 2025 00:00
@bdraco bdraco requested review from asvetlov and webknjaz as code owners May 29, 2025 00:00
@bdraco bdraco merged commit e550c78 into 3.13 May 29, 2025
36 checks passed
@bdraco bdraco deleted the backport_close_fix branch May 29, 2025 00:01
@patchback
Copy link
Contributor

patchback bot commented May 29, 2025

Backport to 3.11: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.11/e550c78a943be34ffd01b58acbcc0f06485bbbd9/pr-11074

Backported as #11076

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request May 29, 2025
…ections to close (#11074)

fixes #1925 fixes #3736

(cherry picked from commit e550c78)
@patchback
Copy link
Contributor

patchback bot commented May 29, 2025

Backport to 3.12: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.12/e550c78a943be34ffd01b58acbcc0f06485bbbd9/pr-11074

Backported as #11077

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request May 29, 2025
…ections to close (#11074)

fixes #1925 fixes #3736

(cherry picked from commit e550c78)
bdraco added a commit that referenced this pull request May 29, 2025
…nections to close (#11076)

Co-authored-by: J. Nick Koston <[email protected]>
fixes #1925 fixes #3736
bdraco added a commit that referenced this pull request May 29, 2025
…nections to close (#11077)

Co-authored-by: J. Nick Koston <[email protected]>
fixes #1925 fixes #3736
@bdraco bdraco mentioned this pull request May 30, 2025
1 task
@God-damnit-all
Copy link

I recommend editing the release notes for v3.13.0 to mention this bugfix, since it's fairly important.

@Dreamsorcerer
Copy link
Member

Hmm, where did that change fragment go? It wasn't in the branch during release..

@Dreamsorcerer
Copy link
Member

Oh, no, this is already released previously:
faa742b

@God-damnit-all
Copy link

Oh, no, this is already released previously: faa742b

Strange, it's in the least of commit differences between 3.12.15 and 3.13.0: v3.12.15...v3.13.0

jinyingli-saturn added a commit to Saturn-Technologies/async-pynamodb that referenced this pull request Dec 12, 2025
Upgrading aiohttp to at least 3.12.4.
This includes aio-libs/aiohttp#11074 which fixes
connections not being immediately closed (which potentially results in
us exhausting our connections if we open a lot in a short span of time).

---------

Co-authored-by: Jinying Li <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:chronographer:provided There is a change note present in this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants