diff --git a/CHANGELOG.md b/CHANGELOG.md index 15ac01e6c4..53234070e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -116,13 +116,21 @@ file is the human-readable summary of what changed and why. costing no space until the guest writes something. On macOS the clone never happened: the copy fell back to writing the whole 8 GB image out byte for byte, filling in the empty space the image leaves sparse. Creating or - resetting a sandbox spent that time writing gigabytes, then the guest booted - while the disk was still catching up and had to compete for it — which is why - a sandbox's first start could crawl and sometimes gave up with "sandbox - server did not become healthy" before finishing. The clone now happens for - real: creation is immediate, a sandbox costs roughly nothing on disk until it - is used, and first start takes about as long as every start after it. + resetting a sandbox spent that time writing gigabytes to disk. The clone now + happens for real: creation is immediate, and a sandbox costs roughly nothing + on disk until it is used. ([#293](https://github.com/quickbeard/codev-code/pull/293)) +- **Starting a sandbox no longer hangs for minutes** — a sandbox would + sometimes sit on "starting" for five minutes and then give up with "sandbox + server did not become healthy", even though the guest inside it had finished + booting seconds in. While waiting for the guest, the app checks its health + over and over, one check at a time. Those checks had no deadline, so when one + connected to a guest that was still opening its port and then got no answer + at all, that single check waited forever and every check behind it queued up + behind it — the sandbox was ready, and nothing was left to notice. Each check + now gives up after two seconds and the next one carries on, so a stalled + connection costs a moment instead of the whole startup. + ([#294](https://github.com/quickbeard/codev-code/pull/294)) - **Work sessions reach the model on WSL's mirrored networking** — on Windows, a sandbox's model traffic crosses from the WSL guest to a relay on the Windows host, and the guest found the host by assuming WSL's default NAT diff --git a/packages/codev-sandbox/src/boot.test.ts b/packages/codev-sandbox/src/boot.test.ts index e769f364b1..f0f918426c 100644 --- a/packages/codev-sandbox/src/boot.test.ts +++ b/packages/codev-sandbox/src/boot.test.ts @@ -1,5 +1,5 @@ import { expect, test } from "bun:test" -import { generateSandboxCredentials, pollSandboxHealth, sandboxBootEnv } from "./boot" +import { checkSandboxHealth, generateSandboxCredentials, pollSandboxHealth, sandboxBootEnv } from "./boot" test("boot env mirrors the WSL sidecar contract and carries the server address", () => { expect( @@ -62,3 +62,42 @@ test("health polling returns once the server responds", async () => { ) expect(checks).toBe(3) }) + +// pollSandboxHealth awaits each probe in turn, so a request that never answers +// used to stall the whole loop: the sandbox stayed "starting" until the start +// timeout gave up, minutes later, even though the guest had come up seconds in. +// A probe must always settle. +test("health probe gives up on a connection that never answers", async () => { + const started = Date.now() + const healthy = await checkSandboxHealth( + "http://127.0.0.1:1", + { username: "codev", password: "secret" }, + // Opens and then hangs, the way an accepted socket to a guest that is not + // listening yet can: no response, no error. + ((_url: string, init?: RequestInit) => + new Promise((_resolve, reject) => { + init?.signal?.addEventListener("abort", () => reject(new Error("aborted"))) + })) as unknown as typeof fetch, + ) + + expect(healthy).toBe(false) + expect(Date.now() - started).toBeLessThan(10_000) +}) + +// The caller's signal must cancel an in-flight probe too, so aborting the poll +// does not leave a request outliving the start it belonged to. +test("health probe aborts with the caller's signal", async () => { + const controller = new AbortController() + const probe = checkSandboxHealth( + "http://127.0.0.1:1", + { username: "codev", password: "secret" }, + ((_url: string, init?: RequestInit) => + new Promise((_resolve, reject) => { + init?.signal?.addEventListener("abort", () => reject(new Error("aborted"))) + })) as unknown as typeof fetch, + controller.signal, + ) + controller.abort() + + expect(await probe).toBe(false) +}) diff --git a/packages/codev-sandbox/src/boot.ts b/packages/codev-sandbox/src/boot.ts index 2055bb2ef7..de82453835 100644 --- a/packages/codev-sandbox/src/boot.ts +++ b/packages/codev-sandbox/src/boot.ts @@ -56,13 +56,27 @@ export function sandboxBootEnv( // Absolute guest path of the launcher baked into every image. export const SANDBOX_GUEST_LAUNCHER = "/opt/codev-guest/server-launch.mjs" +// A probe must not be able to outlive its usefulness. pollSandboxHealth awaits +// each attempt in turn, so one request that opens and then never answers stalls +// every later attempt forever — the sandbox never comes up, and the start +// timeout is the only thing left to end it. That happens in practice: the +// forwarder accepts the TCP connection and dials through to the guest while its +// server is still binding the port, and the connection can be left hanging +// rather than refused. A healthy guest answers in single-digit milliseconds, so +// a short deadline costs a live probe nothing and turns a permanent hang into +// one skipped attempt. +const HEALTH_REQUEST_TIMEOUT_MS = 2_000 + export async function checkSandboxHealth( url: string, credentials: SandboxCredentials, fetchImpl: typeof fetch = fetch, + signal?: AbortSignal, ) { + const deadline = AbortSignal.timeout(HEALTH_REQUEST_TIMEOUT_MS) const response = await fetchImpl(`${url}/global/health`, { headers: { authorization: `Basic ${btoa(`${credentials.username}:${credentials.password}`)}` }, + signal: signal ? AbortSignal.any([signal, deadline]) : deadline, }).catch(() => null) return !!response?.ok } diff --git a/packages/codev-sandbox/src/bwrap/provider.ts b/packages/codev-sandbox/src/bwrap/provider.ts index 676e15b31d..e5f64b97fe 100644 --- a/packages/codev-sandbox/src/bwrap/provider.ts +++ b/packages/codev-sandbox/src/bwrap/provider.ts @@ -185,7 +185,7 @@ export function createBwrapProvider(options: BwrapProviderOptions): SandboxProvi const abort = new AbortController() let timeoutHandle: ReturnType | undefined const outcome = await Promise.race([ - pollSandboxHealth(() => checkSandboxHealth(url, credentials, fetchImpl), abort.signal).then( + pollSandboxHealth(() => checkSandboxHealth(url, credentials, fetchImpl, abort.signal), abort.signal).then( () => "healthy" as const, ), exited, diff --git a/packages/codev-sandbox/src/vz/provider.ts b/packages/codev-sandbox/src/vz/provider.ts index 4a2288010b..e3cd349819 100644 --- a/packages/codev-sandbox/src/vz/provider.ts +++ b/packages/codev-sandbox/src/vz/provider.ts @@ -249,7 +249,7 @@ export function createVzProvider(options: VzProviderOptions): SandboxProvider { const url = `http://127.0.0.1:${forwarder.port}` const abort = new AbortController() const healthOutcome = await Promise.race([ - pollSandboxHealth(() => checkSandboxHealth(url, credentials, fetchImpl), abort.signal).then( + pollSandboxHealth(() => checkSandboxHealth(url, credentials, fetchImpl, abort.signal), abort.signal).then( () => "healthy" as const, ), helperError, diff --git a/packages/codev-sandbox/src/wsl/provider.ts b/packages/codev-sandbox/src/wsl/provider.ts index 2019d71ce5..f25fb2b678 100644 --- a/packages/codev-sandbox/src/wsl/provider.ts +++ b/packages/codev-sandbox/src/wsl/provider.ts @@ -386,7 +386,7 @@ export function createWslProvider(options: WslProviderOptions): SandboxProvider const abort = new AbortController() let timeoutHandle: ReturnType | undefined const outcome = await Promise.race([ - pollSandboxHealth(() => checkSandboxHealth(url, credentials, fetchImpl), abort.signal).then( + pollSandboxHealth(() => checkSandboxHealth(url, credentials, fetchImpl, abort.signal), abort.signal).then( () => "healthy" as const, ), exited,