fix(joint-router-avoid): cancel a queued one-shot pass superseded by start() (#3490)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Arthur Khakhlou <arthur.khokhlov@gmail.com>

Author: 3 people

.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

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<RoutingResult> {
@@ -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<RoutingResult> {
@@ -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<RoutingResult> {
         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);

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,