tower / commit
fix(terrain): a dead worker no longer freezes the world (D-080)
decisions.md | 27 ++++++++++++++++ design.md | 4 ++- src/terrain/service.ts | 83 +++++++++++++++++++++++++++++++++++++++++++++++--- src/terrain/worker.ts | 5 +++ 4 files changed, 113 insertions(+), 6 deletions(-) diff --git a/decisions.md b/decisions.md index 8097748..b961066 100644 --- a/decisions.md +++ b/decisions.md @@ -2645,3 +2645,30 @@ as if the sun took the whole beam (`direct`), so the world is exactly as bright Strictly, a sun taking 30% of the beam would light it less. That is on the backlog (design.md §5): pick the split, then the light budget follows from it. The map, which does not draw the rod, is untouched. + +## D-080 — A dead terrain worker no longer freezes the world + +**Reported** (author, 2026-09-28): now and then, over several days, the page froze on +"Teaching Eastwall Water which way is down", with no clear console errors. + +**What it was.** That label is the compile's **last** step, not a slow one: the whole compile +takes about 5 s, and that step 3 ms. After it, the page sent the compiled world to every +worker in the pool and waited for all of them, with no time limit. A worker that never +started never answered, so the wait never ended — and a comlink call to a dead worker fails +silently. The only trace was Firefox's warning "Attempting to create a Worker from an empty +source", which the author's screenshots showed a dozen times: Firefox had been handed an +empty script for a worker and started it with nothing to run. Why it sometimes gets an empty +script was not pinned down (twenty clean loads and eighteen with files changing mid-load all +worked); a page that made one worker silent on purpose reproduced the freeze exactly, on the +same label, with the same warning and nothing else. + +**What changed** (`src/terrain/service.ts`). Before a world is handed out, every worker must +answer a `ping` within 15 s. One that does not is terminated and replaced once; if the +replacement is silent too it is dropped, and the pool carries on smaller. Handing over the +world has a 30 s deadline per worker on the same terms. Each of these says so with a +`console.error` naming the worker, and a worker's own `error` event is logged too, so the +next time this happens it is visible. Only an empty pool is fatal, and it says so. + +Checked in Firefox with the same sabotage: one silent worker now costs about 15 s and a +console error; a silent worker with a silent replacement costs about 30 s and loads with five; +a normal load is unchanged. diff --git a/design.md b/design.md index 85ede38..524548f 100644 --- a/design.md +++ b/design.md @@ -714,7 +714,9 @@ puts Sisters Tarn at 2,173 m. - **Three.js** for the exterior and interior 3D views. - **Canvas 2D** for the map view. - **Web workers** for world generation (world-generation.md §5): a pool of up to six, - driven through **comlink**. Noise is **simplex-noise**, sampled in 3D on the + driven through **comlink**. Each must answer a ping before it is given the world, and + one that does not is replaced once, then dropped, so a dead worker costs seconds rather + than freezing the load (D-080). Noise is **simplex-noise**, sampled in 3D on the cylinder itself so nothing tears at θ = 0. Those two are the only runtime dependencies besides Three.js. - **GitLab CI → GitLab Pages**, publishing `dist/` as `static/`. diff --git a/src/terrain/service.ts b/src/terrain/service.ts index 01c7b47..c3c5b0d 100644 --- a/src/terrain/service.ts +++ b/src/terrain/service.ts @@ -46,10 +46,30 @@ export interface Progress { type Listener = () => void interface Slot { + worker: Worker api: Remote<WorkerApi> inFlight: number } +/** + * How long a worker gets to load its code and answer a ping, and to take a compiled world. + * Generous: in dev a worker loads its modules one by one, and a slow machine is not a dead + * worker. A dead one is not slow — it never answers at all. + */ +const WORKER_START_MS = 15_000 +const WORKER_LOAD_MS = 30_000 + +class Timeout extends Error {} + +/** A promise that rejects if `p` has not settled in `ms`. */ +function within<T>(p: Promise<T>, ms: number, what: string): Promise<T> { + let timer: ReturnType<typeof setTimeout> + const late = new Promise<never>((_, reject) => { + timer = setTimeout(() => reject(new Timeout(`${what}: no answer in ${ms / 1000} s`)), ms) + }) + return Promise.race([p, late]).finally(() => clearTimeout(timer)) +} + export class TerrainService { private slots: Slot[] private signature = '' @@ -90,13 +110,52 @@ export class TerrainService { */ const cap = Number(params.get('workers')) const size = Math.max(1, Math.min(Number.isFinite(cap) && cap > 0 ? cap : 6, cores - 1)) - this.slots = Array.from({ length: size }, () => ({ - api: wrap<WorkerApi>(new Worker(this.workerUrl, { type: 'module' })), - inFlight: 0, - })) + this.slots = Array.from({ length: size }, (_, i) => this.spawn(i)) this.caching = !import.meta.env.DEV || params.get('cache') === '1' } + private spawn(index: number): Slot { + const worker = new Worker(this.workerUrl, { type: 'module' }) + // A worker whose script fails to load or throws says so here and nowhere else. + worker.addEventListener('error', (e) => { + console.error(`Terrain worker ${index} failed:`, e.message || e) + }) + return { worker, api: wrap<WorkerApi>(worker), inFlight: 0 } + } + + /** + * Make sure every worker in the pool is alive, before the world is handed to them. + * + * The page used to wait on all of them with no limit, so one worker that never started + * froze the world on the compile's last step — "Teaching Eastwall Water which way is + * down" — with nothing in the console but Firefox's "Attempting to create a Worker from + * an empty source" (D-080). Now a worker that does not answer a ping is replaced once, + * and dropped if its replacement does not answer either. The pool is smaller for it, + * but the world loads; an empty pool is the one case that is an error. + */ + private async ensureWorkers(): Promise<void> { + const alive = await Promise.all(this.slots.map(async (slot, i) => { + try { + await within(slot.api.ping(), WORKER_START_MS, `Terrain worker ${i}`) + return slot + } catch (e) { + console.error(`${(e as Error).message} — replacing it.`) + slot.worker.terminate() + const fresh = this.spawn(i) + try { + await within(fresh.api.ping(), WORKER_START_MS, `Terrain worker ${i} (replacement)`) + return fresh + } catch (e2) { + console.error(`${(e2 as Error).message} — carrying on without it.`) + fresh.worker.terminate() + return null + } + } + })) + this.slots = alive.filter((s): s is Slot => s !== null) + if (!this.slots.length) throw new Error('No terrain worker started; the world cannot be made.') + } + get workers(): number { return this.slots.length } @@ -169,6 +228,7 @@ export class TerrainService { let done = 0 this.setProgress({ busy: true, label: 'Reading the plans', done: 0, total, startedAt: performance.now() }) + await this.ensureWorkers() const key = cacheKey(`${this.workerUrl.href}\n${terrainSignature(world)}`) let compiled = this.caching ? await loadCompiled(key) : null this.lastFromCache = compiled !== null @@ -179,7 +239,20 @@ export class TerrainService { if (this.caching) void saveCompiled(key, compiled) } - await Promise.all(this.slots.map((s) => s.api.load(compiled, gen))) + // Each worker gets the world with a deadline; one that misses it leaves the pool rather + // than holding up the rest (D-080). + const loaded = await Promise.all(this.slots.map(async (s, i) => { + try { + await within(s.api.load(compiled, gen), WORKER_LOAD_MS, `Terrain worker ${i} taking the world`) + return s + } catch (e) { + console.error(`${(e as Error).message} — carrying on without it.`) + s.worker.terminate() + return null + } + })) + this.slots = loaded.filter((s): s is Slot => s !== null) + if (!this.slots.length) throw new Error('No terrain worker took the world; it cannot be shown.') this.compiled = compiled this.runtime = prepare(compiled) for (const fn of this.worldListeners) fn() diff --git a/src/terrain/worker.ts b/src/terrain/worker.ts index 2cd71ae..f22a4f5 100644 --- a/src/terrain/worker.ts +++ b/src/terrain/worker.ts @@ -16,6 +16,11 @@ let runtime: Runtime | null = null let generation = -1 const api = { + /** Answers once the worker's code has loaded: the pool's check that it is alive. */ + ping(): true { + return true + }, + compile(world: World, onStep: (label: string) => void): CompiledWorld { return compileWorld(world, onStep) },