From d9aa2bd06d308a41a07fb47e2acc8d45c5e40961 Mon Sep 17 00:00:00 2001 From: Sameh Khamis Date: Fri, 16 Oct 2026 16:53:42 -0700 Subject: [PATCH] Release attempt timers on destroy and the replaced fallback terrain destroy() now expires every retry attempt in flight, clearing its deadline timer and aborting its signal at once rather than when the attempt settles, and leaves no retry marked as running. A successful terrain retry overwrote the cached fallback without releasing it. The replaced fallback is now disposed as soon as the real terrain takes its place on screen, or, when it stayed installed behind a 3D tileset, once the next globe activation replaces it. --- src/maps/controller.js | 35 ++++++++-- src/maps/controller.test.mjs | 126 +++++++++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+), 5 deletions(-) diff --git a/src/maps/controller.js b/src/maps/controller.js index 9fb2d8f22..bfdd82033 100644 --- a/src/maps/controller.js +++ b/src/maps/controller.js @@ -66,6 +66,10 @@ export class MapSourceController { this._retrying = false; this._retryRequested = false; this._retryDelay = FALLBACK_RETRY_MS; + /** Expire callbacks of retry attempts in flight; destroy() runs them. */ + this._retryDeadlines = new Set(); + /** A fallback terrain a retry replaced while it stayed installed. */ + this._replacedTerrain = null; this._subscribers = new Set(); this._destroyed = false; } @@ -326,6 +330,10 @@ export class MapSourceController { if (result.terrain) this.viewer.scene.setTerrain(result.terrain); else this.viewer.terrainProvider = result.provider; this._terrainFallback = result.fallback ? result : null; + if (this._replacedTerrain && this._replacedTerrain !== result) { + this._dispose(this._replacedTerrain.provider); + this._replacedTerrain = null; + } } /** @@ -361,15 +369,20 @@ export class MapSourceController { // requests have no timeout, so a late result is released, never used. const attempt = async (create, release) => { const expiry = new AbortController(); - let timer; + let expire; + const deadline = new Promise((resolve) => { + expire = () => { + clearTimeout(timer); + resolve(expiry); + }; + }); + const timer = setTimeout(expire, FALLBACK_ATTEMPT_MS); + this._retryDeadlines.add(expire); const pending = Promise.resolve().then(() => create?.({ signal: AbortSignal.any([this._abort.signal, expiry.signal]), }), ); - const deadline = new Promise((resolve) => { - timer = setTimeout(resolve, FALLBACK_ATTEMPT_MS, expiry); - }); try { const result = await Promise.race([pending, deadline]); if (result !== expiry) return result; @@ -380,6 +393,7 @@ export class MapSourceController { return null; // Still unavailable: keep the fallback. } finally { clearTimeout(timer); + this._retryDeadlines.delete(expire); } }; // Each recovers as soon as it loads, never waiting on the other. @@ -426,8 +440,14 @@ export class MapSourceController { this._terrainProviders.set(terrain.id, Promise.resolve(real)); if (this._terrainFallback === flat && this.viewer.scene.globe.show) { this._applyTerrain(real); + this._dispose(flat.provider); this._requestRender('map-stack'); - } else this._terrainMode = null; // The next globe activation installs it. + } else { + this._terrainMode = null; // The next globe activation installs it. + // Release the replaced fallback once nothing shows it. + if (this._terrainFallback === flat) this._replacedTerrain = flat; + else this._dispose(flat.provider); + } return true; }; const settled = (retry) => @@ -559,6 +579,11 @@ export class MapSourceController { this._switchGen++; this._abort.abort(); clearTimeout(this._retryTimer); + for (const expire of this._retryDeadlines) expire(); + this._retryDeadlines.clear(); + this._retrying = false; + this._dispose(this._replacedTerrain?.provider); + this._replacedTerrain = null; this._isSwitching = false; this._removeImageryLayer(); this._credits.destroy(); diff --git a/src/maps/controller.test.mjs b/src/maps/controller.test.mjs index 1ff2ceac1..f7e764754 100644 --- a/src/maps/controller.test.mjs +++ b/src/maps/controller.test.mjs @@ -1256,3 +1256,129 @@ test('destroy releases recovered imagery and terrain that lands afterwards', asy assert.equal(real.destroyed, true); assert.ok(!installed.includes(real)); }); + +function liveTimers(t) { + const live = new Set(); + const { setTimeout: set, clearTimeout: clear } = globalThis; + globalThis.setTimeout = (fn, ms, ...args) => { + const id = set( + (...values) => { + live.delete(id); + fn(...values); + }, + ms, + ...args, + ); + live.add(id); + return id; + }; + globalThis.clearTimeout = (id) => { + live.delete(id); + clear(id); + }; + t.after(() => + Object.assign(globalThis, { setTimeout: set, clearTimeout: clear }), + ); + return live; +} + +test('destroy during a hung retry attempt clears its deadline at once', async (t) => { + t.mock.timers.enable({ apis: ['setTimeout'] }); + const live = liveTimers(t); + const env = publicFixture(); + const esri = failingEsri(env); + await env.controller.setStack('esri-imagery'); + esri.next = new Promise(() => {}); + t.mock.timers.tick(30_000); + await settle(); + assert.equal(esri.attempts, 2, 'the attempt hangs'); + assert.equal(live.size, 1, 'only its deadline is armed'); + env.controller.destroy(); + assert.equal(live.size, 0, 'destroy clears the deadline'); + assert.equal(env.controller._retrying, false); + await settle(); + assert.equal(live.size, 0, 'and nothing re-arms'); +}); + +function destroyableFlat(n) { + const provider = { id: `flat-${n}` }; + provider.destroy = () => (provider.destroyed = true); + return { provider, fallback: true }; +} + +test('terrain recovered on screen releases the fallback it replaced', async (t) => { + const real = { id: 'real' }; + let flat; + const { env, installed } = terrainFixture(t, async (n) => + n === 1 ? (flat = destroyableFlat(n)) : { provider: real }, + ); + await env.controller.setStack('esri-imagery'); + t.mock.timers.tick(30_000); + await settle(); + assert.equal(installed.at(-1), real); + assert.equal(flat.provider.destroyed, true); + env.controller.destroy(); +}); + +test('terrain recovered behind a tileset releases the fallback once a globe replaces it', async (t) => { + const real = { id: 'real' }; + const gate = deferred(); + let flat; + const { env, installed } = terrainFixture(t, async (n) => { + if (n === 1) return (flat = destroyableFlat(n)); + await gate.promise; + return { provider: real }; + }); + await env.controller.setStack('esri-imagery'); + t.mock.timers.tick(30_000); + await settle(); + await env.controller.setStack('photoreal'); + gate.resolve(); + await settle(); + assert.equal(installed.at(-1), flat.provider); + assert.ok(!flat.provider.destroyed, 'still installed behind the tileset'); + await env.controller.setStack('osm'); + assert.equal(installed.at(-1), real); + assert.equal(flat.provider.destroyed, true); + env.controller.destroy(); +}); + +test('a hung imagery attempt that lands after its deadline is released, never cached or shown', async (t) => { + t.mock.timers.enable({ apis: ['setTimeout'] }); + const env = publicFixture(); + const hung = deferred(); + const late = { id: 'late', errorEvent: event() }; + const good = { id: 'good', errorEvent: event() }; + for (const provider of [late, good]) + provider.destroy = () => (provider.destroyed = true); + let attempts = 0; + env.registry.sources.find( + (source) => source.descriptor.id === 'esri-imagery', + ).imagery = async () => { + if (++attempts === 1) throw new Error('Esri unreachable'); + if (attempts === 2) { + await hung.promise; + return late; + } + return good; + }; + await env.controller.setStack('esri-imagery'); + t.mock.timers.tick(30_000); + await settle(); + t.mock.timers.tick(60_000); // The deadline. + await settle(); + hung.resolve(); + await settle(); + assert.equal(late.destroyed, true, 'the late provider is released'); + assert.equal(env.controller.getActiveId(), 'osm', 'and not shown'); + t.mock.timers.tick(60_000); + await settle(); + assert.equal(attempts, 3); + assert.equal(env.imagery.at(-1).provider, good, 'the next retry installs'); + await env.controller.setStack('osm'); + await env.controller.setStack('esri-imagery'); + assert.equal(env.imagery.at(-1).provider, good, 'only its own is cached'); + env.controller.destroy(); + await settle(); + assert.equal(good.destroyed, true); +});