fix(aiohttp): dispose recycled client sessions deterministically

[yassin-berriai]

Relevant issues

Fixes #24230

Internal mirror of #32003 (credit to @anmolg1997) retargeted at litellm_internal_staging. The original PR targets litellm_oss_staging and has been open since July 3; this brings the fix into internal staging directly. Builds on the analysis in #24231

Linear ticket

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

• I have added meaningful tests
• My PR passes all CI/CD checks (e.g., lint, format, unit tests)
• My PR's scope is as isolated as possible; it only solves 1 specific problem
• I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Screenshots / Proof of Fix

Verbatim repro script from #24230, re-run after the rebase with the import path verified first (imported: .../worktree/litellm/llms/custom_httpx/aiohttp_transport.py)

Before, at the current litellm_internal_staging tip this branch is now based on (3c2264c), with the fix confirmed absent (grep -c _close_recycled_session returns 0):

recycled session replaced: True
old session closed immediately: False
old session closed after loop tick: False   <- session leaked to GC

After, at this branch (214c0bc):

recycled session replaced: True
old session closed immediately: False
old session closed after loop tick: True

Mutation check, both invariants this branch has to hold at once. Restoring the unfixed source and re-running the test file makes the regression tests added here fail, and they pass with the fix

$ pytest tests/test_litellm/llms/custom_httpx/test_aiohttp_transport.py   # unfixed source
6 failed, 28 passed
$ pytest tests/test_litellm/llms/custom_httpx/test_aiohttp_transport.py   # this branch
34 passed
$ pytest tests/test_litellm/llms/custom_httpx/                            # whole directory
204 passed

Dropping the self._owns_session guard that gates disposal makes #34962's shared-session test fail, so the disposal cannot silently start closing a session this transport does not own

$ pytest tests/test_litellm/llms/custom_httpx/test_aiohttp_transport.py   # guard removed
1 failed, 33 passed
FAILED ...::test_stale_loop_rebuild_does_not_close_unowned_session

Type

🐛 Bug Fix

Changes

LiteLLMAiohttpTransport replaced its cached aiohttp.ClientSession in three places without reliably closing the previous session: the loop-mismatch recycle discarded the asyncio.create_task(old_session.close()) reference so the close task could be garbage-collected before it ran, the (RuntimeError, AttributeError) fallback branch replaced the session without closing it at all (the exact repro in #24230), and cross-loop sessions were either abandoned to GC or closed from the wrong loop. Replaced sessions surfaced as intermittent Unclosed client session / Unclosed connector ERROR-severity asyncio log entries at GC time

This PR adds one disposal helper, _close_recycled_session(), used by all three replacement sites (loop-mismatch, fallback branch, and the "Session is closed" retry in handle_async_request). It handles the three lifecycles a recycled session can be in: on the current running loop it schedules the async close and keeps a strong reference in a class-level registry until the task completes; on a loop running in another thread it hands the close to the session's own loop with asyncio.run_coroutine_threadsafe; on a closed or stopped loop it disposes synchronously through the connector teardown aiohttp's own finalizer uses, which releases pooled connections and flips the closed flags the finalizers check

No public API changes; the happy path (valid session on the current loop) is untouched

Commit 214c0bc addresses the Greptile finding: _on_threadsafe_close_done now returns early when the future was cancelled (the foreign loop stopped before the handed-off close ran), matching the guard its sibling _on_close_task_done already had, so future.exception() cannot raise CancelledError out of the callback machinery. Comes with a regression test that fails on the unguarded callback

Rebase onto current staging

The branch is rebased onto 3c2264c to clear a conflict with #34962, which landed in the same code path after this PR was opened. That PR routed every session replacement through a shared _rebuild_session() helper so a rebuilt session keeps the connector's keep-alive config, and added an ownership guard so a transport handed the proxy's shared session leaves it open on rebuild

The resolution keeps _rebuild_session() at every replacement site and gates each _close_recycled_session() call on self._owns_session, read before _rebuild_session() claims ownership for the replacement. Disposal therefore applies to sessions this transport owns, which is every session it built itself, and #34962's shared-session guarantee still holds. The two mutation runs above show both directions are load-bearing: reverting the disposal fails 6 of the tests here, and dropping the ownership guard fails #34962's test_stale_loop_rebuild_does_not_close_unowned_session

Final Attestation

• The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

[greptile-apps]

Greptile Summary

Fixes intermittent Unclosed client session / Unclosed connector asyncio warnings that appeared at GC time when LiteLLMAiohttpTransport replaced its cached aiohttp.ClientSession without deterministically closing the old one.

• Introduces _close_recycled_session(), used at all three replacement sites, dispatching to create_task (same loop, strong reference in class-level registry), run_coroutine_threadsafe (foreign running loop), or synchronous BaseConnector._close teardown (dead/stopped loop).
• Fixes the (RuntimeError, AttributeError) fallback branch that previously discarded the old session without closing it, and fixes handle_async_request to dispose the faulted local session rather than self.client, protecting a concurrently rebuilt healthy session.
• The _on_threadsafe_close_done callback now guards future.cancelled() before calling future.exception(), matching the sibling guard and preventing CancelledError from escaping callback machinery on the foreign loop's thread.

Confidence Score: 5/5

Safe to merge — the change is a targeted disposal fix inside LiteLLMAiohttpTransport, the happy path (valid session on the current loop) is untouched, and the ownership guard preserves the shared-session guarantee from #34962.

The disposal logic covers all three session lifecycle cases correctly. The private BaseConnector._close fallback is properly guarded with a callable check and a broad except, so an aiohttp internals change degrades silently rather than crashing. Eight regression tests confirm both the fix and the ownership guard are load-bearing. No existing tests were weakened, and no real network calls are made in the new test file.

Files Needing Attention: No files require special attention.

Important Files Changed

Filename	Overview
litellm/llms/custom_httpx/aiohttp_transport.py	Adds _close_recycled_session helper covering three session-lifecycle cases (same loop, foreign running loop, dead loop), wires it into all three replacement sites, and holds strong task references in a class-level registry to prevent GC of pending close tasks.
tests/test_litellm/llms/custom_httpx/test_aiohttp_transport.py	Adds 8 targeted regression tests covering the fallback branch, dead-loop disposal, strong-reference registry, foreign-running-loop threadsafe close, cancelled-future callback guard, and concurrent-replacement isolation — all mock-only, no real network calls.

_{Reviews (4): Last reviewed commit: "fix(aiohttp): guard threadsafe close cal..." | Re-trigger Greptile}

[codecov]

Codecov Report

❌ Patch coverage is 83.33333% with 11 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
litellm/llms/custom_httpx/aiohttp_transport.py	83.33%	11 Missing ⚠️

📢 Thoughts on this report? Let us know!

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ anmolg1997
❌ yassin-berriai
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[yassin-berriai]

@greptileai please review the current head 2c22d93

[codspeed-hq]

Merging this PR will not alter performance

✅ 31 untouched benchmarks

---

_{Comparing litellm_mirror_32003_aiohttp_recycled_session (3140976) with litellm_internal_staging (05c9815)¹}

Footnotes

1. No successful run was found on litellm_internal_staging (416e398) during the generation of this report, so 05c9815 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

[yassin-berriai]

Rebased onto current staging (3c2264c) to clear a conflict with #34962, which refactored the same replacement sites behind _rebuild_session() and added an ownership guard. Every _close_recycled_session() call is now gated on self._owns_session, read before _rebuild_session() claims ownership, so disposal covers sessions this transport owns and the shared-session guarantee from #34962 still holds

@greptileai please review the current head 214c0bc

[yassin-berriai]

Pushed a lint-budget fix folded into the commit that introduced the registry: the class-level _background_close_tasks set tripped LIT002, so it now carries a # mutable-ok: marker. The set is the documented asyncio idiom for holding a strong reference to a pending task, so an immutable rewrite would reintroduce the GC race this PR exists to close. scripts/type_discipline_gate.py passes locally against the same base SHA CI used

@greptileai please review the current head 3140976

[yassin-berriai]

Note for whoever picks this up: the code-quality failure is not from this branch. recursive_detector reports one unignored recursive function, _render_all in litellm/proxy/management_endpoints/management_v1/list_framework.py, which arrived with #35308 (416e398, merged into staging today). Reproduced three ways locally: this branch alone reports {}, current staging alone reports _render_all, and this branch merged into the staging tip (what CI actually evaluates) reports only _render_all. Other open PRs still show green because their runs predate that merge

Leaving the ignore list alone here since the violation belongs to staging; it needs a fix on staging or an ignore entry in its own PR