Skip to content

Commit 3a8e5fa

Browse files
zhansheng.lzsclaude
andcommitted
Fix session pool: remove broken atexit handler and clean stale entries
The atexit handler created a new event loop to close sessions bound to the original loop, which silently failed. OS reclaims resources on exit, so explicit close is unnecessary. Also adds stale-entry cleanup in get_shared_aio_session() to prevent unbounded dict growth when event loops are repeatedly created and closed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent ac32bc5 commit 3a8e5fa

4 files changed

Lines changed: 36 additions & 27 deletions

File tree

dashscope/api_entities/aio_session.py

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,15 @@
77
is created once and shared across all sessions.
88
"""
99
import asyncio
10-
import atexit
1110
import ssl
12-
from typing import Dict, Optional
11+
import weakref
12+
from typing import Optional
1313

1414
import aiohttp
1515
import certifi
1616

1717
_shared_ssl_context: Optional[ssl.SSLContext] = None
18-
_aio_sessions: Dict[int, aiohttp.ClientSession] = {}
18+
_aio_sessions: "weakref.WeakKeyDictionary" = weakref.WeakKeyDictionary()
1919

2020

2121
def get_ssl_context() -> ssl.SSLContext:
@@ -35,37 +35,20 @@ async def get_shared_aio_session() -> aiohttp.ClientSession:
3535
is handled by the underlying TCPConnector.
3636
"""
3737
loop = asyncio.get_running_loop()
38-
loop_id = id(loop)
39-
session = _aio_sessions.get(loop_id)
38+
39+
session = _aio_sessions.get(loop)
4040
if session is not None and not session.closed:
4141
return session
4242

4343
connector = aiohttp.TCPConnector(ssl=get_ssl_context())
4444
session = aiohttp.ClientSession(connector=connector)
45-
_aio_sessions[loop_id] = session
45+
_aio_sessions[loop] = session
4646
return session
4747

4848

4949
async def close_shared_aio_session() -> None:
5050
"""Close the shared session for the current event loop."""
5151
loop = asyncio.get_running_loop()
52-
loop_id = id(loop)
53-
session = _aio_sessions.pop(loop_id, None)
52+
session = _aio_sessions.pop(loop, None)
5453
if session is not None and not session.closed:
5554
await session.close()
56-
57-
58-
def _atexit_close_sessions() -> None:
59-
sessions = list(_aio_sessions.values())
60-
_aio_sessions.clear()
61-
for session in sessions:
62-
if not session.closed:
63-
try:
64-
loop = asyncio.new_event_loop()
65-
loop.run_until_complete(session.close())
66-
loop.close()
67-
except Exception:
68-
pass
69-
70-
71-
atexit.register(_atexit_close_sessions)

dashscope/api_entities/aiohttp_request.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,10 @@ async def _handle_request(self):
251251
try:
252252
session = await get_shared_aio_session()
253253
if self.stream:
254-
request_timeout = aiohttp.ClientTimeout(sock_read=self.timeout)
254+
request_timeout = aiohttp.ClientTimeout(
255+
total=None,
256+
sock_read=self.timeout,
257+
)
255258
else:
256259
request_timeout = aiohttp.ClientTimeout(total=self.timeout)
257260

dashscope/api_entities/http_request.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,10 @@ async def _handle_aio_request(self): # pylint: disable=too-many-branches
170170
session = await get_shared_aio_session()
171171

172172
if self.stream:
173-
request_timeout = aiohttp.ClientTimeout(sock_read=self.timeout)
173+
request_timeout = aiohttp.ClientTimeout(
174+
total=None,
175+
sock_read=self.timeout,
176+
)
174177
else:
175178
request_timeout = aiohttp.ClientTimeout(total=self.timeout)
176179

tests/unit/test_aio_session.py

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
import asyncio
1313
import ssl
14-
from unittest.mock import patch, MagicMock
14+
from unittest.mock import patch
1515

1616
import aiohttp
1717
import pytest
@@ -113,6 +113,26 @@ async def test_close_idempotent(self):
113113
await aio_session.close_shared_aio_session()
114114
await aio_session.close_shared_aio_session()
115115

116+
@pytest.mark.asyncio
117+
async def test_stale_sessions_cleaned_up(self):
118+
"""Test that closed sessions are replaced in the dict."""
119+
s1 = await aio_session.get_shared_aio_session()
120+
loop = asyncio.get_running_loop()
121+
122+
# Manually close without calling close_shared_aio_session
123+
await s1.close()
124+
assert s1.closed
125+
assert loop in aio_session._aio_sessions
126+
127+
# Getting a new session should replace the stale entry
128+
s2 = await aio_session.get_shared_aio_session()
129+
try:
130+
assert s2 is not s1
131+
assert not s2.closed
132+
assert aio_session._aio_sessions[loop] is s2
133+
finally:
134+
await aio_session.close_shared_aio_session()
135+
116136

117137
class TestSessionPerLoop:
118138
"""Test that different event loops get different sessions."""

0 commit comments

Comments
 (0)