From 5889d353272723a75e8b0bfaacbc2085873e15f7 Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Sun, 11 Oct 2026 09:51:10 -0400 Subject: [PATCH 1/4] fix(maps): stop delivering settled state to detached subscribers --- CHANGELOG.md | 1 + docs/CURRENT-STATE.md | 5 ++ src/maps/controller.js | 1 + src/maps/map-subscription-ownership.test.mjs | 55 ++++++++++++++++++++ 4 files changed, 62 insertions(+) create mode 100644 src/maps/map-subscription-ownership.test.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index a770fbd35..362eb93e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ - Tool string length validation counts Unicode characters correctly, including characters outside the basic multilingual plane. - Director timeline seeks cross the dateline along the short longitude arc. +- Map state updates stop delivering to subscribers removed during the same notification. - Treat explicitly current, complete empty vessel coverage as a successful update, clearing obsolete contacts while retaining stale or incomplete snapshots. diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 1f794a5b5..558963496 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -5326,3 +5326,8 @@ datacenters, dams and the Nepal locator retain that shared credit while displaye Military fragments stitch by source identity. Different mapped parcels merge only when their footprints touch or contain one another, never across a seam tolerance gap. + + +### fix(maps): stop delivering settled state to detached subscribers + +Recheck listener ownership and controller lifetime before each settled-state callback. Still-active subscribers keep their ordering and exception isolation. diff --git a/src/maps/controller.js b/src/maps/controller.js index e595ede26..d51b67082 100644 --- a/src/maps/controller.js +++ b/src/maps/controller.js @@ -142,6 +142,7 @@ export class MapSourceController { if (this._destroyed || !this._subscribers.size) return; const state = this.getState(); for (const listener of [...this._subscribers]) { + if (this._destroyed || !this._subscribers.has(listener)) continue; try { listener(state); } catch (error) { diff --git a/src/maps/map-subscription-ownership.test.mjs b/src/maps/map-subscription-ownership.test.mjs new file mode 100644 index 000000000..638de65a2 --- /dev/null +++ b/src/maps/map-subscription-ownership.test.mjs @@ -0,0 +1,55 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { MapSourceController } from './controller.js'; +function fixture() { + return new MapSourceController( + { + scene: { + globe: { show: true }, + requestRender() {}, + frameState: { + creditDisplay: { addStaticCredit() {}, removeStaticCredit() {} }, + }, + primitives: { add() {}, remove() {} }, + }, + imageryLayers: { add() {}, remove() {} }, + }, + { + registry: { + defaultId: 'fixture', + sources: [ + { + descriptor: { id: 'fixture', label: 'Fixture', kind: 'imagery' }, + imagery: async () => ({}), + }, + ], + }, + createImageryLayer: (provider) => ({ provider }), + }, + ); +} +test('an unsubscribed owner is skipped in the same map settlement delivery', async () => { + const controller = fixture(); + const calls = []; + let detach; + controller.subscribe(() => { + calls.push('first'); + detach(); + }); + detach = controller.subscribe(() => calls.push('detached')); + controller.subscribe(() => calls.push('last')); + await controller.setStack('fixture'); + assert.deepEqual(calls, ['first', 'last']); + controller.destroy(); +}); +test('destroying the controller during delivery stops remaining listeners', async () => { + const controller = fixture(); + const calls = []; + controller.subscribe(() => { + calls.push('destroy'); + controller.destroy(); + }); + controller.subscribe(() => calls.push('detached')); + await controller.setStack('fixture'); + assert.deepEqual(calls, ['destroy']); +}); From bbbdd0aeba19f9f73a0ba43db4a0a2d7c84b5b35 Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Mon, 12 Oct 2026 11:14:31 -0400 Subject: [PATCH 2/4] docs: address maintainer review for PR #1047 --- docs/CURRENT-STATE.md | 7 ++----- src/maps/controller.js | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 558963496..ef3720370 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -5200,6 +5200,8 @@ the project's MIT license; see `DATA_SOURCES.md`. ## Map source factories and coordination +A subscriber removed, or a controller destroyed, during a settled-state delivery receives no further callbacks from that delivery. + Map selection is composed from separate imagery, terrain and 3D factories. The default registry retains Google 3D, Bing Aerial/Labels, Esri Satellite and OSM, including their setup guidance and existing preset/share IDs. Esri still @@ -5326,8 +5328,3 @@ datacenters, dams and the Nepal locator retain that shared credit while displaye Military fragments stitch by source identity. Different mapped parcels merge only when their footprints touch or contain one another, never across a seam tolerance gap. - - -### fix(maps): stop delivering settled state to detached subscribers - -Recheck listener ownership and controller lifetime before each settled-state callback. Still-active subscribers keep their ordering and exception isolation. diff --git a/src/maps/controller.js b/src/maps/controller.js index d51b67082..6dd2720f7 100644 --- a/src/maps/controller.js +++ b/src/maps/controller.js @@ -142,7 +142,7 @@ export class MapSourceController { if (this._destroyed || !this._subscribers.size) return; const state = this.getState(); for (const listener of [...this._subscribers]) { - if (this._destroyed || !this._subscribers.has(listener)) continue; + if (!this._subscribers.has(listener)) continue; try { listener(state); } catch (error) { From e225f0851b63f014f40233f9e611c79af215e394 Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Wed, 14 Oct 2026 20:13:15 -0400 Subject: [PATCH 3/4] docs: separate CHANGELOG bullets after rebase Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 362eb93e7..168d5b9db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ - Tool string length validation counts Unicode characters correctly, including characters outside the basic multilingual plane. - Director timeline seeks cross the dateline along the short longitude arc. + - Map state updates stop delivering to subscribers removed during the same notification. - Treat explicitly current, complete empty vessel coverage as a successful update, From b9fa0eedcc2367a018f501688a9bd6d4e86bb956 Mon Sep 17 00:00:00 2001 From: Sameh Khamis Date: Wed, 14 Oct 2026 19:23:46 -0700 Subject: [PATCH 4/4] fix(maps): stop all delivery once the controller is destroyed A listener could destroy the controller and re-subscribe one still pending in the snapshot, which then ran. Delivery now stops at destruction and a destroyed controller takes no listeners. Adds ordering tests for adds, duplicates and re-adds during delivery, and moves the current-state note after the section intro. --- docs/CURRENT-STATE.md | 6 ++-- src/maps/controller.js | 4 ++- src/maps/map-subscription-ownership.test.mjs | 34 ++++++++++++++++++++ 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 8ecd0eae8..8474257b6 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -5209,8 +5209,6 @@ the project's MIT license; see `DATA_SOURCES.md`. ## Map source factories and coordination -A subscriber removed, or a controller destroyed, during a settled-state delivery receives no further callbacks from that delivery. - Map selection is composed from separate imagery, terrain and 3D factories. The default registry retains Google 3D, Bing Aerial/Labels, Esri Satellite and OSM, including their setup guidance and existing preset/share IDs. Esri still @@ -5218,6 +5216,10 @@ falls back to OSM on construction failure or two active tile failures, and credits follow the source actually displayed. Terrain remains lazy while the photoreal globe is hidden. +Map-stack subscribers hear each settled activation once. A subscriber removed +during a delivery, or every subscriber once the controller is destroyed, gets +no further callback from it, and a destroyed controller accepts no new ones. + Map credentials are passed to each source constructor rather than changing SDK-wide Google or ion defaults. Bing Aerial and Bing Labels resolve their ion imagery endpoint with the supplied diff --git a/src/maps/controller.js b/src/maps/controller.js index 6dd2720f7..2b2ea462f 100644 --- a/src/maps/controller.js +++ b/src/maps/controller.js @@ -131,7 +131,8 @@ export class MapSourceController { * @returns {() => void} Unsubscribe. */ subscribe(listener) { - if (typeof listener !== 'function') return () => {}; + // A destroyed controller never settles again, so it takes no listeners. + if (typeof listener !== 'function' || this._destroyed) return () => {}; this._subscribers.add(listener); return () => { this._subscribers.delete(listener); @@ -142,6 +143,7 @@ export class MapSourceController { if (this._destroyed || !this._subscribers.size) return; const state = this.getState(); for (const listener of [...this._subscribers]) { + if (this._destroyed) return; if (!this._subscribers.has(listener)) continue; try { listener(state); diff --git a/src/maps/map-subscription-ownership.test.mjs b/src/maps/map-subscription-ownership.test.mjs index 638de65a2..cf0708c09 100644 --- a/src/maps/map-subscription-ownership.test.mjs +++ b/src/maps/map-subscription-ownership.test.mjs @@ -53,3 +53,37 @@ test('destroying the controller during delivery stops remaining listeners', asyn await controller.setStack('fixture'); assert.deepEqual(calls, ['destroy']); }); + +test('a listener re-added after destroy during delivery is not called', async () => { + const controller = fixture(); + const calls = []; + const later = () => calls.push('later'); + controller.subscribe(() => { + calls.push('destroy'); + controller.destroy(); + controller.subscribe(later); + }); + controller.subscribe(later); + await controller.setStack('fixture'); + assert.deepEqual(calls, ['destroy']); +}); + +test('delivery order: added during delivery waits, duplicates run once, re-added runs', async () => { + const controller = fixture(); + const calls = []; + const twice = () => calls.push('twice'); + const readded = () => calls.push('readded'); + let offReadded; + controller.subscribe(() => { + calls.push('first'); + controller.subscribe(() => calls.push('added-late')); + offReadded(); + controller.subscribe(readded); + }); + controller.subscribe(twice); + controller.subscribe(twice); + offReadded = controller.subscribe(readded); + await controller.setStack('fixture'); + assert.deepEqual(calls, ['first', 'twice', 'readded']); + controller.destroy(); +});