From 8b62537847dd0d85b7379559b841c832181b1022 Mon Sep 17 00:00:00 2001 From: RongJie G <111257566+CorgiBoyG@users.noreply.github.com> Date: Wed, 14 Oct 2026 06:43:07 +0800 Subject: [PATCH] fix(sandbox): honor the readiness budget on a monotonic clock (#6823) wait_for_sandbox_ready polls a sandbox's health endpoint until ready or a timeout-second budget elapses, but measured that budget on the wall clock (start_time = time.time(), while time.time() - start_time < timeout). A backward wall-clock step (NTP correction, VM resume, manual change) while polling keeps the loop running far past timeout -- and this runs on the sandbox acquisition path, so it pins the acquiring thread instead of giving up -- while a forward jump ends the wait early and destroys a sandbox that was about to become ready. The loop also only checked the deadline at the top and then blocked on a fixed 5s request timeout plus a fixed 1s sleep, so a single slow probe could overshoot timeout by up to ~6s; on the 5s acquisition path (local_backend) that roughly doubles the budget. Rework the wait to a monotonic deadline that clamps both the per-probe request timeout and the inter-probe sleep to the remaining budget (min(5.0, remaining) / min(1.0, remaining)), making the sync twin a faithful mirror of wait_for_sandbox_ready_async, which already does this on loop.time(). Add two deterministic regressions: one advances a scripted monotonic clock past the deadline while a fake wall clock steps backward (red on the old wall-clock loop), and one asserts the per-probe timeout and sleep are clamped to the remaining budget (red on the old fixed 5s/1s values). Signed-off-by: CorgiBoyG <111257566+CorgiBoyG@users.noreply.github.com> --- .../deerflow/community/aio_sandbox/backend.py | 23 +++- backend/tests/test_aio_sandbox_readiness.py | 111 +++++++++++++++++- 2 files changed, 129 insertions(+), 5 deletions(-) diff --git a/backend/packages/harness/deerflow/community/aio_sandbox/backend.py b/backend/packages/harness/deerflow/community/aio_sandbox/backend.py index 7ad71fa02c..346177bce1 100644 --- a/backend/packages/harness/deerflow/community/aio_sandbox/backend.py +++ b/backend/packages/harness/deerflow/community/aio_sandbox/backend.py @@ -72,19 +72,34 @@ def wait_for_sandbox_ready( Returns: True if sandbox is ready, False otherwise. """ - start_time = time.time() + # Measure the readiness budget on the monotonic clock and honor it the way + # the async twin does. ``timeout`` is an elapsed-time budget, so a backward + # wall-clock step (NTP correction, VM resume, manual change) must not stretch + # the wait — which on the acquisition path would pin the thread far past + # ``timeout`` — and a forward jump must not cut it short and destroy a sandbox + # that was about to become ready. Clamp both the per-probe request timeout and + # the inter-probe sleep to the remaining budget (``min(5.0, remaining)`` / + # ``min(1.0, remaining)``) so a single slow probe plus its backoff cannot + # overshoot ``timeout``, mirroring ``wait_for_sandbox_ready_async``. + deadline = time.monotonic() + timeout with requests.Session() as session: session.trust_env = sandbox_http_trust_env(sandbox_url) if headers: session.headers.update(headers) - while time.time() - start_time < timeout: + while True: + remaining = deadline - time.monotonic() + if remaining <= 0: + break try: - response = session.get(f"{sandbox_url}/v1/sandbox", timeout=5) + response = session.get(f"{sandbox_url}/v1/sandbox", timeout=min(5.0, remaining)) if response.status_code == 200: return True except requests.exceptions.RequestException: pass - time.sleep(1) + remaining = deadline - time.monotonic() + if remaining <= 0: + break + time.sleep(min(1.0, remaining)) return False diff --git a/backend/tests/test_aio_sandbox_readiness.py b/backend/tests/test_aio_sandbox_readiness.py index 8d0b092230..758b66b106 100644 --- a/backend/tests/test_aio_sandbox_readiness.py +++ b/backend/tests/test_aio_sandbox_readiness.py @@ -87,6 +87,7 @@ def test_wait_for_sandbox_ready_bypasses_environment_proxy_for_docker_host(monke def get(self, url: str, *, timeout: float): assert url == "http://host.docker.internal:8080/v1/sandbox" + # Ample budget remains, so the per-probe timeout clamps to the 5s ceiling. assert timeout == 5 return SimpleNamespace(status_code=200) @@ -96,7 +97,7 @@ def test_wait_for_sandbox_ready_bypasses_environment_proxy_for_docker_host(monke assert ( readiness.wait_for_sandbox_ready( "http://host.docker.internal:8080", - timeout=1, + timeout=30, headers=headers, ) is True @@ -201,3 +202,111 @@ async def test_wait_for_sandbox_ready_async_clamps_request_and_sleep_to_deadline assert calls == ["http://sandbox/v1/sandbox"] assert request_timeouts == [1.5] assert sleeps == [0.25] + + +def test_wait_for_sandbox_ready_times_out_on_the_monotonic_clock(monkeypatch: pytest.MonkeyPatch) -> None: + """The sync readiness budget is measured on the monotonic clock. + + Regression for the sync poller measuring its ``timeout`` with wall-clock + ``time.time()``: a backward wall-clock step (NTP / VM resume / manual change) + while polling must not stretch the readiness wait past ``timeout`` — on the + acquisition path that would pin the acquiring thread far past the budget. The + async twin already deadlines on ``loop.time()``; this pins the sync twin to + the same monotonic contract. + + The monotonic clock is scripted to advance past the deadline after two polls + while the wall clock steps backward on every read. With the fix the loop + exits on the monotonic deadline after exactly two polls; a wall-clock loop + would never reach ``timeout`` and keep polling until the scripted response + queue is exhausted, surfacing as a hard failure instead of a hang. + """ + # start, loop1 top, after get1, loop2 top, after get2, loop3 top (past deadline) + monotonic_values = iter([100.0, 100.1, 100.2, 100.3, 100.4, 102.5]) + + def fake_monotonic() -> float: + return next(monotonic_values) + + wall = 1_000_000.0 + + def fake_wall_time() -> float: + # Steps backward on every read; a wall-clock loop never reaches timeout. + nonlocal wall + wall -= 1_000.0 + return wall + + sleeps: list[float] = [] + + class FakeSession: + trust_env = True + + def __init__(self) -> None: + self.headers: dict[str, str] = {} + # Exactly two "not ready" probes are scripted. The monotonic loop + # makes two polls before the deadline; a wall-clock loop would make a + # third and raise StopIteration here. + self._responses = iter([SimpleNamespace(status_code=503), SimpleNamespace(status_code=503)]) + + def __enter__(self): + return self + + def __exit__(self, *_exc_info) -> None: + return None + + def get(self, url: str, *, timeout: float): + return next(self._responses) + + monkeypatch.setattr(readiness.requests, "Session", FakeSession) + monkeypatch.setattr(readiness.time, "monotonic", fake_monotonic) + monkeypatch.setattr(readiness.time, "time", fake_wall_time) + monkeypatch.setattr(readiness.time, "sleep", lambda delay: sleeps.append(delay)) + + assert readiness.wait_for_sandbox_ready("http://sandbox", timeout=2) is False + assert sleeps == [1.0, 1.0] + + +def test_wait_for_sandbox_ready_clamps_request_and_sleep_to_deadline(monkeypatch: pytest.MonkeyPatch) -> None: + """The sync poller clamps the per-probe timeout and sleep to the budget. + + Regression for the sync twin overshooting ``timeout``: it polled with a + fixed 5s request timeout and slept a fixed 1s, so a single slow probe plus + its backoff could run the wait up to ~6s past the budget — on the 5s + acquisition path (``local_backend``) that roughly doubles it. Mirror the + async twin's clamp (``min(5.0, remaining)`` / ``min(1.0, remaining)``) so the + budget the readiness comment now promises is actually honored. Scripted + monotonic values match ``...async_clamps_request_and_sleep_to_deadline``. + """ + # start, loop top, after get (503), loop top (past deadline) + monotonic_values = iter([100.0, 100.5, 101.75, 102.0]) + + def fake_monotonic() -> float: + return next(monotonic_values) + + request_timeouts: list[float] = [] + sleeps: list[float] = [] + + class FakeSession: + trust_env = True + + def __init__(self) -> None: + self.headers: dict[str, str] = {} + + def __enter__(self): + return self + + def __exit__(self, *_exc_info) -> None: + return None + + def get(self, url: str, *, timeout: float): + request_timeouts.append(timeout) + return SimpleNamespace(status_code=503) + + monkeypatch.setattr(readiness.requests, "Session", FakeSession) + monkeypatch.setattr(readiness.time, "monotonic", fake_monotonic) + monkeypatch.setattr(readiness.time, "sleep", lambda delay: sleeps.append(delay)) + + # timeout=2 -> deadline at +2. First probe sees remaining 1.5 (clamped under + # the 5s ceiling); the post-probe remaining is 0.25, clamping the sleep; the + # next loop check is exactly at the deadline and exits. + assert readiness.wait_for_sandbox_ready("http://sandbox", timeout=2) is False + assert request_timeouts == [1.5] + assert sleeps == [0.25]