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>
This commit is contained in:
RongJie G 2026-10-14 06:43:07 +08:00 committed by GitHub
parent 8ca2910b39
commit 8b62537847
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 129 additions and 5 deletions

View File

@ -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

View File

@ -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]