From 227e3c2051a5c7be9119794357fffe1a9cfd51a4 Mon Sep 17 00:00:00 2001 From: German Khokhlov Date: Tue, 3 Nov 2026 22:56:24 +0400 Subject: [PATCH] Serialize shell attach and detach so recovery waits for a failed attachment's cleanup --- src/app/shellPart.js | 2 +- src/ui/applicationShell.js | 21 +++++++---- src/ui/shellSceneSwap.test.mjs | 65 ++++++++++++++++++++++++++++++---- 3 files changed, 73 insertions(+), 15 deletions(-) diff --git a/src/app/shellPart.js b/src/app/shellPart.js index bfa4cc86a..210f75e1e 100644 --- a/src/app/shellPart.js +++ b/src/app/shellPart.js @@ -18,7 +18,7 @@ export function shellScenePart(styleManager, { builtOn, catalog }) { return; } first = null; - styleManager.attachScene({ + return styleManager.attachScene({ viewer: next.viewer, engine: next.engine, mapStackController: next.mapStackController, diff --git a/src/ui/applicationShell.js b/src/ui/applicationShell.js index c7af4bfc2..d7bf883f7 100644 --- a/src/ui/applicationShell.js +++ b/src/ui/applicationShell.js @@ -702,7 +702,17 @@ export class StyleManager extends ShellFacade { * stay; the parts bound to a globe are built for this one and the * address's view is restored on it. */ - attachScene({ + attachScene(options = {}) { + return this._queueSceneTransition(() => this._attachScene(options)); + } + + _queueSceneTransition(work) { + const run = (this._sceneTransition ?? Promise.resolve()).then(work, work); + this._sceneTransition = run.catch(() => {}); + return run; + } + + async _attachScene({ viewer = null, engine = engineFromViewer(viewer), mapStackController = null, @@ -710,6 +720,7 @@ export class StyleManager extends ShellFacade { engineSwitch = true, } = {}) { if (this._disposed) return; + if (this._scene.engine === engine && this._scene.viewer === viewer) return; if (this._scene.engine) throw new Error('The shell is attached to a globe already'); // An attachment that fails part-way releases what it built: the shell @@ -770,7 +781,7 @@ export class StyleManager extends ShellFacade { }); } } catch (error) { - void this.detachScene().catch((cleanupError) => + await this._detachScene().catch((cleanupError) => console.error( 'The shell could not leave a failed globe:', cleanupError, @@ -787,11 +798,7 @@ export class StyleManager extends ShellFacade { * Context mode for the next globe. */ detachScene(options = {}) { - // One detach at a time: a second request joins the one under way. - this._sceneDetach ??= this._detachScene(options).finally(() => { - this._sceneDetach = null; - }); - return this._sceneDetach; + return this._queueSceneTransition(() => this._detachScene(options)); } async _detachScene({ diff --git a/src/ui/shellSceneSwap.test.mjs b/src/ui/shellSceneSwap.test.mjs index 40ece5654..675b3ccd8 100644 --- a/src/ui/shellSceneSwap.test.mjs +++ b/src/ui/shellSceneSwap.test.mjs @@ -142,7 +142,7 @@ test('the next globe gets new globe-bound parts and the restore runs on it', asy await shell.detachScene({ handoff: true }); log.length = 0; const engine = { kind: 'navara' }; - shell.attachScene({ + await shell.attachScene({ viewer: null, engine, mapStackController: {}, @@ -172,14 +172,18 @@ test('the next globe gets new globe-bound parts and the restore runs on it', asy '_syncCctvPanelViewport', ]); assert.equal(shell._navigation, session.navigation); - assert.throws(() => shell.attachScene({ engine }), /attached to a globe/); + await shell.attachScene({ engine }); + await assert.rejects( + shell.attachScene({ engine: { kind: 'cesium' } }), + /attached to a globe/, + ); }); test('a panel whose layer is another instance in the new catalog is rebuilt', async () => { const { shell, log } = fakeShell(); await shell.detachScene({ handoff: true }); log.length = 0; - shell.attachScene({ + await shell.attachScene({ engine: { kind: 'cesium' }, services: { cctvLayer: 'other' }, }); @@ -202,8 +206,8 @@ test('an attachment that throws part-way leaves the shell detached and attachabl shell._initSceneOverlays = () => { throw new Error('overlay host failed'); }; - assert.throws( - () => shell.attachScene({ engine: { kind: 'navara' } }), + await assert.rejects( + shell.attachScene({ engine: { kind: 'navara' } }), /overlay host failed/, ); // The shell releases what it built and is detached when that settles. @@ -212,7 +216,7 @@ test('an attachment that throws part-way leaves the shell detached and attachabl log.length = 0; shell._initSceneOverlays = () => log.push('_initSceneOverlays'); const engine = { kind: 'cesium' }; - shell.attachScene({ engine }); + await shell.attachScene({ engine }); assert.equal(shell.engine, engine); assert.ok(log.includes('shareRestoration.start')); }); @@ -238,8 +242,55 @@ test('between engines a style key waits for the next engine instead of the relea shell._layerBindings = { observeCamera() {}, stop() {}, disconnect() {} }; shell._visualSettings = { setStyle: (name) => applied.push(name) }; }; - shell.attachScene({ engine: { kind: 'cesium' } }); + await shell.attachScene({ engine: { kind: 'cesium' } }); await Promise.resolve(); await Promise.resolve(); assert.deepEqual(applied, ['crt']); }); + +test('shell recovery waits for failed attachment cleanup and serializes repeated detach', async () => { + const { shell, session } = fakeShell(); + await shell.detachScene({ handoff: true }); + let release; + const pending = new Promise((resolve) => { + release = resolve; + }); + session.context.canRetainScene = () => false; + session.context.restoreForDisposal = () => pending; + shell._initSceneOverlays = () => { + throw new Error('overlay failed'); + }; + const failed = shell.attachScene({ engine: { kind: 'navara' } }); + const rejected = assert.rejects(failed, /overlay failed/); + await Promise.resolve(); + const detach = shell.detachScene(); + const again = shell.detachScene(); + shell._initSceneOverlays = () => {}; + const engine = { kind: 'cesium' }; + const recovered = shell.attachScene({ engine }); + await Promise.resolve(); + assert.notEqual(shell.engine, engine, 'recovery waits for cleanup'); + release(); + await Promise.all([rejected, detach, again, recovered]); + assert.equal(shell.engine, engine); + await shell.attachScene({ engine }); + assert.equal(shell.engine, engine, 'same attachment is idempotent'); +}); + +test('shell attach waits for a deferred outgoing detach', async () => { + const { shell, session } = fakeShell(); + let release; + session.context.canRetainScene = () => false; + session.context.restoreForDisposal = () => + new Promise((resolve) => { + release = resolve; + }); + const leaving = shell.detachScene({ handoff: true }); + const engine = { kind: 'navara' }; + const entering = shell.attachScene({ engine }); + await Promise.resolve(); + assert.notEqual(shell.engine, engine); + release(); + await Promise.all([leaving, entering]); + assert.equal(shell.engine, engine); +});