Plug memory leaks during multi-model loads#1493
Merged
pablo-mayrgundter merged 2 commits intobldrs-ai:mainfrom May 1, 2026
Merged
Plug memory leaks during multi-model loads#1493pablo-mayrgundter merged 2 commits intobldrs-ai:mainfrom
pablo-mayrgundter merged 2 commits intobldrs-ai:mainfrom
Conversation
Cherry-picked from bldrs-ai#1477. Disposes the prior IfcViewerAPIExtended on each new model load (WebGL context, geometries, materials, textures, ResizeObserver hook, global mouse handlers) so GPU/heap don't grow across loads. Also cleans up theme/hash/canvas listeners, revokes object URLs (worker-aware: deferred until the worker is done with the URL), disposes IFrameCommunicationChannel ports, prunes per-note edit state on deletion, and dedupes snack-clear timeouts. Extends Markus's viewer disposal with: - WebGLRenderer.forceContextLoss() so the GL context itself is reclaimed (renderer.dispose alone leaves it allocated and browsers cap active contexts) - optional currentViewer.dispose?.() to reach subsystems the manual scene traverse misses (controls, post-processor, highlighter, isolator, viewsManager) - disposal of additional texture-map slots: normalMap, roughnessMap, metalnessMap, emissiveMap, aoMap, bumpMap, displacementMap, alphaMap, lightMap, envMap See MEMORY_LEAK_FIXES.md for the full per-leak breakdown. Co-Authored-By: Markus Steinbrecher <74647806+MarkusSteinbrecher@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for bldrs-share-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Adds focused unit tests for each leak fix from the prior commit: - src/Containers/viewer.test.js (NEW, 12 tests): verifies disposeViewer removes the same window mouse handlers it added (same fn refs), traverses the scene and disposes geometry + every texture-map slot + the material itself, calls renderer.dispose() AND forceContextLoss(), invokes the viewer's built-in dispose() when present, disconnects the ResizeObserver, nulls glbClipper, and is idempotent. Also verifies initViewer disposes the previous viewer before constructing the new one. - src/Components/Apps/AppsMessagesHandler.test.js (NEW, 7 tests): verifies the channel posts port2 with init, attaches a message handler, and dispose() nulls onmessage, closes the port, drops all references, is idempotent, and survives port.close throwing. Polyfills MessageChannel for jest-fixed-jsdom. - src/store/NotesSlice.test.js: 4 new tests covering setDeletedNotes pruning behavior — prunes editBodies/editModes/editOriginalBodies for the deleted id, leaves others untouched, and no-ops on null / legacy-shape inputs. - src/utils/loader.test.js: 3 new tests verifying URL.revokeObjectURL is called in loadLocalFile (sync fallback), loadLocalFileFallback, and saveDnDFileToOpfsFallback — and revoke happens before the callback fires. - src/Components/Camera/CameraControl.test.jsx: 1 new test verifying every canvas listener added by onLoad gets removed on unmount with the same fn ref (this was the original leak's root cause). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Cherry-pick of the memory-leak fixes from @MarkusSteinbrecher's #1477, on a focused branch (without the unrelated Google Drive / theme / floor-plan / project-hierarchy work in that PR).
The headline fix:
initViewer()was building a newIfcViewerAPIExtendedon every model load without disposing the previous one. The old WebGL context, renderer, geometries, materials, and textures all stayed alive, which is the resource creep we've seen across multi-model sessions. NowdisposeViewer()runs first, releasing GPU resources before allocating new ones.Three small extensions on top of Markus's viewer disposal:
WebGLRenderer.forceContextLoss()afterrenderer.dispose()— Three.js'sdispose()frees programs/render targets but the WebGL context itself stays allocated. Chrome caps active contexts at ~16, so without this we could still hit the cap after enough loads.currentViewer.dispose?.()— optional call to any built-in dispose onIfcViewerAPIExtended. Belt-and-suspenders against subsystems the manual scene traverse misses (controls, post-processor, highlighter, isolator, viewsManager).material.map. Now iterates all common slots (normalMap,roughnessMap,metalnessMap,emissiveMap,aoMap,bumpMap,displacementMap,alphaMap,lightMap,envMap).The other 7 leaks fixed (per Markus's
MEMORY_LEAK_FIXES.md):onModelPathre-registered without removal)useEffecthad no cleanup)removeHashListener(name)URL.createObjectURLand never revoked (4 sites). For the two sites whose URL is consumed by a Web Worker, revocation is deferred until the worker reports completion to avoid the worker fetching a dead URL.IFrameCommunicationChannelhad nodispose()— added one;AppIFrametracks the channel via a ref and disposes on unmount/recreatesetDeletedNotesleft orphaned entries ineditBodies/editModes/editOriginalBodiesSaveModelControlsnack timeouts piled up — now dedup'd via a single tracked timerTest plan
yarn lint && yarn typecheck— cleanyarn test-src— 160 suites, 958 tests, all passtheme.themeChangeListenersdoesn't growNotes
bldrs-ai/Share:main, notpablo-mayrgundter/Share:main, so the PR is a clean diff against the org's main.MEMORY_LEAK_FIXES.mdfor the full per-leak breakdown (Markus's doc, with a one-line preamble noting the cherry-pick origin and the three extensions).🤖 Generated with Claude Code