From 165b71a562f68e5478a5bc009e8b5a594d948332 Mon Sep 17 00:00:00 2001 From: Jeff West Date: Thu, 21 May 2026 19:55:31 -0500 Subject: [PATCH 1/3] polish(transport): make Sync/AsyncTransport.close() explicitly idempotent - Add a `_closed` flag set in `__init__` and gate `close()` / async `close()` so the second and subsequent calls are no-ops on the underlying httpx client (stacked cleanup: context-manager exits, `atexit`, explicit teardown in tests). - Document idempotence in both close docstrings; raising on use-after-close is intentionally out of scope (separate concern). - Add `test_sync_transport_close_is_idempotent` and `test_async_transport_close_is_idempotent` mirroring the existing `KalshiAuth.test_close_is_idempotent` triple-close pattern. Closes #301. --- kalshi/_base_client.py | 20 ++++++++++++++++++++ tests/test_async_client.py | 18 ++++++++++++++++++ tests/test_client.py | 15 +++++++++++++++ 3 files changed, 53 insertions(+) diff --git a/kalshi/_base_client.py b/kalshi/_base_client.py index bc5497a8..cbc60752 100644 --- a/kalshi/_base_client.py +++ b/kalshi/_base_client.py @@ -227,6 +227,7 @@ def __init__( if config.limits is not None: client_kwargs["limits"] = config.limits self._client = httpx.Client(**client_kwargs) + self._closed = False @property def is_authenticated(self) -> bool: @@ -412,6 +413,16 @@ def request( raise KalshiError("Max retries exhausted") def close(self) -> None: + """Close the underlying httpx client. + + #301: idempotent — safe to call multiple times. The second and + subsequent calls are no-ops so cleanup paths (context-manager + exits, ``atexit`` handlers, explicit ``client.close()`` in tests) + can stack without raising on the already-closed httpx client. + """ + if self._closed: + return + self._closed = True self._client.close() @@ -439,6 +450,7 @@ def __init__( if config.limits is not None: client_kwargs["limits"] = config.limits self._client = httpx.AsyncClient(**client_kwargs) + self._closed = False @property def is_authenticated(self) -> bool: @@ -610,4 +622,12 @@ async def request( raise KalshiError("Max retries exhausted") async def close(self) -> None: + """Close the underlying httpx async client. + + #301: idempotent — safe to call multiple times. See + :meth:`SyncTransport.close` for the rationale. + """ + if self._closed: + return + self._closed = True await self._client.aclose() diff --git a/tests/test_async_client.py b/tests/test_async_client.py index ccddea8c..eb54d594 100644 --- a/tests/test_async_client.py +++ b/tests/test_async_client.py @@ -821,3 +821,21 @@ async def fake_sleep(d: float) -> None: await transport.request("POST", "/portfolio/orders", json={"ticker": "T"}) assert route.call_count == 1 assert isinstance(exc_info.value.__cause__, httpx.ReadError) + + +class TestAsyncTransportLifecycle: + @pytest.mark.asyncio + async def test_async_transport_close_is_idempotent( + self, transport: AsyncTransport + ) -> None: + """#301: async ``close()`` must be safe to call multiple times. + + Mirrors ``test_auth.py::test_close_is_idempotent`` for ``KalshiAuth`` + and the sync ``test_sync_transport_close_is_idempotent``: stacked + cleanup paths must not raise on an already-closed httpx AsyncClient. + """ + await transport.close() + assert transport._closed is True + await transport.close() + await transport.close() # triple-close OK + assert transport._closed is True diff --git a/tests/test_client.py b/tests/test_client.py index d8a7846a..fed5ba2c 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1452,3 +1452,18 @@ def test_post_does_not_retry_on_read_error( transport.request("POST", "/portfolio/orders", json={"ticker": "T"}) assert route.call_count == 1 assert isinstance(exc_info.value.__cause__, httpx.ReadError) + + +class TestSyncTransportLifecycle: + def test_sync_transport_close_is_idempotent(self, transport: SyncTransport) -> None: + """#301: ``close()`` must be safe to call multiple times. + + Mirrors ``test_auth.py::test_close_is_idempotent`` for ``KalshiAuth``: + cleanup paths (context exits, ``atexit``, explicit teardown in tests) + can stack without raising on an already-closed httpx client. + """ + transport.close() + assert transport._closed is True + transport.close() + transport.close() # triple-close OK + assert transport._closed is True From 302db5d51608cc4ba6d6ba87877b81c5f52bf94f Mon Sep 17 00:00:00 2001 From: Jeff West Date: Thu, 21 May 2026 20:03:32 -0500 Subject: [PATCH 2/3] fix-up(transport): note thread-safety scope on close() idempotence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses round-1 review on #309: - Sync close() docstring now explicitly bounds the idempotence contract to sequential callers; concurrent threads degrade to one redundant httpx.Client.close() (itself idempotent). - Async close() docstring notes cooperative scheduling makes it race-free. Skipping a threading.Lock — the KalshiAuth precedent (#267) uses one because get_sign_executor() can recreate the executor under contention, which has no analogue here. SyncTransport.close() is a single httpx close + flag flip; the redundant-close worst case is benign by virtue of httpx own idempotence. --- kalshi/_base_client.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/kalshi/_base_client.py b/kalshi/_base_client.py index cbc60752..d7e55dd0 100644 --- a/kalshi/_base_client.py +++ b/kalshi/_base_client.py @@ -413,12 +413,12 @@ def request( raise KalshiError("Max retries exhausted") def close(self) -> None: - """Close the underlying httpx client. + """Close the underlying httpx client. Idempotent for sequential callers (#301). - #301: idempotent — safe to call multiple times. The second and - subsequent calls are no-ops so cleanup paths (context-manager - exits, ``atexit`` handlers, explicit ``client.close()`` in tests) - can stack without raising on the already-closed httpx client. + Concurrent close() across threads is not race-free — ``_closed`` is + checked-then-set without a lock — but the worst case degrades to + one redundant httpx ``Client.close()`` (itself idempotent), so the + practical contract holds. """ if self._closed: return @@ -622,10 +622,10 @@ async def request( raise KalshiError("Max retries exhausted") async def close(self) -> None: - """Close the underlying httpx async client. + """Close the underlying httpx async client. Idempotent (#301). - #301: idempotent — safe to call multiple times. See - :meth:`SyncTransport.close` for the rationale. + Concurrent close() is safe under asyncio's cooperative scheduling — + no race between the ``_closed`` check and the flip. """ if self._closed: return From 615afa24ea12f893dd091d6eb3c4e37040913e32 Mon Sep 17 00:00:00 2001 From: Jeff West Date: Thu, 21 May 2026 20:10:11 -0500 Subject: [PATCH 3/3] fix-up(transport): explicit bool annotation on _closed (style consistency with auth.py) --- kalshi/_base_client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kalshi/_base_client.py b/kalshi/_base_client.py index d7e55dd0..72980cbd 100644 --- a/kalshi/_base_client.py +++ b/kalshi/_base_client.py @@ -227,7 +227,7 @@ def __init__( if config.limits is not None: client_kwargs["limits"] = config.limits self._client = httpx.Client(**client_kwargs) - self._closed = False + self._closed: bool = False @property def is_authenticated(self) -> bool: @@ -450,7 +450,7 @@ def __init__( if config.limits is not None: client_kwargs["limits"] = config.limits self._client = httpx.AsyncClient(**client_kwargs) - self._closed = False + self._closed: bool = False @property def is_authenticated(self) -> bool: