Bound the wait for layer attachments, park enables and explicit refreshes until the next scene binds

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

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);
+});