Skip to content

fix(inspector): release animation and texture state on teardown - #136

Merged
chiefcll merged 1 commit into
mainfrom
fix/inspector-leaks-solidtv
Aug 4, 2026
Merged

fix(inspector): release animation and texture state on teardown#136
chiefcll merged 1 commit into
mainfrom
fix/inspector-leaks-solidtv

Conversation

@chiefcll

@chiefcll chiefcll commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Two retention paths in Inspector.ts, both only reachable with the inspector enabled.

Originally found while reviewing lightning-js/renderer#846. Of the four issues in that review, this repo has already fixed two — destroyNode(node) uses the mirror div stored on the node instead of a document-global getElementById, and the parent branch of updateNodeProperty already falls back with parentNode.div ?? this.root. The two below are still present.

1. activeAnimations never releases animations killed by destroy

Inspector.activeAnimations is a static Map holding the IAnimationController. Entries are only removed by trackAnimationEnd, which runs on stop / restore / waitUntilStopped or the controller's stopped event. A node destroyed mid-animation fires none of those, so the entry and the controller it holds are retained by a static root for the lifetime of the page.

Destroy now ends those animations as cancelled rather than deleting them outright, so getAnimationStats() and animationHistory stay consistent — restore() already records cancelled for the same situation.

2. textureMetrics pinned every texture ever used

It was a strong Map<Texture, TextureMetrics> on an object that lives as long as the renderer. setupTextureListeners detaches the old texture's listeners on every swap but never dropped its metrics entry, and destroy only cleaned the texture a node happened to hold at that moment. Every texture ever swapped off a node stayed reachable, bitmap included.

Now a WeakMap. Nothing iterates it — only has / get / set / delete — so the API is unchanged.

Testing

pnpm test green (360 tests, 42 files), tsc --noEmit clean, prettier clean, no new eslint warnings (the two reported on Inspector.ts are on main too).

No regression test here: this repo has no DOM test environment, and the inspector needs one to instantiate. The equivalent test — asserting getActiveAnimations() no longer lists a destroyed node — is in lightning-js/renderer#847, where happy-dom is already a devDependency. Happy to add happy-dom here and port it over if that's wanted.

🤖 Generated with Claude Code

Two retention paths, both only reachable with the inspector enabled:

- Animations still running when their node is destroyed never emit
  `stopped`, so their entry — and the IAnimationController it holds —
  stayed in the static `activeAnimations` map for the lifetime of the
  page. Destroy now ends them as `cancelled`, which also keeps
  `getAnimationStats()` consistent instead of silently dropping them.

- `textureMetrics` was a strong `Map<Texture, TextureMetrics>` on an
  object that lives as long as the renderer, and entries were only
  dropped for the texture a node happened to hold at destroy time.
  `setupTextureListeners` detaches the old texture's listeners on every
  swap but never dropped its metrics entry, so every texture ever
  swapped off a node stayed reachable, bitmap included. `WeakMap`
  instead — nothing iterates it, so the API is unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@chiefcll
chiefcll merged commit 77fa91c into main Aug 4, 2026
3 checks passed
@chiefcll
chiefcll deleted the fix/inspector-leaks-solidtv branch August 4, 2026 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant