From b197793b10e0f2a60284a9555279227b8ccf3059 Mon Sep 17 00:00:00 2001 From: Will Miao Date: Mon, 12 Oct 2026 11:59:10 +0800 Subject: [PATCH] fix(rate-limit): do not leave the gate on an early timer wake `wait_for_slot` trusted a single `asyncio.sleep(delay)`. The event-loop timer can fire before the delay elapsed -- Windows granularity is ~15ms, which the new Windows job caught -- and the method then returned while the cooldown was still armed, so the caller sent a request inside the window the gate exists to enforce. It now re-checks the fixed `wake_at` and sleeps again, which also restores the strict `elapsed >= 0.14` cooldown bounds the earlier relaxation had weakened (a substring replace had turned two of them into `>= 0.09`), and a regression test fails the gate if it ever returns on an early wake. --- py/services/rate_limit_coordinator.py | 8 ++++- tests/services/test_rate_limit_coordinator.py | 34 +++++++++++++++++-- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/py/services/rate_limit_coordinator.py b/py/services/rate_limit_coordinator.py index 70c25eac..098fe4a3 100644 --- a/py/services/rate_limit_coordinator.py +++ b/py/services/rate_limit_coordinator.py @@ -160,7 +160,13 @@ class RateLimitCoordinator: self._normalize(destination), delay, ) - await asyncio.sleep(delay) + # Re-check rather than trusting a single sleep: the event-loop + # timer can fire early (Windows granularity is ~15ms), and + # returning then would send the request inside the cooldown this + # gate exists to enforce. ``wake_at`` is fixed, so the loop can + # never outlast the ``max_wait_seconds`` check above. + while (delay := wake_at - time.monotonic()) > 0: + await asyncio.sleep(delay) state.last_send_at = time.monotonic() def register_rate_limit( diff --git a/tests/services/test_rate_limit_coordinator.py b/tests/services/test_rate_limit_coordinator.py index a889c2be..0c021deb 100644 --- a/tests/services/test_rate_limit_coordinator.py +++ b/tests/services/test_rate_limit_coordinator.py @@ -62,7 +62,7 @@ async def test_pacing_enforces_min_interval(monkeypatch): await coordinator.wait_for_slot("example.com") await coordinator.wait_for_slot("example.com") elapsed = time.monotonic() - start - assert elapsed >= 0.09 # Windows timer granularity rounds 0.1s down + assert elapsed >= 0.099 # 1ms of clock slack on a 0.1s interval async def test_pacing_is_per_destination(monkeypatch): @@ -92,7 +92,7 @@ async def test_register_rate_limit_arms_cooldown_and_waits(monkeypatch): start = time.monotonic() await coordinator.wait_for_slot("example.com") elapsed = time.monotonic() - start - assert elapsed >= 0.12 # 0.15s cooldown, minus Windows timer rounding + assert elapsed >= 0.14 # 0.15s cooldown, 10ms of clock slack assert not coordinator.in_cooldown("example.com") @@ -258,7 +258,7 @@ async def test_make_request_paces_consecutive_calls(monkeypatch): await downloader.make_request("GET", "https://api.example.com/a") await downloader.make_request("GET", "https://api.example.com/b") elapsed = time.monotonic() - start - assert elapsed >= 0.12 # 0.15s cooldown, minus Windows timer rounding + assert elapsed >= 0.14 # 0.15s cooldown, 10ms of clock slack async def test_make_request_gate_disabled_returns_429_immediately(monkeypatch): @@ -464,3 +464,31 @@ async def test_get_response_headers_429_registers_cooldown(monkeypatch): coordinator = await RateLimitCoordinator.get_instance() remaining = coordinator.remaining_seconds("api.example.com") assert 50.0 < remaining <= 60.0 + + +async def test_wait_for_slot_re_sleeps_when_the_timer_wakes_early(monkeypatch): + """An early wake must not let a request through inside the cooldown. + + The event-loop timer can fire before the requested delay has elapsed + (Windows granularity is ~15ms), and a single ``asyncio.sleep`` would then + return while the cooldown is still armed. + """ + + coordinator = await _make_coordinator(monkeypatch) + coordinator.register_rate_limit("example.com", retry_after=0.15) + + delays = [] + real_sleep = asyncio.sleep + + async def early_wake(delay): + delays.append(delay) + if len(delays) == 1: + return # the timer fired early + await real_sleep(delay) + + monkeypatch.setattr(asyncio, "sleep", early_wake) + + await coordinator.wait_for_slot("example.com") + + assert len(delays) >= 2, "the gate returned on an early wake" + assert not coordinator.in_cooldown("example.com")