From 7fb11507d5642690063f56e5201b76bcf946fd93 Mon Sep 17 00:00:00 2001 From: German Khokhlov Date: Tue, 3 Nov 2026 15:23:20 +0400 Subject: [PATCH] Bound the wait for layer attachments, park enables and explicit refreshes until the next scene binds --- src/data/lifecycle.js | 150 +++++++++++++++++++++++++++--- src/data/lifecycleScene.test.mjs | 155 +++++++++++++++++++++++++++++++ 2 files changed, 290 insertions(+), 15 deletions(-) diff --git a/src/data/lifecycle.js b/src/data/lifecycle.js index 4a2d7db63..636a4fac5 100644 --- a/src/data/lifecycle.js +++ b/src/data/lifecycle.js @@ -15,6 +15,9 @@ function cloneLayerParams(value) { } const SUPERSEDED_VISIBILITY_INTENT = Symbol('superseded-visibility-intent'); +// A scene bind or detach waits this long for asynchronous layer attachments. +// A late attachment sees its scene signal aborted and releases its own work. +const ATTACH_SETTLE_TIMEOUT_MS = 5000; const VALID_LAYER_SERIALIZATION_DISPOSITIONS = new Set([ 'enabled-only', 'enabled+options', @@ -83,6 +86,8 @@ export class LayerLifecycle { * allowQaRegistration?: boolean, getSourceAvailability?: Function }} [options] * `engine` (default: the viewer's registered engine): every module call * gets `{ signal, engine }` as its second argument. + * `attachSettleTimeoutMs`: how long a scene bind or detach waits for + * asynchronous layer attachments before it continues without them. */ constructor( viewer, @@ -91,6 +96,7 @@ export class LayerLifecycle { getSourceAvailability, engine, visibilityTarget = null, + attachSettleTimeoutMs = ATTACH_SETTLE_TIMEOUT_MS, } = {}, ) { this._catalogSourceAvailability = getSourceAvailability; @@ -113,6 +119,11 @@ export class LayerLifecycle { this._sceneController = null; this._sceneDetached = false; this._pendingAttachments = new Set(); + this._attachSettleTimeoutMs = attachSettleTimeoutMs; + // Lifecycle work and explicit refreshes that wait for the next scene. + // detachScene() does not wait for parked work, which waits for it. + this._sceneWaiters = new Set(); + this._parkListeners = new Set(); } /** The second argument of every layer module call: `{ signal, engine }`. */ @@ -136,7 +147,8 @@ export class LayerLifecycle { * always left unbound, so the next bindCatalog() can attach a scene. The * failures are reported together. A layer whose release failed keeps it * pending: a repeated detachScene(), or the next bindCatalog(), retries it. - * Periodic refreshes due while no scene is bound wait for the next one. + * Periodic refreshes, explicit refreshes and enables requested while no + * scene is bound wait for the next one. */ async detachScene() { // Attachments still in flight stop at their next checkpoint; their @@ -148,10 +160,10 @@ export class LayerLifecycle { const errors = []; const droppedQaLayers = []; try { - await Promise.allSettled([...this._pendingAttachments]); + await this._awaitAttachments([...this._pendingAttachments], 'detach'); for (const [id, entry] of [...this.layers]) { try { - await entry.toggleChain.catch(() => {}); + await this._lifecycleIdleOrParked(entry); if (entry.module.attachRenderer && entry.module.detachRenderer) { await this._detachEntryRenderer(entry, viewer); } else { @@ -206,7 +218,6 @@ export class LayerLifecycle { this._catalogSourceAvailability = catalog.getSourceAvailability; const scene = new AbortController(); this._sceneController = scene; - this._sceneDetached = false; // An attachment that is still running when the scene detaches sees this // signal aborted and must stop touching the scene. const context = { ...this._moduleContext(scene.signal), ...options }; @@ -231,10 +242,97 @@ export class LayerLifecycle { } this.finalizeRegistrations(catalog.metadata); const pending = attachments.filter((ready) => ready?.then); - const settled = Promise.allSettled(pending).then(() => { - if (this._sceneController === scene) this._runDeferredRefreshes(); + // The scene counts as bound for refreshes and parked enables once its + // attachments settled (or the wait for them timed out). + const bound = () => { + if (this._sceneController !== scene) return; + this._sceneDetached = false; + this._runDeferredRefreshes(); + this._settleSceneWaiters(true); + }; + if (!pending.length) { + bound(); + return undefined; + } + return this._awaitAttachments(pending, 'bind').then((settled) => { + bound(); + const failure = settled?.find(({ status }) => status === 'rejected'); + if (failure) throw failure.reason; + }); + } + + /** + * Wait for attachments to settle, at most attachSettleTimeoutMs. Returns + * their allSettled results, or null when the wait timed out. + */ + async _awaitAttachments(attachments, phase) { + if (!attachments.length) return []; + let timer; + const timedOut = new Promise((resolve) => { + timer = setTimeout(resolve, this._attachSettleTimeoutMs, null); + }); + const settled = await Promise.race([ + Promise.allSettled(attachments), + timedOut, + ]); + clearTimeout(timer); + if (!settled) + console.warn( + `[Data] layer attachments still running after ${this._attachSettleTimeoutMs} ms; scene ${phase} continues without them`, + ); + return settled; + } + + /** + * Wait until a scene is bound. Resolves true then, or false when the + * signal aborts or the entry is destroyed. `refresh` marks an explicit + * refresh, which replaces the entry's deferred periodic one. + */ + _waitForScene(entry, signal = null, { refresh = false } = {}) { + if (!this._sceneDetached) return Promise.resolve(true); + if (signal?.aborted) return Promise.resolve(false); + return new Promise((resolve) => { + const onAbort = () => waiter.settle(false); + const waiter = { + entry, + refresh, + settle: (bound) => { + if (!this._sceneWaiters.delete(waiter)) return; + if (refresh) entry.sceneRefreshWaiters -= 1; + signal?.removeEventListener?.('abort', onAbort); + resolve(bound); + }, + }; + if (refresh) entry.sceneRefreshWaiters += 1; + this._sceneWaiters.add(waiter); + signal?.addEventListener?.('abort', onAbort, { once: true }); + for (const listener of [...this._parkListeners]) listener(entry); + }); + } + + _settleSceneWaiters(bound, entry = null) { + for (const waiter of [...this._sceneWaiters]) + if (!entry || waiter.entry === entry) waiter.settle(bound); + } + + /** The entry's queued lifecycle work settled, or is parked for a scene. */ + _lifecycleIdleOrParked(entry) { + const parked = () => + [...this._sceneWaiters].some( + (waiter) => waiter.entry === entry && !waiter.refresh, + ); + if (parked()) return Promise.resolve(); + return new Promise((resolve) => { + const done = () => { + this._parkListeners.delete(listener); + resolve(); + }; + const listener = (parkedEntry) => { + if (parkedEntry === entry && parked()) done(); + }; + this._parkListeners.add(listener); + entry.toggleChain.catch(() => {}).then(done); }); - if (pending.length) return settled.then(() => Promise.all(pending)); } /** @@ -272,6 +370,8 @@ export class LayerLifecycle { for (const [layerId, entry] of this.layers) { if (!entry.refreshDeferred) continue; entry.refreshDeferred = false; + // An explicit refresh waiting for this scene runs instead. + if (entry.sceneRefreshWaiters > 0) continue; void this._runPeriodicUpdate(layerId, entry); } } @@ -359,6 +459,7 @@ export class LayerLifecycle { sceneDetached: false, pendingSceneDetach: null, refreshDeferred: false, + sceneRefreshWaiters: 0, }); } @@ -467,6 +568,7 @@ export class LayerLifecycle { signal?.aborted ) return false; + entry.refreshDeferred = false; const refreshEpoch = ++entry.refreshEpoch; entry.refreshing = true; this._publishActivity({ type: 'status' }); @@ -568,14 +670,19 @@ export class LayerLifecycle { */ async refreshLayer(layerId, { signal = null } = {}) { const entry = this.layers.get(layerId); - if ( - !entry || - !entry.enabled || - entry.lifecycleState !== 'enabled' || - entry.destroying || - signal?.aborted - ) - return false; + const refreshable = () => + this.layers.get(layerId) === entry && + entry.enabled && + entry.lifecycleState === 'enabled' && + !entry.destroying && + !signal?.aborted; + if (!entry || !refreshable()) return false; + // Between two scenes (an engine switch) the refresh runs once the next + // scene is bound, so callers never read the switch as a failed refresh. + if (this._sceneDetached) { + const bound = await this._waitForScene(entry, signal, { refresh: true }); + if (!bound || !refreshable()) return false; + } if (entry.refreshing) { const settled = await new Promise((resolve) => { @@ -614,6 +721,10 @@ export class LayerLifecycle { return false; } + if (this._sceneDetached) { + const bound = await this._waitForScene(entry, signal, { refresh: true }); + if (!bound || !refreshable()) return false; + } return this._runPeriodicUpdate(layerId, entry, { signal }); } @@ -1189,6 +1300,13 @@ export class LayerLifecycle { return false; }; signal?.addEventListener('abort', cancelEnable, { once: true }); + if (this._sceneDetached) { + // Between two scenes (an engine switch): init and enable get the + // next scene instead of a null viewer and engine. + this._setVisibilityIntentPhase(entry, intentEpoch, 'scene'); + if (!(await this._waitForScene(entry, signal))) + return finishCancelledEnable('scene'); + } if (!entry.initialized) { this._setVisibilityIntentPhase(entry, intentEpoch, 'init'); try { @@ -2371,6 +2489,8 @@ export class LayerLifecycle { if (!entry || entry.destroying) return false; cancelCameraArrival(this.viewer); entry.destroying = true; + // Work parked for the next scene ends now; teardown waits for its queue. + this._settleSceneWaiters(false, entry); this._invalidateRefresh(layerId, entry, 'layer-destroyed'); // Teardown becomes authoritative before the first await. Advancing the // intent epoch and aborting active work prevents a pending enable/update diff --git a/src/data/lifecycleScene.test.mjs b/src/data/lifecycleScene.test.mjs index 30771e4c1..40dbb00dc 100644 --- a/src/data/lifecycleScene.test.mjs +++ b/src/data/lifecycleScene.test.mjs @@ -51,8 +51,10 @@ const firstViewer = { name: 'first viewer' }; const secondViewer = { name: 'second viewer' }; async function boundManager(...layers) { + const options = layers.at(-1)?.module ? {} : layers.pop(); const manager = new LayerLifecycle(firstViewer, { engine: engine('first'), + ...options, }); const modules = layers.map(({ module }) => module); await manager.bindCatalog(firstViewer, engine('first'), catalog(...modules)); @@ -302,3 +304,156 @@ test('QA layers are reported when a scene switch drops them', async () => { assert.equal(manager.layers.has('qa-probe'), false); assert.ok(warnings.some((line) => line.includes('qa-probe'))); }); + +const updatesOf = (calls) => calls.filter((call) => call[0] === 'update'); +const tick = () => new Promise((resolve) => setImmediate(resolve)); + +test('a detach does not wait forever for an attachment that never settles', async () => { + let attachSignal; + const stuck = sceneLayer('stuck', { + attach(viewer, { signal }) { + if (viewer !== secondViewer) return undefined; + attachSignal = signal; + return new Promise(() => {}); + }, + }); + const { manager, modules } = await boundManager(stuck, { + attachSettleTimeoutMs: 20, + }); + await manager.detachScene(); + const bound = quietly(() => + manager.bindCatalog(secondViewer, engine('second'), catalog(...modules)), + ); + await bound; + assert.equal(manager.viewer, secondViewer, 'the bind went on without it'); + await quietly(() => manager.detachScene()); + assert.equal(attachSignal.aborted, true, 'the late attachment was told'); + assert.deepEqual(detaches(stuck.calls).at(-1), ['detach', secondViewer]); +}); + +test('a refresh due while attachments run is deferred once, not run twice', async () => { + let finishAttach; + const feed = sceneLayer('feed', { + attach(viewer) { + if (viewer !== secondViewer) return undefined; + return new Promise((resolve) => { + finishAttach = resolve; + }); + }, + }); + const { manager, modules } = await boundManager(feed); + const entry = manager.layers.get('feed'); + const before = updatesOf(feed.calls).length; + await manager.detachScene(); + const bound = manager.bindCatalog( + secondViewer, + engine('second'), + catalog(...modules), + ); + // An interval tick while the next scene is still attaching. + assert.equal(await manager._runPeriodicUpdate('feed', entry), false); + assert.equal(updatesOf(feed.calls).length, before); + finishAttach(); + await bound; + await tick(); + assert.equal(updatesOf(feed.calls).length, before + 1); + assert.equal(entry.refreshDeferred, false); +}); + +test('a normal refresh clears a deferred one', async () => { + const feed = sceneLayer('feed'); + const { manager, modules } = await boundManager(feed); + const entry = manager.layers.get('feed'); + await manager.detachScene(); + await manager._runPeriodicUpdate('feed', entry); + assert.equal(entry.refreshDeferred, true); + manager._sceneDetached = false; + await manager._runPeriodicUpdate('feed', entry); + assert.equal(entry.refreshDeferred, false); + manager._sceneDetached = true; + const before = updatesOf(feed.calls).length; + await manager.bindCatalog( + secondViewer, + engine('second'), + catalog(...modules), + ); + await tick(); + assert.equal(updatesOf(feed.calls).length, before); +}); + +test('an explicit refresh between scenes waits for the next one and runs once', async () => { + const feed = sceneLayer('feed'); + const { manager, modules } = await boundManager(feed); + const entry = manager.layers.get('feed'); + await manager.detachScene(); + await manager._runPeriodicUpdate('feed', entry); // a deferred interval tick + const before = updatesOf(feed.calls).length; + const refreshed = manager.refreshLayer('feed'); + await tick(); + assert.equal(updatesOf(feed.calls).length, before); + await manager.bindCatalog( + secondViewer, + engine('second'), + catalog(...modules), + ); + assert.equal(await refreshed, true); + await tick(); + assert.deepEqual(updatesOf(feed.calls).slice(before), [ + ['update', secondViewer], + ]); +}); + +test('an explicit refresh between scenes ends false when its caller aborts', async () => { + const feed = sceneLayer('feed'); + const { manager } = await boundManager(feed); + await manager.detachScene(); + const caller = new AbortController(); + const refreshed = manager.refreshLayer('feed', { signal: caller.signal }); + caller.abort(); + assert.equal(await refreshed, false); +}); + +test('a layer enabled between scenes initializes on the next scene', async () => { + const late = sceneLayer('late'); + const present = sceneLayer('present'); + const { manager, modules } = await boundManager(present); + const all = [...modules, late.module]; + // Registered with the first catalog, never enabled there. + await manager.detachScene(); + manager.register(late.module); + const enabled = manager.setEnabled('late', true, { origin: 'user' }); + await tick(); + assert.deepEqual(late.calls, [], 'no init against a missing scene'); + await manager.bindCatalog(secondViewer, engine('second'), catalog(...all)); + assert.equal(await enabled, true); + assert.deepEqual(late.calls.slice(0, 2), ['init', ['attach', secondViewer]]); +}); + +test('a detach does not deadlock on an enable parked for the next scene', async () => { + let finishAttach; + const slow = sceneLayer('slow', { + attach(viewer) { + if (viewer !== secondViewer) return undefined; + return new Promise((resolve) => { + finishAttach = resolve; + }); + }, + }); + const other = sceneLayer('other'); + const manager = new LayerLifecycle(firstViewer, { engine: engine('first') }); + const modules = [slow.module, other.module]; + await manager.bindCatalog(firstViewer, engine('first'), catalog(...modules)); + await manager.setEnabled('slow', true, { origin: 'user' }); + await manager.detachScene(); + void manager.bindCatalog(secondViewer, engine('second'), catalog(...modules)); + // Enabled while the second scene is still attaching: parked. + const enabled = manager.setEnabled('other', true, { origin: 'user' }); + await tick(); + const detached = manager.detachScene(); + finishAttach(); + await detached; + assert.equal(manager.viewer, null); + // Destroying the parked layer ends its enable instead of hanging. + assert.equal(await manager.destroyLayer('other'), true); + assert.equal(await enabled, false); +});