From 3e88d2395c8dbd9ab0d46dff443480a268917c5b Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Tue, 16 Jun 2026 16:57:35 -0700 Subject: [PATCH] fix(tls): stub backoff via a _sleep seam, not the global asyncio.sleep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test-postgres failure on test_init_retries_exhausted_raises surfaced the root cause: sleeps held 2275x 0.1 instead of [1.0, 2.0]. Those 0.1s came from a concurrent background poller doing asyncio.sleep(0.1) on anyio's shared (persistent) event loop — the tls retry tests patched the *global* asyncio.sleep, which intercepted that poller too. - Before: the stub didn't yield, so the poller busy-looped and monopolized the loop -> the test hung (the CI-only "after 92%" hang on 3.12+). - The earlier "make the stub yield" change converted the hang into this flood (the poller spins instead of blocking), which is what exposed it. Fix: route init()'s backoff through TLSClient._sleep so the tests stub that method in isolation and never touch the global asyncio.sleep. Tasks sharing the loop are no longer affected; schedule assertions are unchanged. The deeper fragility this exploited — a leaked, un-cancelled background poller surviving on the shared test loop — is left as a follow-up. --- tests/test_tls_client.py | 24 +++++++----------------- turnstone/core/tls.py | 15 ++++++++++++--- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/tests/test_tls_client.py b/tests/test_tls_client.py index 6d256648..031c8ee2 100644 --- a/tests/test_tls_client.py +++ b/tests/test_tls_client.py @@ -96,10 +96,8 @@ def _make_flaky_client(monkeypatch, failures: int): """TLSClient whose CA fetch fails ``failures`` times, then succeeds. Returns (client, calls, sleeps) — mutable lists recording each CA-fetch - attempt and each backoff delay (asyncio.sleep is stubbed out). + attempt and each backoff delay (the client's backoff sleep is stubbed). """ - import asyncio - from turnstone.core.tls import TLSClient client = TLSClient( @@ -118,19 +116,15 @@ def _make_flaky_client(monkeypatch, failures: int): async def ok_request(): pass - real_sleep = asyncio.sleep - async def fake_sleep(delay): - # Record the backoff delay and skip the real wait, but still yield to - # the loop. An async stub that returns without ever suspending lets the - # whole retry run complete in a single event-loop step with no - # checkpoint, which is fragile under the async test runner. + # Stub the client's own _sleep seam, NOT the global asyncio.sleep: + # patching the global also intercepts any concurrent task sharing the + # event loop, which corrupted a background poller and hung CI. sleeps.append(delay) - await real_sleep(0) monkeypatch.setattr(client, "_fetch_ca_cert", flaky_fetch) monkeypatch.setattr(client, "_request_cert", ok_request) - monkeypatch.setattr(asyncio, "sleep", fake_sleep) + monkeypatch.setattr(client, "_sleep", fake_sleep) return client, calls, sleeps @@ -182,8 +176,6 @@ async def test_init_retries_exhausted_raises(monkeypatch): @pytest.mark.anyio async def test_init_retries_discovery_failure(monkeypatch): """Console discovery (not-yet-registered console) is retried too.""" - import asyncio - from turnstone.core.tls import TLSClient client = TLSClient(storage=get_storage(), hostnames=["node-1"]) @@ -195,18 +187,16 @@ async def test_init_retries_discovery_failure(monkeypatch): raise RuntimeError("No console service found in services table.") return "http://console:9999" - real_sleep = asyncio.sleep - async def ok(): pass async def fake_sleep(_delay): - await real_sleep(0) + pass monkeypatch.setattr(client, "_discover_console_url", flaky_discover) monkeypatch.setattr(client, "_fetch_ca_cert", ok) monkeypatch.setattr(client, "_request_cert", ok) - monkeypatch.setattr(asyncio, "sleep", fake_sleep) + monkeypatch.setattr(client, "_sleep", fake_sleep) await client.init(attempts=2) assert attempts == [1, 2] diff --git a/turnstone/core/tls.py b/turnstone/core/tls.py index c8ae0fdd..69f9c48d 100644 --- a/turnstone/core/tls.py +++ b/turnstone/core/tls.py @@ -294,8 +294,6 @@ class TLSClient: Discovery, CA fetch, and cert request are all idempotent, so the whole sequence is retried as a unit. """ - import asyncio - if attempts < 1: # range(1, attempts + 1) would be empty: init() would return # "successfully" with no CA and no cert. @@ -321,7 +319,18 @@ class TLSClient: delay_seconds=delay, error=f"{type(exc).__name__}: {exc}", ) - await asyncio.sleep(delay) + await self._sleep(delay) + + async def _sleep(self, delay: float) -> None: + """Backoff sleep behind a seam so tests can stub it in isolation. + + Patching the module-global ``asyncio.sleep`` would also intercept it + for every other task sharing the event loop; routing the retry backoff + through a method keeps test stubs from corrupting concurrent tasks. + """ + import asyncio + + await asyncio.sleep(delay) def _discover_console_url(self) -> str: """Look up the console URL from the services table."""