Skip to content

Commit 8c24dd0

Browse files
jrvb-rlclaude
andcommitted
Key async pool by event loop with WeakKeyDictionary
Address review feedback: async shared pool is now keyed by the running event loop using weakref.WeakKeyDictionary, so clients in different asyncio.run() calls get separate pools. close() properly awaits aclose() on the last release. __del__ uses a sync-only release path. Async tests converted to async def so they run inside an event loop and can exercise per-loop pool sharing correctly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 01b73fa commit 8c24dd0

2 files changed

Lines changed: 128 additions & 49 deletions

File tree

src/runloop_api_client/_base_client.py

Lines changed: 74 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import asyncio
99
import inspect
1010
import logging
11+
import weakref
1112
import platform
1213
import warnings
1314
import threading
@@ -93,11 +94,22 @@
9394

9495
# Shared HTTP connection pool state. One pool per client type (sync/async),
9596
# refcounted so the pool is closed when the last SDK client releases it.
97+
# The async pool is keyed by event loop because httpx.AsyncClient binds to
98+
# the loop that created it and cannot be reused across asyncio.run() calls.
9699
_pool_lock = threading.Lock()
97100
_shared_sync_client: httpx.Client | None = None
98101
_shared_sync_refcount: int = 0
99-
_shared_async_client: httpx.AsyncClient | None = None
100-
_shared_async_refcount: int = 0
102+
103+
104+
class _AsyncPoolState:
105+
__slots__ = ("client", "refcount")
106+
107+
def __init__(self, client: httpx.AsyncClient) -> None:
108+
self.client = client
109+
self.refcount = 1
110+
111+
112+
_async_pools: weakref.WeakKeyDictionary[asyncio.AbstractEventLoop, _AsyncPoolState] = weakref.WeakKeyDictionary()
101113

102114
# TODO: make base page type vars covariant
103115
SyncPageT = TypeVar("SyncPageT", bound="BaseSyncPage[Any]")
@@ -1516,6 +1528,7 @@ class AsyncAPIClient(BaseClient[httpx.AsyncClient, AsyncStream[Any]]):
15161528
_default_stream_cls: type[AsyncStream[Any]] | None = None
15171529
_uses_shared_pool: bool
15181530
_closed: bool
1531+
_pool_loop: asyncio.AbstractEventLoop | None
15191532

15201533
def __init__(
15211534
self,
@@ -1560,29 +1573,39 @@ def __init__(
15601573
)
15611574

15621575
self._closed = False
1576+
self._pool_loop = None
15631577

15641578
if http_client is not None:
15651579
self._client = http_client
15661580
self._uses_shared_pool = False
15671581
elif shared_http_pool:
1568-
global _shared_async_client, _shared_async_refcount
1569-
with _pool_lock:
1570-
if _shared_async_client is None or _shared_async_client.is_closed:
1571-
_shared_async_client = AsyncHttpxClientWrapper(
1572-
base_url=base_url,
1573-
timeout=cast(Timeout, timeout),
1574-
)
1575-
_shared_async_refcount = 0
1576-
_shared_async_refcount += 1
1577-
self._client = _shared_async_client
1578-
self._uses_shared_pool = True
1582+
self._acquire_shared_pool(base_url, cast(Timeout, timeout))
15791583
else:
15801584
self._client = AsyncHttpxClientWrapper(
15811585
base_url=base_url,
15821586
timeout=cast(Timeout, timeout),
15831587
)
15841588
self._uses_shared_pool = False
15851589

1590+
def _acquire_shared_pool(self, base_url: str | URL, timeout: Timeout) -> None:
1591+
try:
1592+
loop = asyncio.get_running_loop()
1593+
except RuntimeError:
1594+
loop = None
1595+
with _pool_lock:
1596+
pool_state = _async_pools.get(loop) if loop is not None else None
1597+
if pool_state is None or pool_state.client.is_closed:
1598+
pool_state = _AsyncPoolState(
1599+
AsyncHttpxClientWrapper(base_url=base_url, timeout=timeout),
1600+
)
1601+
if loop is not None:
1602+
_async_pools[loop] = pool_state
1603+
else:
1604+
pool_state.refcount += 1
1605+
self._client = pool_state.client
1606+
self._pool_loop = loop
1607+
self._uses_shared_pool = True
1608+
15861609
def is_closed(self) -> bool:
15871610
return self._closed or self._client.is_closed
15881611

@@ -1595,36 +1618,59 @@ async def close(self) -> None:
15951618
return
15961619
self._closed = True
15971620
if self._uses_shared_pool:
1598-
self._release_shared_pool()
1621+
await self._release_shared_pool()
15991622
else:
16001623
await self._client.aclose()
16011624

1602-
def _release_shared_pool(self) -> None:
1603-
global _shared_async_client, _shared_async_refcount
1604-
should_close = False
1605-
client = None
1625+
async def _release_shared_pool(self) -> None:
1626+
client: httpx.AsyncClient | None = None
16061627
with _pool_lock:
16071628
if not self._uses_shared_pool:
16081629
return
16091630
self._uses_shared_pool = False
1610-
_shared_async_refcount -= 1
1611-
if _shared_async_refcount <= 0:
1612-
client = _shared_async_client
1613-
_shared_async_client = None
1614-
_shared_async_refcount = 0
1615-
should_close = True
1616-
if should_close and client is not None:
1631+
loop = self._pool_loop
1632+
if loop is None:
1633+
return
1634+
pool = _async_pools.get(loop)
1635+
if pool is not None:
1636+
pool.refcount -= 1
1637+
if pool.refcount <= 0:
1638+
client = pool.client
1639+
del _async_pools[loop]
1640+
if client is not None:
16171641
try:
1618-
loop = asyncio.get_running_loop()
1619-
loop.create_task(client.aclose())
1642+
await client.aclose()
1643+
except Exception:
1644+
pass
1645+
1646+
def _release_shared_pool_sync(self) -> None:
1647+
"""Best-effort synchronous release for use in __del__."""
1648+
client: httpx.AsyncClient | None = None
1649+
with _pool_lock:
1650+
if not self._uses_shared_pool:
1651+
return
1652+
self._uses_shared_pool = False
1653+
loop = self._pool_loop
1654+
if loop is None:
1655+
return
1656+
pool = _async_pools.get(loop)
1657+
if pool is not None:
1658+
pool.refcount -= 1
1659+
if pool.refcount <= 0:
1660+
client = pool.client
1661+
del _async_pools[loop]
1662+
if client is not None:
1663+
try:
1664+
running_loop = asyncio.get_running_loop()
1665+
running_loop.create_task(client.aclose())
16201666
except Exception:
16211667
pass
16221668

16231669
def __del__(self) -> None:
16241670
if not getattr(self, "_uses_shared_pool", False):
16251671
return
16261672
try:
1627-
self._release_shared_pool()
1673+
self._release_shared_pool_sync()
16281674
except Exception:
16291675
pass
16301676

tests/test_shared_pool.py

Lines changed: 54 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from __future__ import annotations
88

99
import os
10+
import asyncio
1011
from typing import Any, Iterator
1112

1213
import httpx
@@ -36,10 +37,7 @@ def _clear_pool_state() -> None:
3637
_base_mod._shared_sync_client = None
3738
_base_mod._shared_sync_refcount = 0
3839

39-
if _base_mod._shared_async_client is not None and not _base_mod._shared_async_client.is_closed:
40-
pass # async close is best-effort in sync context
41-
_base_mod._shared_async_client = None
42-
_base_mod._shared_async_refcount = 0
40+
_base_mod._async_pools.clear()
4341

4442

4543
def _make_client(**kwargs: Any) -> Runloop:
@@ -193,7 +191,7 @@ def test_copy_of_non_shared_stays_non_shared(self):
193191

194192

195193
class TestAsyncSharedPool:
196-
def test_shared_pool_uses_same_client(self):
194+
async def test_shared_pool_uses_same_client(self):
197195
c1 = _make_async_client(shared_http_pool=True)
198196
c2 = _make_async_client(shared_http_pool=True)
199197

@@ -215,45 +213,80 @@ def test_custom_http_client_bypasses_sharing(self):
215213
assert c1._client is custom
216214
assert c1._uses_shared_pool is False
217215

218-
def test_default_is_shared(self):
216+
async def test_default_is_shared(self):
219217
c1 = _make_async_client()
220218
assert c1._uses_shared_pool is True
221219

222220

221+
def _async_pool_refcount(loop: asyncio.AbstractEventLoop | None) -> int:
222+
pool = _base_mod._async_pools.get(loop) if loop is not None else None
223+
return pool.refcount if pool is not None else 0
224+
225+
223226
class TestAsyncRefcounting:
224-
def test_close_one_keeps_pool_alive(self):
227+
async def test_close_one_keeps_pool_alive(self):
225228
c1 = _make_async_client(shared_http_pool=True)
226229
c2 = _make_async_client(shared_http_pool=True)
230+
loop = c1._pool_loop
227231

228-
# Release c1's ref synchronously (the release_shared_pool is sync)
229-
c1._release_shared_pool()
230-
assert _base_mod._shared_async_refcount == 1
231-
assert _base_mod._shared_async_client is not None
232+
c1._release_shared_pool_sync()
233+
assert _async_pool_refcount(loop) == 1
232234

233-
c2._release_shared_pool()
234-
assert _base_mod._shared_async_refcount == 0
235-
assert _base_mod._shared_async_client is None
235+
c2._release_shared_pool_sync()
236+
assert _async_pool_refcount(loop) == 0
236237

237-
def test_double_release_is_safe(self):
238+
async def test_double_release_is_safe(self):
238239
c1 = _make_async_client(shared_http_pool=True)
239-
c1._release_shared_pool()
240-
c1._release_shared_pool() # should not raise or go negative
241-
assert _base_mod._shared_async_refcount == 0
240+
c1._release_shared_pool_sync()
241+
c1._release_shared_pool_sync() # should not raise or go negative
242+
assert _async_pool_refcount(c1._pool_loop) == 0
242243

243244

244245
class TestAsyncCopy:
245-
def test_copy_inherits_shared_pool(self):
246+
async def test_copy_inherits_shared_pool(self):
246247
c1 = _make_async_client(shared_http_pool=True)
247248
c2 = c1.copy()
249+
loop = c1._pool_loop
248250

249251
assert c2._uses_shared_pool is True
250252
assert c2._client is c1._client
251-
assert _base_mod._shared_async_refcount == 2
253+
assert _async_pool_refcount(loop) == 2
252254

253-
def test_copy_with_custom_client_disables_sharing(self):
255+
async def test_copy_with_custom_client_disables_sharing(self):
254256
c1 = _make_async_client(shared_http_pool=True)
255257
custom = httpx.AsyncClient()
256258
c2 = c1.copy(http_client=custom)
257259

258260
assert c2._uses_shared_pool is False
259261
assert c2._client is custom
262+
263+
264+
class TestAsyncCrossLoop:
265+
def test_separate_loops_get_separate_pools(self):
266+
"""Clients created in different asyncio.run() calls must not share a pool."""
267+
268+
async def create_client() -> int:
269+
c = _make_async_client(shared_http_pool=True)
270+
client_id = id(c._client)
271+
await c.close()
272+
return client_id
273+
274+
id1 = asyncio.run(create_client())
275+
id2 = asyncio.run(create_client())
276+
277+
assert id1 != id2, "each loop should get its own httpx.AsyncClient"
278+
279+
def test_same_loop_shares_pool(self):
280+
"""Clients created in the same asyncio.run() must share a pool."""
281+
282+
async def create_two() -> tuple[int, int]:
283+
c1 = _make_async_client(shared_http_pool=True)
284+
c2 = _make_async_client(shared_http_pool=True)
285+
id1 = id(c1._client)
286+
id2 = id(c2._client)
287+
await c1.close()
288+
await c2.close()
289+
return id1, id2
290+
291+
id1, id2 = asyncio.run(create_two())
292+
assert id1 == id2

0 commit comments

Comments
 (0)