Skip to content

Commit 2728bdc

Browse files
jrvb-rlclaude
andcommitted
Fix shared pool bugs and add tests
- Add hasattr guard to AsyncAPIClient.close() matching sync version - Track per-instance _closed flag so is_closed() works correctly for shared pool clients (underlying transport stays open for others) - Add shared_http_pool param to copy()/with_options() to match __init__ - Add 19 tests covering sharing, refcounting, copy propagation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent bdf843a commit 2728bdc

3 files changed

Lines changed: 283 additions & 8 deletions

File tree

src/runloop_api_client/_base_client.py

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -856,6 +856,7 @@ class SyncAPIClient(BaseClient[httpx.Client, Stream[Any]]):
856856
_client: httpx.Client
857857
_default_stream_cls: type[Stream[Any]] | None = None
858858
_uses_shared_pool: bool
859+
_closed: bool
859860

860861
def __init__(
861862
self,
@@ -899,6 +900,8 @@ def __init__(
899900
_strict_response_validation=_strict_response_validation,
900901
)
901902

903+
self._closed = False
904+
902905
if http_client is not None:
903906
self._client = http_client
904907
self._uses_shared_pool = False
@@ -922,7 +925,7 @@ def __init__(
922925
self._uses_shared_pool = False
923926

924927
def is_closed(self) -> bool:
925-
return self._client.is_closed
928+
return self._closed or self._client.is_closed
926929

927930
def close(self) -> None:
928931
"""Close the underlying HTTPX client.
@@ -931,6 +934,7 @@ def close(self) -> None:
931934
"""
932935
if not hasattr(self, "_client"):
933936
return
937+
self._closed = True
934938
if self._uses_shared_pool:
935939
self._release_shared_pool()
936940
else:
@@ -1511,6 +1515,7 @@ class AsyncAPIClient(BaseClient[httpx.AsyncClient, AsyncStream[Any]]):
15111515
_client: httpx.AsyncClient
15121516
_default_stream_cls: type[AsyncStream[Any]] | None = None
15131517
_uses_shared_pool: bool
1518+
_closed: bool
15141519

15151520
def __init__(
15161521
self,
@@ -1554,6 +1559,8 @@ def __init__(
15541559
_strict_response_validation=_strict_response_validation,
15551560
)
15561561

1562+
self._closed = False
1563+
15571564
if http_client is not None:
15581565
self._client = http_client
15591566
self._uses_shared_pool = False
@@ -1577,13 +1584,16 @@ def __init__(
15771584
self._uses_shared_pool = False
15781585

15791586
def is_closed(self) -> bool:
1580-
return self._client.is_closed
1587+
return self._closed or self._client.is_closed
15811588

15821589
async def close(self) -> None:
15831590
"""Close the underlying HTTPX client.
15841591
15851592
The client will *not* be usable after this.
15861593
"""
1594+
if not hasattr(self, "_client"):
1595+
return
1596+
self._closed = True
15871597
if self._uses_shared_pool:
15881598
self._release_shared_pool()
15891599
else:

src/runloop_api_client/_client.py

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,7 @@ def copy(
254254
base_url: str | httpx.URL | None = None,
255255
timeout: float | Timeout | None | NotGiven = not_given,
256256
http_client: httpx.Client | None = None,
257+
shared_http_pool: bool | None = None,
257258
max_retries: int | NotGiven = not_given,
258259
default_headers: Mapping[str, str] | None = None,
259260
set_default_headers: Mapping[str, str] | None = None,
@@ -283,16 +284,18 @@ def copy(
283284
params = set_default_query
284285

285286
if http_client is not None:
286-
shared_http_pool = False
287+
resolved_shared = False
288+
elif shared_http_pool is not None:
289+
resolved_shared = shared_http_pool
287290
else:
288-
shared_http_pool = self._uses_shared_pool
291+
resolved_shared = self._uses_shared_pool
289292

290293
return self.__class__(
291294
bearer_token=bearer_token or self.bearer_token,
292295
base_url=base_url or self.base_url,
293296
timeout=self.timeout if isinstance(timeout, NotGiven) else timeout,
294297
http_client=http_client,
295-
shared_http_pool=shared_http_pool,
298+
shared_http_pool=resolved_shared,
296299
max_retries=max_retries if is_given(max_retries) else self.max_retries,
297300
default_headers=headers,
298301
default_query=params,
@@ -524,6 +527,7 @@ def copy(
524527
base_url: str | httpx.URL | None = None,
525528
timeout: float | Timeout | None | NotGiven = not_given,
526529
http_client: httpx.AsyncClient | None = None,
530+
shared_http_pool: bool | None = None,
527531
max_retries: int | NotGiven = not_given,
528532
default_headers: Mapping[str, str] | None = None,
529533
set_default_headers: Mapping[str, str] | None = None,
@@ -553,16 +557,18 @@ def copy(
553557
params = set_default_query
554558

555559
if http_client is not None:
556-
shared_http_pool = False
560+
resolved_shared = False
561+
elif shared_http_pool is not None:
562+
resolved_shared = shared_http_pool
557563
else:
558-
shared_http_pool = self._uses_shared_pool
564+
resolved_shared = self._uses_shared_pool
559565

560566
return self.__class__(
561567
bearer_token=bearer_token or self.bearer_token,
562568
base_url=base_url or self.base_url,
563569
timeout=self.timeout if isinstance(timeout, NotGiven) else timeout,
564570
http_client=http_client,
565-
shared_http_pool=shared_http_pool,
571+
shared_http_pool=resolved_shared,
566572
max_retries=max_retries if is_given(max_retries) else self.max_retries,
567573
default_headers=headers,
568574
default_query=params,

tests/test_shared_pool.py

Lines changed: 259 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,259 @@
1+
"""Tests for shared HTTP connection pool behavior.
2+
3+
Verifies that SDK clients share (or don't share) the underlying httpx
4+
transport, and that refcounting correctly manages the pool lifecycle.
5+
"""
6+
7+
from __future__ import annotations
8+
9+
import os
10+
11+
import httpx
12+
import pytest
13+
14+
import runloop_api_client._base_client as _base_mod
15+
from runloop_api_client import Runloop, AsyncRunloop
16+
17+
base_url = os.environ.get("TEST_API_BASE_URL", "http://127.0.0.1:4010")
18+
bearer_token = "My Bearer Token"
19+
20+
21+
@pytest.fixture(autouse=True)
22+
def _reset_shared_pool():
23+
"""Reset module-level shared pool state before and after each test."""
24+
_clear_pool_state()
25+
yield
26+
_clear_pool_state()
27+
28+
29+
def _clear_pool_state():
30+
with _base_mod._pool_lock:
31+
if _base_mod._shared_sync_client is not None and not _base_mod._shared_sync_client.is_closed:
32+
try:
33+
_base_mod._shared_sync_client.close()
34+
except Exception:
35+
pass
36+
_base_mod._shared_sync_client = None
37+
_base_mod._shared_sync_refcount = 0
38+
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
43+
44+
45+
def _make_client(**kwargs) -> Runloop:
46+
kwargs.setdefault("base_url", base_url)
47+
kwargs.setdefault("bearer_token", bearer_token)
48+
return Runloop(**kwargs)
49+
50+
51+
def _make_async_client(**kwargs) -> AsyncRunloop:
52+
kwargs.setdefault("base_url", base_url)
53+
kwargs.setdefault("bearer_token", bearer_token)
54+
return AsyncRunloop(**kwargs)
55+
56+
57+
# ---------------------------------------------------------------------------
58+
# Sync: sharing behavior
59+
# ---------------------------------------------------------------------------
60+
61+
62+
class TestSyncSharedPool:
63+
def test_shared_pool_uses_same_client(self):
64+
c1 = _make_client(shared_http_pool=True)
65+
c2 = _make_client(shared_http_pool=True)
66+
67+
assert c1._client is c2._client
68+
assert c1._uses_shared_pool is True
69+
assert c2._uses_shared_pool is True
70+
71+
c1.close()
72+
c2.close()
73+
74+
def test_private_pool_uses_different_clients(self):
75+
c1 = _make_client(shared_http_pool=False)
76+
c2 = _make_client(shared_http_pool=False)
77+
78+
assert c1._client is not c2._client
79+
assert c1._uses_shared_pool is False
80+
assert c2._uses_shared_pool is False
81+
82+
c1.close()
83+
c2.close()
84+
85+
def test_custom_http_client_bypasses_sharing(self):
86+
custom = httpx.Client()
87+
c1 = _make_client(http_client=custom, shared_http_pool=True)
88+
89+
assert c1._client is custom
90+
assert c1._uses_shared_pool is False
91+
92+
c1.close()
93+
custom.close()
94+
95+
def test_default_is_shared(self):
96+
c1 = _make_client()
97+
assert c1._uses_shared_pool is True
98+
c1.close()
99+
100+
101+
class TestSyncRefcounting:
102+
def test_close_one_keeps_pool_alive(self):
103+
c1 = _make_client(shared_http_pool=True)
104+
c2 = _make_client(shared_http_pool=True)
105+
pool = c1._client
106+
107+
c1.close()
108+
assert not pool.is_closed
109+
assert _base_mod._shared_sync_refcount == 1
110+
111+
c2.close()
112+
assert pool.is_closed
113+
assert _base_mod._shared_sync_client is None
114+
assert _base_mod._shared_sync_refcount == 0
115+
116+
def test_double_close_is_safe(self):
117+
c1 = _make_client(shared_http_pool=True)
118+
c1.close()
119+
c1.close() # should not raise or go negative
120+
assert _base_mod._shared_sync_refcount == 0
121+
122+
def test_three_clients_refcount(self):
123+
c1 = _make_client(shared_http_pool=True)
124+
c2 = _make_client(shared_http_pool=True)
125+
c3 = _make_client(shared_http_pool=True)
126+
pool = c1._client
127+
128+
assert _base_mod._shared_sync_refcount == 3
129+
130+
c1.close()
131+
assert _base_mod._shared_sync_refcount == 2
132+
assert not pool.is_closed
133+
134+
c2.close()
135+
assert _base_mod._shared_sync_refcount == 1
136+
assert not pool.is_closed
137+
138+
c3.close()
139+
assert _base_mod._shared_sync_refcount == 0
140+
assert pool.is_closed
141+
142+
def test_pool_recreated_after_full_release(self):
143+
c1 = _make_client(shared_http_pool=True)
144+
pool1 = c1._client
145+
c1.close()
146+
147+
c2 = _make_client(shared_http_pool=True)
148+
pool2 = c2._client
149+
assert pool2 is not pool1
150+
assert not pool2.is_closed
151+
152+
c2.close()
153+
154+
155+
class TestSyncCopy:
156+
def test_copy_inherits_shared_pool(self):
157+
c1 = _make_client(shared_http_pool=True)
158+
c2 = c1.copy()
159+
160+
assert c2._uses_shared_pool is True
161+
assert c2._client is c1._client
162+
assert _base_mod._shared_sync_refcount == 2
163+
164+
c1.close()
165+
c2.close()
166+
167+
def test_copy_with_custom_client_disables_sharing(self):
168+
c1 = _make_client(shared_http_pool=True)
169+
custom = httpx.Client()
170+
c2 = c1.copy(http_client=custom)
171+
172+
assert c2._uses_shared_pool is False
173+
assert c2._client is custom
174+
175+
c1.close()
176+
c2.close()
177+
custom.close()
178+
179+
def test_copy_of_non_shared_stays_non_shared(self):
180+
c1 = _make_client(shared_http_pool=False)
181+
c2 = c1.copy()
182+
183+
assert c2._uses_shared_pool is False
184+
assert c2._client is not c1._client
185+
186+
c1.close()
187+
c2.close()
188+
189+
190+
# ---------------------------------------------------------------------------
191+
# Async: sharing behavior
192+
# ---------------------------------------------------------------------------
193+
194+
195+
class TestAsyncSharedPool:
196+
def test_shared_pool_uses_same_client(self):
197+
c1 = _make_async_client(shared_http_pool=True)
198+
c2 = _make_async_client(shared_http_pool=True)
199+
200+
assert c1._client is c2._client
201+
assert c1._uses_shared_pool is True
202+
assert c2._uses_shared_pool is True
203+
204+
def test_private_pool_uses_different_clients(self):
205+
c1 = _make_async_client(shared_http_pool=False)
206+
c2 = _make_async_client(shared_http_pool=False)
207+
208+
assert c1._client is not c2._client
209+
assert c1._uses_shared_pool is False
210+
211+
def test_custom_http_client_bypasses_sharing(self):
212+
custom = httpx.AsyncClient()
213+
c1 = _make_async_client(http_client=custom, shared_http_pool=True)
214+
215+
assert c1._client is custom
216+
assert c1._uses_shared_pool is False
217+
218+
def test_default_is_shared(self):
219+
c1 = _make_async_client()
220+
assert c1._uses_shared_pool is True
221+
222+
223+
class TestAsyncRefcounting:
224+
def test_close_one_keeps_pool_alive(self):
225+
c1 = _make_async_client(shared_http_pool=True)
226+
c2 = _make_async_client(shared_http_pool=True)
227+
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+
233+
c2._release_shared_pool()
234+
assert _base_mod._shared_async_refcount == 0
235+
assert _base_mod._shared_async_client is None
236+
237+
def test_double_release_is_safe(self):
238+
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
242+
243+
244+
class TestAsyncCopy:
245+
def test_copy_inherits_shared_pool(self):
246+
c1 = _make_async_client(shared_http_pool=True)
247+
c2 = c1.copy()
248+
249+
assert c2._uses_shared_pool is True
250+
assert c2._client is c1._client
251+
assert _base_mod._shared_async_refcount == 2
252+
253+
def test_copy_with_custom_client_disables_sharing(self):
254+
c1 = _make_async_client(shared_http_pool=True)
255+
custom = httpx.AsyncClient()
256+
c2 = c1.copy(http_client=custom)
257+
258+
assert c2._uses_shared_pool is False
259+
assert c2._client is custom

0 commit comments

Comments
 (0)