diff --git a/.changeset/calm-passes-yield.md b/.changeset/calm-passes-yield.md new file mode 100644 index 0000000000..a8c20f1110 --- /dev/null +++ b/.changeset/calm-passes-yield.md @@ -0,0 +1,5 @@ +--- +"@joint/router-avoid": patch +--- + +RouterService - a `routeAll()`/`routeSubgraph()` pass still queued when `start()` is called now resolves as `cancelled` instead of running after `start()`'s full sync and resetting the engine to the pass's subset under the live graph listener - a crash path, since the listener referencing an element the engine no longer held aborted the WASM module irrecoverably diff --git a/packages/joint-router-avoid/src/RouterService.mts b/packages/joint-router-avoid/src/RouterService.mts index 17580326d5..bde64e131d 100644 --- a/packages/joint-router-avoid/src/RouterService.mts +++ b/packages/joint-router-avoid/src/RouterService.mts @@ -335,6 +335,7 @@ export class RouterService { * normal teardown does not surface as an unhandled promise rejection. * Unexpected errors are re-thrown and stay unhandled - there is no * caller to propagate them to. + * */ private backgroundSync(): void { this.sync(this.graph.getCells()).catch((error) => { @@ -373,7 +374,7 @@ export class RouterService { * graph's current cells. Unlike {@link start}, this attaches no graph * listener - nothing keeps the graph routed as it changes afterwards. * - * @returns The {@link RoutingResult} of the pass. + * @returns The {@link RoutingResult} of the pass - `{ status: 'cancelled' }` if {@link start} or {@link destroy} is called before the queued pass runs. * @throws If the router is currently started (see {@link isStarted}) - call {@link stop} first. */ routeAll(): Promise { @@ -395,7 +396,7 @@ export class RouterService { * graph listener. * * @param cells - The cells to route; only the elements and links in this array are considered. - * @returns The {@link RoutingResult} of the pass. + * @returns The {@link RoutingResult} of the pass - `{ status: 'cancelled' }` if {@link start} or {@link destroy} is called before the queued pass runs. * @throws If the router is currently started (see {@link isStarted}) - call {@link stop} first. */ routeSubgraph(cells: dia.Cell[]): Promise { @@ -412,6 +413,9 @@ export class RouterService { * provider - resolves with `{ status: 'cancelled' }` instead of * rejecting, so fire-and-forget callers are not left with unhandled * rejections; provider errors unrelated to destruction still reject. + * A pass superseded by {@link start} while still queued resolves with + * `{ status: 'cancelled' }` too - `start()`'s own full-graph sync + * replaces whatever the pass would have produced. * * Passes run strictly one after another: a pass invoked while another * is still in flight waits for it, since each pass replaces the @@ -428,6 +432,13 @@ export class RouterService { private async performRoute(cells: dia.Cell[]): Promise { if (this.destroyed) return { status: 'cancelled' }; + // Superseded by `start()`: the pass was queued while stopped, but by + // the time it runs a graph listener is live and `start()`'s own full + // sync has already replaced the engine's content. Running this pass + // now would reset the engine to just `cells` under the live + // listener - and the listener then referencing an element the + // engine no longer holds aborts the WASM module irrecoverably. + if (this.isStarted) return { status: 'cancelled' }; try { await this.sync(cells); diff --git a/packages/joint-router-avoid/test/index.js b/packages/joint-router-avoid/test/index.js index faf6ee3cec..0a51dd4db5 100644 --- a/packages/joint-router-avoid/test/index.js +++ b/packages/joint-router-avoid/test/index.js @@ -655,6 +655,64 @@ QUnit.module('destroy()', () => { }); }); +QUnit.module('start() during an in-flight one-shot pass', () => { + // Regression: a queued `routeSubgraph()` pass used to run AFTER + // `start()`'s full-graph sync and replaced the engine's content with + // only the subgraph - leaving the live graph listener working against + // an engine that no longer held the rest of the graph (a listener + // referencing an element the engine does not hold aborts the WASM + // module irrecoverably). The queued pass is now superseded: it resolves + // as cancelled and the engine keeps holding the full graph. + QUnit.test('a queued routeSubgraph() pass is superseded by start(), leaving the engine holding the full graph', async assert => { + const graph = new joint.dia.Graph(); + const size = { width: 100, height: 100 }; + const a = new joint.shapes.standard.Rectangle({ position: { x: 0, y: 0 }, size }); + const b = new joint.shapes.standard.Rectangle({ position: { x: 300, y: 0 }, size }); + const linkAB = new joint.shapes.standard.Link({ source: { id: a.id }, target: { id: b.id }}); + const c = new joint.shapes.standard.Rectangle({ position: { x: 0, y: 400 }, size }); + const d = new joint.shapes.standard.Rectangle({ position: { x: 300, y: 400 }, size }); + const linkCD = new joint.shapes.standard.Link({ source: { id: c.id }, target: { id: d.id }}); + graph.resetCells([a, b, linkAB, c, d, linkCD]); + + const routerService = await joint.routers.avoid.initAvoidRouter(graph, {}); + + const subgraphPass = routerService.routeSubgraph([a, b, linkAB]); + routerService.start(); + const result = await subgraphPass; + assert.equal(result.status, 'cancelled', 'the queued one-shot pass reports it was superseded'); + + const routedLinks = []; + routerService.on('link:routed', (l) => routedLinks.push(l)); + + c.position(0, 800); + + assert.ok(routedLinks.includes(linkCD), 'a link outside the one-shot subset is still routed while started'); + assert.ok(isOrthogonalPath(linkCD), 'and its route is a real avoid route'); + + routerService.destroy(); + }); + + QUnit.test('start() with no pass in flight still applies routes synchronously (main thread)', async assert => { + const graph = new joint.dia.Graph(); + const size = { width: 100, height: 100 }; + const source = new joint.shapes.standard.Rectangle({ position: { x: 0, y: 0 }, size }); + const target = new joint.shapes.standard.Rectangle({ position: { x: 300, y: 0 }, size }); + const link = new joint.shapes.standard.Link({ source: { id: source.id }, target: { id: target.id }}); + graph.resetCells([source, target, link]); + + const routerService = await joint.routers.avoid.initAvoidRouter(graph, {}); + + const routedLinks = []; + routerService.on('link:routed', (l) => routedLinks.push(l)); + + routerService.start(); + + assert.equal(routedLinks.length, 1, 'the initial sync ran during start(), not a microtask later'); + + routerService.destroy(); + }); +}); + QUnit.module('idle after incremental changes (main thread)', () => { // `MainThreadProvider` routes synchronously, so `idle` (driven by the // provider's `processed`) must fire during the originating graph change,