fix: dispose pending infrastructure loads and entity contexts

This commit is contained in:
Sameh Khamis committed 2026-09-11 13:18:59 -07:00
1 parent a547e99a7b
commit e465197cee
4 files changed
+177 -2

No files matched your search

+3
View File
@@ -13,6 +13,9 @@ of current runtime behavior, see [`docs/CURRENT-STATE.md`](docs/CURRENT-STATE.md
## [Unreleased] ## [Unreleased]
- Local GeoJSON layers share concurrent loads, cancel pending fetches on destruction,
discard late results, and remove their entity-context records on teardown.
- Unchanged local infrastructure overlays no longer sustain idle rendering. - Unchanged local infrastructure overlays no longer sustain idle rendering.
Ground samples wait for visible terrain to settle and cannot place a marker Ground samples wait for visible terrain to settle and cannot place a marker
below its loaded surface; roofs and valid below-sea-level heights are retained. below its loaded surface; roofs and valid below-sea-level heights are retained.
+5
View File
@@ -2,6 +2,11 @@
## Infrastructure marker visibility ## Infrastructure marker visibility
Local GeoJSON layers coalesce concurrent enables into one load. Disabling while
loading keeps the result hidden; destruction aborts the fetch and rejects late
parse/add results. Destruction and failed setup remove owned context records,
so replacing a layer cannot retain stale entities or listeners.
Datacenters and dams retain their full datasets while limiting active marker Datacenters and dams retain their full datasets while limiting active marker
stems per layer: 80 at camera heights of 3,000 km or above, 200 from 200 km, stems per layer: 80 at camera heights of 3,000 km or above, 200 from 200 km,
and 420 below 200 km. Selection favors existing label priority and nearby and 420 below 200 km. Selection favors existing label priority and nearby
+27 -2
View File
@@ -3,6 +3,7 @@ import { governorRequestRender } from '../renderGovernor.js';
import { import {
clearSelectedEntityContextForLayer, clearSelectedEntityContextForLayer,
registerEntityContext, registerEntityContext,
removeEntityContextsForLayer,
selectEntityContext, selectEntityContext,
} from './contextStore.js'; } from './contextStore.js';
import { import {
@@ -328,6 +329,8 @@ export function createLocalGeoJsonLayer({
let _stemGeometryDirty = true; let _stemGeometryDirty = true;
let _lastVisibilityUpdate = 0; let _lastVisibilityUpdate = 0;
let _destroyed = false; let _destroyed = false;
let _loadPromise = null;
let _loadController = null;
/** /**
* Globe-LOD active set: the record ids allowed to carry a live stem right * Globe-LOD active set: the record ids allowed to carry a live stem right
* now. This bounds geometry refreshes and ground-sample work to the * now. This bounds geometry refreshes and ground-sample work to the
@@ -487,6 +490,8 @@ export function createLocalGeoJsonLayer({
// 1. Initialize data source // 1. Initialize data source
if (!_dataSource) { if (!_dataSource) {
if (!_loadPromise) _loadPromise = (async () => {
_loadController = new AbortController();
const baseColor = Cesium.Color.fromCssColorString(color); const baseColor = Cesium.Color.fromCssColorString(color);
// Fetch and parse JSON Lines (.geojsonl) into a FeatureCollection. // Fetch and parse JSON Lines (.geojsonl) into a FeatureCollection.
@@ -500,13 +505,15 @@ export function createLocalGeoJsonLayer({
// windows (before vs after the add settles) need different cleanup. // windows (before vs after the add settles) need different cleanup.
let addedToScene = false; let addedToScene = false;
try { try {
const response = await fetch(url); const response = await fetch(url, { signal: _loadController.signal });
if (_destroyed) return;
// A 404 returns an HTML body that would otherwise die in JSON.parse // A 404 returns an HTML body that would otherwise die in JSON.parse
// one line later, reported as a parse error for a missing file. // one line later, reported as a parse error for a missing file.
if (!response.ok) { if (!response.ok) {
throw new Error(`HTTP ${response.status ?? '?'}`); throw new Error(`HTTP ${response.status ?? '?'}`);
} }
const text = await response.text(); const text = await response.text();
if (_destroyed) return;
const lines = text.split('\n').filter(l => l.trim().length > 0); const lines = text.split('\n').filter(l => l.trim().length > 0);
const features = lines.map(line => JSON.parse(line)); const features = lines.map(line => JSON.parse(line));
@@ -526,6 +533,7 @@ export function createLocalGeoJsonLayer({
markerColor: baseColor, markerColor: baseColor,
}); });
if (_destroyed) return;
loaded.name = name; loaded.name = name;
loaded.show = false; loaded.show = false;
// Cesium's DataSourceCollection.add() returns a promise and only // Cesium's DataSourceCollection.add() returns a promise and only
@@ -537,6 +545,10 @@ export function createLocalGeoJsonLayer({
// of leaving it uncaught with healthy-looking stats. // of leaving it uncaught with healthy-looking stats.
await viewer.dataSources.add(loaded); await viewer.dataSources.add(loaded);
addedToScene = true; addedToScene = true;
if (_destroyed) {
viewer.dataSources.remove(loaded, true);
return;
}
// Convert parsed points into 3D stems or style polygons // Convert parsed points into 3D stems or style polygons
const entities = loaded.entities.values; const entities = loaded.entities.values;
@@ -640,7 +652,7 @@ export function createLocalGeoJsonLayer({
} catch (e) { } catch (e) {
// The dataset ships with the build, so this is a broken install, // The dataset ships with the build, so this is a broken install,
// not a blip — it has to reach the chip, not just the console. // not a blip — it has to reach the chip, not just the console.
_error = localDatasetError(e); if (!_destroyed) _error = localDatasetError(e);
// Roll the partial build back so a later enable() retries from // Roll the partial build back so a later enable() retries from
// scratch instead of inheriting a half-populated source. Only the // scratch instead of inheriting a half-populated source. Only the
// post-add window has something in the scene to remove: a failure // post-add window has something in the scene to remove: a failure
@@ -649,11 +661,14 @@ export function createLocalGeoJsonLayer({
if (addedToScene) { if (addedToScene) {
try { viewer?.dataSources?.remove(loaded, true); } catch { /* already gone */ } try { viewer?.dataSources?.remove(loaded, true); } catch { /* already gone */ }
} }
if (_destroyed) return;
removeEntityContextsForLayer(id);
_count = 0; _count = 0;
_stemRecords = []; _stemRecords = [];
console.error(`Failed to load ${id}:`, e); console.error(`Failed to load ${id}:`, e);
} }
if (_destroyed) return;
// 2. Install native global click handler // 2. Install native global click handler
if (!_clickHandler) { if (!_clickHandler) {
_clickHandler = screenSpaceEventHandlerFactory(viewer.scene.canvas); _clickHandler = screenSpaceEventHandlerFactory(viewer.scene.canvas);
@@ -699,8 +714,16 @@ export function createLocalGeoJsonLayer({
} }
}, Cesium.ScreenSpaceEventType.LEFT_CLICK); }, Cesium.ScreenSpaceEventType.LEFT_CLICK);
} }
})();
try {
await _loadPromise;
} finally {
_loadPromise = null;
_loadController = null;
}
} }
if (_destroyed) return;
// 3. Add an incredibly fast pre-render occluder to hide points behind the globe // 3. Add an incredibly fast pre-render occluder to hide points behind the globe
if (_enabled && !_preRenderRemover) { if (_enabled && !_preRenderRemover) {
_preRenderRemover = viewer.scene.preRender.addEventListener(() => { _preRenderRemover = viewer.scene.preRender.addEventListener(() => {
@@ -879,9 +902,11 @@ export function createLocalGeoJsonLayer({
destroy: (viewer) => { destroy: (viewer) => {
if (_destroyed) return; if (_destroyed) return;
_destroyed = true; _destroyed = true;
_loadController?.abort();
// Defensively disable first so listeners and selection state are // Defensively disable first so listeners and selection state are
// torn down even if destroy is called while the layer is enabled. // torn down even if destroy is called while the layer is enabled.
disableLayer(viewer); disableLayer(viewer);
removeEntityContextsForLayer(id);
if (_clickHandler) { if (_clickHandler) {
_clickHandler.destroy(); _clickHandler.destroy();
_clickHandler = null; _clickHandler = null;
+142
View File
@@ -0,0 +1,142 @@
import test from 'node:test';
import assert from 'node:assert/strict';
import * as Cesium from 'cesium';
import { createLocalGeoJsonLayer } from './localGeojson.js';
import { getContextStore } from './contextStore.js';
const dataset = JSON.stringify({
type: 'Feature', id: 'dam', properties: { name: 'Test dam' },
geometry: { type: 'Polygon', coordinates: [[[0, 0], [0.01, 0], [0, 0.01], [0, 0]]] },
});
const response = () => ({ ok: true, text: async () => dataset });
const deferred = () => {
let resolve;
const promise = new Promise(done => { resolve = done; });
return { promise, resolve };
};
function harness(t) {
const previousWindow = globalThis.window;
globalThis.window = { dispatchEvent() {} };
t.after(() => {
layer.destroy(viewer);
if (previousWindow === undefined) delete globalThis.window;
else globalThis.window = previousWindow;
});
const sources = new Set();
const listeners = new Set();
const event = { addEventListener(fn) { listeners.add(fn); return () => listeners.delete(fn); } };
let handlers = 0;
const viewer = {
dataSources: {
async add(source) { sources.add(source); return source; },
remove(source) { return sources.delete(source); },
},
scene: { canvas: {}, preRender: event, requestRender() {} },
camera: { moveEnd: event },
};
const create = () => createLocalGeoJsonLayer({
id: 'local-dams', name: 'Dams', color: '#0088ff', url: '/dams.geojsonl',
overlayHost: { setVisible() {}, setEntries() {}, clearSource() {} },
screenSpaceEventHandlerFactory() {
handlers++;
return { setInputAction() {}, destroy() { handlers--; } };
},
});
const layer = create();
return { layer, create, viewer, sources, listeners, handlers: () => handlers };
}
test('destroy removes context records and permits a fresh replacement', async t => {
const env = harness(t);
t.mock.method(globalThis, 'fetch', async () => response());
await env.layer.enable(env.viewer);
const oldEntity = [...getContextStore().entities.values()][0].entity;
assert.equal(getContextStore().entities.size, 1);
env.layer.disable(env.viewer);
assert.equal(getContextStore().entities.size, 1, 'disabled cached source retains its records');
env.layer.destroy(env.viewer);
assert.equal(getContextStore().entities.size, 0);
assert.equal(env.sources.size, 0);
assert.equal(env.listeners.size, 0);
assert.equal(env.handlers(), 0);
const replacement = env.create();
await replacement.enable(env.viewer);
assert.equal(getContextStore().entities.size, 1);
assert.notEqual([...getContextStore().entities.values()][0].entity, oldEntity);
replacement.destroy(env.viewer);
await env.layer.enable(env.viewer);
assert.equal(getContextStore().entities.size, 0);
});
for (const phase of ['fetch', 'text', 'parse', 'add']) {
test(`destroy during ${phase} leaves no late objects or context records`, async t => {
const env = harness(t);
const entered = deferred();
const release = deferred();
const pause = async value => { entered.resolve(); await release.promise; return value; };
let signal;
t.mock.method(globalThis, 'fetch', async (_url, options) => {
signal = options.signal;
if (phase === 'fetch') return pause(response());
if (phase === 'text') return { ok: true, text: () => pause(dataset) };
return response();
});
if (phase === 'parse') {
const load = Cesium.GeoJsonDataSource.load;
t.mock.method(Cesium.GeoJsonDataSource, 'load', async (...args) => pause(await load(...args)));
}
if (phase === 'add') {
t.mock.method(env.viewer.dataSources, 'add', async source => {
await pause();
env.sources.add(source);
return source;
});
}
const loading = env.layer.enable(env.viewer);
await entered.promise;
env.layer.destroy(env.viewer);
assert.equal(signal.aborted, true);
release.resolve();
await loading;
assert.equal(env.sources.size, 0);
assert.equal(env.listeners.size, 0);
assert.equal(env.handlers(), 0);
assert.equal(getContextStore().entities.size, 0);
assert.deepEqual(env.layer.getStats(), { count: 0, lastUpdate: null, error: null });
});
}
test('concurrent enable and disable/re-enable share one pending dataset load', async t => {
const env = harness(t);
const release = deferred();
const fetchMock = t.mock.method(globalThis, 'fetch', async () => {
await release.promise;
return response();
});
const first = env.layer.enable(env.viewer);
env.layer.disable(env.viewer);
const second = env.layer.enable(env.viewer);
release.resolve();
await Promise.all([first, second]);
assert.equal(fetchMock.mock.callCount(), 1);
assert.equal(env.sources.size, 1);
assert.equal(env.handlers(), 1);
assert.equal(env.listeners.size, 2);
assert.equal([...env.sources][0].show, true);
});
test('disable during loading keeps the completed source hidden until re-enabled', async t => {
const env = harness(t);
const release = deferred();
t.mock.method(globalThis, 'fetch', async () => { await release.promise; return response(); });
const loading = env.layer.enable(env.viewer);
env.layer.disable(env.viewer);
release.resolve();
await loading;
assert.equal([...env.sources][0].show, false);
assert.equal(env.listeners.size, 0);
await env.layer.enable(env.viewer);
assert.equal([...env.sources][0].show, true);
assert.equal(env.sources.size, 1);
});