Skip to content

fix(transport): AiohttpTransport.aclose() should not close shared ClientSession#21287

Open
ArivunidhiA wants to merge 2 commits intoBerriAI:mainfrom
ArivunidhiA:fix/aiohttp-transport-shared-session-21116
Open

fix(transport): AiohttpTransport.aclose() should not close shared ClientSession#21287
ArivunidhiA wants to merge 2 commits intoBerriAI:mainfrom
ArivunidhiA:fix/aiohttp-transport-shared-session-21116

Conversation

@ArivunidhiA
Copy link

Fixes #21116

When a shared ClientSession is passed to LiteLLMAiohttpTransport, aclose() unconditionally closes it, breaking other clients still using the same session. AiohttpHandler already handles this correctly with _owns_session (added in #19190), but AiohttpTransport was missed.

Fix: Added _owns_session flag to AiohttpTransport:

  • False when a pre-existing ClientSession is passed in (shared — don't close)
  • True when a factory is passed or when the transport creates a new session itself
  • aclose() now only closes the session if the transport owns it

All 5 code paths where new sessions are created (_get_valid_client_session, retry in handle_async_request) set _owns_session = True.

Changed files:

  • litellm/llms/custom_httpx/aiohttp_transport.py
  • tests/test_litellm/llms/custom_httpx/test_aiohttp_transport.py (2 new tests)

All 14 transport tests pass. ruff clean.

@vercel
Copy link

vercel bot commented Feb 16, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Ready Ready Preview, Comment Feb 16, 2026 2:43am

Request Review

@CLAassistant
Copy link

CLAassistant commented Feb 16, 2026

CLA assistant check
All committers have signed the CLA.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 16, 2026

Greptile Summary

This PR adds an _owns_session flag to AiohttpTransport and LiteLLMAiohttpTransport to prevent aclose() from closing a shared ClientSession that was passed in externally, fixing #21116. This mirrors the existing pattern in BaseLLMAIOHTTPHandler (from #19190).

  • _owns_session is set to False when a pre-existing ClientSession is passed, True when a factory or internally-created session is used
  • All 5 code paths that create new sessions correctly set _owns_session = True
  • aclose() now checks _owns_session before closing
  • Two new tests verify both the shared-session and factory-session behaviors
  • The fix is well-aligned with http_handler.py:866 where shared sessions are passed to LiteLLMAiohttpTransport

Confidence Score: 4/5

  • This PR is safe to merge — it correctly fixes the shared session closure bug with thorough test coverage.
  • The core fix is correct and well-tested. The _owns_session flag is properly set across all session-creation paths. One minor gap exists in _get_valid_client_session where a shared session could still be closed during event loop mismatch, but this is an edge case that doesn't affect the primary fix.
  • litellm/llms/custom_httpx/aiohttp_transport.py — the _get_valid_client_session method has a code path (line 201) that may close a shared session during event loop mismatch without checking _owns_session.

Important Files Changed

Filename Overview
litellm/llms/custom_httpx/aiohttp_transport.py Adds _owns_session flag to prevent closing shared ClientSessions on aclose(). All 5 session-creation code paths correctly set the flag. Minor gap: _get_valid_client_session may still close a shared session during event loop mismatch.
tests/test_litellm/llms/custom_httpx/test_aiohttp_transport.py Two well-targeted tests verify the core fix: shared sessions are not closed, factory-created sessions are. No real network calls. Minor whitespace cleanup in existing tests.

Flowchart

flowchart TD
    A[LiteLLMAiohttpTransport.__init__] --> B{client is ClientSession?}
    B -- Yes --> C["_owns_session = False\n(shared session)"]
    B -- No/Factory --> D["_owns_session = True\n(factory/will create)"]
    
    C --> E[aclose called]
    D --> E
    
    E --> F{_owns_session?}
    F -- False --> G["Skip close\n(shared session safe)"]
    F -- True --> H{client is ClientSession?}
    H -- Yes --> I[Close session]
    H -- No --> G
    
    D --> J[_get_valid_client_session]
    J --> K{Session valid?}
    K -- No --> L["Create new session\n_owns_session = True"]
    K -- Yes --> M[Return existing session]
    
    L --> N[handle_async_request retry]
    N --> O["Create new session\n_owns_session = True"]
Loading

Last reviewed commit: 75dddbd

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 16, 2026

Additional Comments (1)

litellm/llms/custom_httpx/aiohttp_transport.py
Shared session may still be closed here

When a shared ClientSession is passed (_owns_session=False) and _get_valid_client_session() detects an event loop mismatch, line 201 will still attempt to close the shared session via asyncio.create_task(old_session.close()). The _owns_session guard only protects aclose(), but not this internal session-replacement logic.

Consider guarding this close call with _owns_session as well:

            if session_loop is None or session_loop != current_loop or session_loop.is_closed():
                # Close old session to prevent leaks
                old_session = self.client
                if self._owns_session:
                    try:
                        if not old_session.closed:
                            try:
                                asyncio.create_task(old_session.close())
                            except RuntimeError:
                                # Different event loop - can't schedule task, rely on GC
                                verbose_logger.debug("Old session from different loop, relying on GC")
                    except Exception as e:
                        verbose_logger.debug(f"Error closing old session: {e}")

This keeps the behavior consistent: if we don't own the session, we shouldn't close it anywhere — just replace our reference to it.

Address Greptile review: _get_valid_client_session could still close a
shared session during event loop mismatch. Now checks _owns_session
before attempting to close the old session.
@ArivunidhiA
Copy link
Author

The 6 failing "LiteLLM Unit Tests (Matrix)" checks (integrations, llms, proxy-guardrails, proxy-unit-a, proxy-unit-b, root) are pre-existing failures that affect all fork PRs due to missing repo secrets. These same tests fail on unrelated docs-only PRs (e.g., #21283, #21282).

All checks that can run on fork PRs pass successfully.

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.

[Bug]: AiohttpTransport.aclose() closes shared ClientSession

2 participants