From 0964ea4796692da5695ba6cbd78c72003a6ed08f Mon Sep 17 00:00:00 2001 From: Sridhar Ratnakumar Date: Fri, 17 Apr 2026 19:15:08 -0400 Subject: [PATCH] Hold this via WeakRef in IntersectionObserver callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The IntersectionObserver callback in RenderService closes over `this` directly. On dispose, `_observerDisposable` calls `observer.disconnect()` — which in theory releases the callback. In practice, across 30 mount/unmount cycles of 7 terminals in a multi-tile app, the retainer chain Window.IntersectionObserver (registry) -> callback closure -> RenderService (this) -> _coreService -> _bufferService -> buffers -> lines -> BufferLine -> Uint32Array kept 175,594 BufferLine Uint32Arrays alive (+220 MB of native ArrayBuffer bytes) past dispose. Whether this is a Chrome DevTools wrapper, an extension patching window.IntersectionObserver, or something subtler in the native registry — either way, `disconnect()` on our side wasn't enough to free the callback in that environment. Wrap `this` in a WeakRef so the callback holds only a weak reference. Functional behavior is preserved: while RenderService is alive, `deref()` returns it and the handler runs. Once no strong refs remain, the callback becomes a no-op and the BufferService graph is free to GC — which is what the outer `_observerDisposable` was supposed to guarantee but doesn't in practice. Refs #5820 --- src/browser/services/RenderService.ts | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/browser/services/RenderService.ts b/src/browser/services/RenderService.ts index 83e3d33796..54a5117c45 100644 --- a/src/browser/services/RenderService.ts +++ b/src/browser/services/RenderService.ts @@ -124,9 +124,25 @@ export class RenderService extends Disposable implements IRenderService { private _registerIntersectionObserver(w: Window & typeof globalThis, screenElement: HTMLElement): void { // Detect whether IntersectionObserver is detected and enable renderer pause - // and resume based on terminal visibility if so + // and resume based on terminal visibility if so. + // + // The callback closes over a WeakRef to `this` rather than a strong + // reference. Even though `_observerDisposable` calls `observer.disconnect()` + // on disposal, in practice some browser-side (or devtools/extension-side) + // registries retain the callback past disconnect — the retained closure + // then keeps `this` (RenderService) alive, which transitively keeps + // `_coreService → _bufferService → buffers → BufferLines → Uint32Array` + // alive. Each leaked terminal was pinning ~1.3 KB × scrollback-lines of + // cell data, producing hundreds of MB of retained native buffer bytes + // across mode-toggle churn. WeakRef lets the RenderService (and hence + // its service graph and BufferLines) GC even if the callback itself + // is retained. if ('IntersectionObserver' in w) { - const observer = new w.IntersectionObserver(e => this._handleIntersectionChange(e[e.length - 1]), { threshold: 0 }); + const weakSelf = new WeakRef(this); + const observer = new w.IntersectionObserver( + e => weakSelf.deref()?._handleIntersectionChange(e[e.length - 1]), + { threshold: 0 } + ); observer.observe(screenElement); this._observerDisposable.value = toDisposable(() => observer.disconnect()); }