From cb258f32c60aa1500ecdf4d3cc8fbaf19fd97f17 Mon Sep 17 00:00:00 2001
From: Nils Henrik Hals <3185998+Strepto@users.noreply.github.com>
Date: Wed, 8 May 2024 10:43:31 +0200
Subject: [PATCH] fix: Disposed geometryGroup softlocks sector load (#3946)

* fix: Disposed geometryGroup softlocks sector load

* Check dispose before dereference

* Update lockfile

* Refactor
---
 viewer/packages/cad-parsers/package.json      | 35 ++++++++++---------
 .../cad-parsers/src/sector/SectorNode.ts      | 21 ++++++++---
 .../utilities/src/three/AutoDisposeGroup.ts   |  4 +++
 viewer/yarn.lock                              |  1 +
 4 files changed, 39 insertions(+), 22 deletions(-)

diff --git a/viewer/packages/cad-parsers/package.json b/viewer/packages/cad-parsers/package.json
index a289bdf0c09..0002b8a4809 100644
--- a/viewer/packages/cad-parsers/package.json
+++ b/viewer/packages/cad-parsers/package.json
@@ -1,17 +1,18 @@
-{
-  "name": "@reveal/cad-parsers",
-  "private": true,
-  "main": "index.ts",
-  "scripts": {
-    "start": "yarn ws:start",
-    "test": "yarn ws:test --config ./../../jest.config.js",
-    "lint": "yarn ws:lint"
-  },
-  "dependencies": {
-    "@reveal/data-providers": "workspace:*",
-    "@reveal/metrics": "workspace:*",
-    "@reveal/model-base": "workspace:*",
-    "@reveal/sector-parser": "workspace:*",
-    "@reveal/utilities": "workspace:*"
-  }
-}
+{
+  "name": "@reveal/cad-parsers",
+  "private": true,
+  "main": "index.ts",
+  "scripts": {
+    "start": "yarn ws:start",
+    "test": "yarn ws:test --config ./../../jest.config.js",
+    "lint": "yarn ws:lint"
+  },
+  "dependencies": {
+    "@reveal/data-providers": "workspace:*",
+    "@reveal/logger": "workspace:*",
+    "@reveal/metrics": "workspace:*",
+    "@reveal/model-base": "workspace:*",
+    "@reveal/sector-parser": "workspace:*",
+    "@reveal/utilities": "workspace:*"
+  }
+}
diff --git a/viewer/packages/cad-parsers/src/sector/SectorNode.ts b/viewer/packages/cad-parsers/src/sector/SectorNode.ts
index 3d02ae11a18..83342e6ec78 100644
--- a/viewer/packages/cad-parsers/src/sector/SectorNode.ts
+++ b/viewer/packages/cad-parsers/src/sector/SectorNode.ts
@@ -5,6 +5,7 @@
 import * as THREE from 'three';
 import { AutoDisposeGroup } from '@reveal/utilities';
 import { LevelOfDetail } from '../cad/LevelOfDetail';
+import { Log } from '@reveal/logger';
 
 export class SectorNode extends THREE.Group {
   public readonly sectorPath: string;
@@ -37,12 +38,18 @@ export class SectorNode extends THREE.Group {
     return this._updatedTimestamp;
   }
 
-  updateGeometry(geomtryGroup: AutoDisposeGroup | undefined, levelOfDetail: LevelOfDetail): void {
+  updateGeometry(geometryGroup: AutoDisposeGroup | undefined, levelOfDetail: LevelOfDetail): void {
     this.resetGeometry();
-    this._group = geomtryGroup;
-    if (this._group !== undefined) {
-      this._group.reference();
+
+    if (geometryGroup) {
+      if (geometryGroup.isDisposed()) {
+        Log.warn('Tried to add an already disposed geometry group to sector:', this.sectorId);
+      } else {
+        geometryGroup.reference();
+      }
     }
+
+    this._group = geometryGroup;
     this._lod = levelOfDetail;
     this._updatedTimestamp = Date.now();
     this.visible = this._lod !== LevelOfDetail.Discarded;
@@ -57,7 +64,11 @@ export class SectorNode extends THREE.Group {
 
   resetGeometry(): void {
     if (this._group !== undefined) {
-      this._group.dereference();
+      if (!this._group.isDisposed()) {
+        this._group.dereference();
+      } else {
+        Log.warn('Tried to dereference an already disposed geometryGroup in sector:', this.sectorId);
+      }
       this.remove(this._group);
     }
 
diff --git a/viewer/packages/utilities/src/three/AutoDisposeGroup.ts b/viewer/packages/utilities/src/three/AutoDisposeGroup.ts
index 46d7712a7a4..ec7e28d1743 100644
--- a/viewer/packages/utilities/src/three/AutoDisposeGroup.ts
+++ b/viewer/packages/utilities/src/three/AutoDisposeGroup.ts
@@ -15,6 +15,10 @@ export class AutoDisposeGroup extends THREE.Group {
   private _isDisposed = false;
   private _referenceCount = 0;
 
+  isDisposed(): boolean {
+    return this._isDisposed;
+  }
+
   reference(): void {
     this.ensureNotDisposed();
     this._referenceCount++;
diff --git a/viewer/yarn.lock b/viewer/yarn.lock
index e9a31e30cd9..4f02160c90b 100644
--- a/viewer/yarn.lock
+++ b/viewer/yarn.lock
@@ -1240,6 +1240,7 @@ __metadata:
   resolution: "@reveal/cad-parsers@workspace:packages/cad-parsers"
   dependencies:
     "@reveal/data-providers": "workspace:*"
+    "@reveal/logger": "workspace:*"
     "@reveal/metrics": "workspace:*"
     "@reveal/model-base": "workspace:*"
     "@reveal/sector-parser": "workspace:*"