Visitar URL original
Clipper and Outliner throw once their world has been disposed, which aborts `components.dispose()` · Issue #839 · ThatOpen/engine_components · GitHub
Skip to content

Clipper and Outliner throw once their world has been disposed, which aborts components.dispose() #839

Open

Description

@Chaosnico9000

Package: @thatopen/components 3.6.0 and @thatopen/components-front 3.6.0 (unchanged on main @ ad540db), Clipper and Outliner

A world can be disposed on its own while the Components instance lives on: Worlds.delete(world) does it, and so do the UI elements: bim-world when its components is set to null, and bim-world-2d on disconnectedCallback. After that, world.renderer is null. #756 made Components.dispose() hold worlds back so that components clean up while their world still stands, and SimplePlane.dispose() already guards the renderer. Two components still assume the renderer is there when the world went first:

1. Clipper: deleting a plane whose world is gone throws

Worlds.delete() leaves the world's planes in clipper.list. The list's onBeforeDelete handler throws Renderer not found for this plane's world! for such a plane. That handler runs for delete(), deleteAll() and for the list.clear() in Clipper.dispose().

2. Outliner: cleaning up after its world is gone throws

Outliner.dispose() and clean() go through detachAll(), which only checks !this.world and then calls getRenderer(). That reads renderer.postproduction on a null renderer and throws a TypeError. remove() has the same check. Other paths in the Outliner (wake(), the enabled accessors) already check world.isDisposing.

Consequence

Both throws happen inside components.dispose(). The loop has no guard, so the error aborts it: the remaining components, the worlds, FragmentsManager and the timer are not disposed, and components.onDisposed never fires.

Reproduction

We first hit both in a browser app (Chromium) on an earlier 3.4.x, where the viewer disposed its world before calling components.dispose(). The results below are from 3.6.0, checked without a browser: the script runs the published 3.6.0 bundles in bun with only the renderer, camera and canvas stubbed, since those need WebGL.

@thatopen/components 3.6.0
@thatopen/components-front 3.6.0
A1 Clipper, components.dispose() only: ok
   planes left in clipper.list after worlds.delete: 1
A2 Clipper, worlds.delete(world) then clipper.deleteAll(): THROWS Error: Renderer not found for this plane's world!
A3 Clipper, worlds.delete(world) then components.dispose(): THROWS Error: Renderer not found for this plane's world!
   components.onDisposed fired: false
No world found while setting up events!
B1 Outliner, components.dispose() only: ok
B2 Outliner, worlds.delete(world) then outliner.clean(): THROWS TypeError: null is not an object (evaluating 'e.postproduction')
B3 Outliner, worlds.delete(world) then components.dispose(): THROWS TypeError: null is not an object (evaluating 'e.postproduction')

(The TypeError wording is JavaScriptCore's; Chromium reports it as Cannot read properties of null (reading 'postproduction'). No world found while setting up events! is an unrelated warning printed when the front components are first created.)

Script (bun)
// GPU-free check of Clipper / Outliner teardown after a world is deleted.
// Uses the published @thatopen/components(-front) 3.6.0 bundles; only the
// renderer, camera and canvas are stubbed, since they need WebGL / a DOM.
import * as THREE from "three";
import * as OBC from "@thatopen/components";
import * as OBCF from "@thatopen/components-front";

function fakeElement(): any {
  const el: any = {
    style: {},
    addEventListener() {},
    removeEventListener() {},
    getBoundingClientRect: () => ({ left: 0, top: 0, width: 100, height: 100 }),
    setPointerCapture() {},
    releasePointerCapture() {},
    parentElement: null,
  };
  el.ownerDocument = { pointerLockElement: null };
  return el;
}

function stubRenderer(): any {
  const r: any = {
    worlds: new Map(),
    onWorldChanged: new OBC.Event(),
    currentWorld: null,
    setPlane: () => {},
    updateClippingPlanes: () => {},
    dispose: () => {},
    three: { domElement: fakeElement() },
    postproduction: {
      excludedObjectsPass: { addExcludedMaterial() {} },
      outlinePass: {
        addGroup() {},
        configureGroup() {},
        removeGroup() {},
        detachOutlinedTile() {},
      },
      outlinesEnabled: false,
    },
  };
  return r;
}

function stubCamera(): any {
  return {
    three: new THREE.PerspectiveCamera(),
    onWorldChanged: new OBC.Event(),
    isDisposeable: () => true,
    isUpdateable: () => false,
    dispose: () => {},
    controls: { addEventListener() {}, removeEventListener() {} },
  };
}

function setup() {
  const components = new OBC.Components();
  const worlds = components.get(OBC.Worlds);
  const world = worlds.create<OBC.SimpleScene, any, any>();
  world.scene = new OBC.SimpleScene(components);
  world.camera = stubCamera();
  world.renderer = stubRenderer();
  return { components, worlds, world };
}

async function run(name: string, fn: () => void | Promise<void>) {
  try {
    await fn();
    console.log(`${name}: ok`);
  } catch (e) {
    console.log(`${name}: THROWS ${(e as Error).constructor.name}: ${(e as Error).message}`);
  }
}

console.log("@thatopen/components", (await import("@thatopen/components/package.json")).version);
console.log("@thatopen/components-front", (await import("@thatopen/components-front/package.json")).version);

// Clipper
{
  const { components } = setup();
  const world = [...components.get(OBC.Worlds).list.values()][0];
  const clipper = components.get(OBC.Clipper);
  clipper.createFromNormalAndCoplanarPoint(world, new THREE.Vector3(0, -1, 0), new THREE.Vector3());
  await run("A1 Clipper, components.dispose() only", () => components.dispose());
}
{
  const { components, worlds, world } = setup();
  const clipper = components.get(OBC.Clipper);
  clipper.createFromNormalAndCoplanarPoint(world, new THREE.Vector3(0, -1, 0), new THREE.Vector3());
  worlds.delete(world);
  console.log("   planes left in clipper.list after worlds.delete:", clipper.list.size);
  await run("A2 Clipper, worlds.delete(world) then clipper.deleteAll()", () => clipper.deleteAll());
}
{
  const { components, worlds, world } = setup();
  const clipper = components.get(OBC.Clipper);
  clipper.createFromNormalAndCoplanarPoint(world, new THREE.Vector3(0, -1, 0), new THREE.Vector3());
  let componentsDisposed = false;
  components.onDisposed.add(() => (componentsDisposed = true));
  worlds.delete(world);
  await run("A3 Clipper, worlds.delete(world) then components.dispose()", () => components.dispose());
  console.log("   components.onDisposed fired:", componentsDisposed);
}

// Outliner
{
  const { components, world } = setup();
  const outliner = components.get(OBCF.Outliner);
  outliner.world = world;
  await run("B1 Outliner, components.dispose() only", () => components.dispose());
}
{
  const { components, worlds, world } = setup();
  const outliner = components.get(OBCF.Outliner);
  outliner.world = world;
  worlds.delete(world);
  await run("B2 Outliner, worlds.delete(world) then outliner.clean()", () => outliner.clean());
  await run("B3 Outliner, worlds.delete(world) then components.dispose()", () => components.dispose());
}

Minimal app steps:

  1. Create a world with a PostproductionRenderer, create a clipping plane with clipper.createFromNormalAndCoplanarPoint(world, normal, point) and set outliner.world = world.
  2. Call components.get(OBC.Worlds).delete(world) (or dispose a bim-world).
  3. Call components.dispose(). It throws at the Clipper or the Outliner, whichever is registered first.

Expected

Disposing a world first does not break later cleanup: clipper.deleteAll(), outliner.clean() and components.dispose() finish without throwing.

Suggested fix

Treat a disposed world like a missing one, the same way SimplePlane.dispose() and the Outliner's wake() already do:

// Clipper.setEvents(), onBeforeDelete: SimplePlane.dispose() already skips the
// renderer when it is gone, so the throw is not needed
this.list.onBeforeDelete.add(({ key, value: plane }) => {
  this.onBeforeDelete.trigger();
  this._deleting.set(key, plane);
  plane.world.renderer?.setPlane(false, plane.three);
  plane.dispose();
});

// Outliner.detachAll()
if (!this.world || this.world.isDisposing) {
  state.attached = new Map();
  return;
}

// Outliner.remove()
if (this.world && !this.world.isDisposing) {
  this.getRenderer().postproduction.outlinePass.removeGroup(name);
}

With these three changes applied to copies of the published 3.6.0 bundles, the same script prints ok for A1 to A3 and B1 to B3, and components.onDisposed fires. Optionally, the Clipper could also listen to worlds.list.onItemDeleted and drop the planes of a deleted world, so they don't linger in clipper.list (see planes left … : 1 above).

Related: #518 (closed; the same throw via components.dispose() alone, which #756 fixed by disposing worlds last).

Environment

  • @thatopen/components 3.6.0, @thatopen/components-front 3.6.0, three 0.184.0
  • Earlier observation in Chromium; the trace above was produced with bun 1.4.4
  • Package manager: bun

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions