Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/calm-passes-yield.md
Original file line number Diff line number Diff line change
@@ -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
15 changes: 13 additions & 2 deletions packages/joint-router-avoid/src/RouterService.mts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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> {
Expand All @@ -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> {
Expand All @@ -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
Expand All @@ -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);
Expand Down
58 changes: 58 additions & 0 deletions packages/joint-router-avoid/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down