Skip to content

Clear IntersectionObserver reference on dispose#5831

Open
srid wants to merge 1 commit intoxtermjs:masterfrom
juspay:fix/intersection-observer-nullify
Open

Clear IntersectionObserver reference on dispose#5831
srid wants to merge 1 commit intoxtermjs:masterfrom
juspay:fix/intersection-observer-nullify

Conversation

@srid
Copy link
Copy Markdown
Contributor

@srid srid commented Apr 23, 2026

Summary

Fixes the IntersectionObserver retention path from #5820 without using WeakRef.

RenderService._registerIntersectionObserver currently creates an observer whose callback closes over this. The disposable disconnects the observer, but the disposable closure also keeps a JS reference to the observer. Since the observer owns its callback, the disposed RenderService can remain reachable through:

RenderService -> MutableDisposable -> disposable closure -> IntersectionObserver -> callback -> RenderService

Store the observer on RenderService instead, and have the disposable disconnect and null out that field. The disposable no longer captures the observer object, so after dispose there is no JS-land reference from the disposed service back to the observer/callback pair.

This is an alternative to #5821's WeakRef version.

Evidence

Kolu reproduced the original leak on the pre-#617 UI and compared three arms in Chrome DevTools MCP. Each arm used a fresh just dev, 7 terminals created/restored through the UI, and 30 Focus<->Canvas round trips via the visible header toggle.

Arm xterm variant xterm buffer retention delta / 30 toggles
A unpatched baseline, pre-#5821 Uint32Array +339, JSArrayBufferData +339
B #5821 WeakRef fix Uint32Array +0, JSArrayBufferData +0
C this observer-nullify variant Uint32Array +0, JSArrayBufferData +0

The baseline reproduced the retained xterm-buffer signature. Both WeakRef and this observer-nullify variant retained zero additional xterm buffer arrays.

Caveat: this MCP run did not reproduce the original +367 MB magnitude because the clean UI-created/restored terminals had only ~118 scrollback rows instead of the large restored buffers from the production repro. The retention signature still distinguishes the baseline from both fixes.

Test

  • npm run lint-changes

Closes #5820

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.

IntersectionObserver callback retains RenderService → BufferLines after dispose

1 participant