Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions src/browser/services/RenderService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Comment on lines +141 to 147
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised this isn't handled by observer.disconnect() dropping the refs to the callback. Is there a way to solve this without the introduction of WeakRef?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me this smells like a browser bug and might be better addressed in chrome directly. Should also be tested, whether it is reproducible on other browser engines.

Copy link
Copy Markdown
Member

@jerch jerch Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked the w3c spec - disconnect does indeed not nullify the callback, in fact the observer object holds onto the callback to be reusable with new .observe(some_element) calls. So this is indeed specced like this.

Would it help to just throw away the observer object itself for the GC to clean the callback up?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As in observer = undefined?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yepp (and to make sure, the ref gets not bound elsewhere).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be preferable to a WeakRef, every time I've thought I needed it there was something simpler like this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed this to help too.

So, we can close this PR and choose #5831 instead.

}
Expand Down
Loading