Skip to content
Open
Show file tree
Hide file tree
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
6 changes: 6 additions & 0 deletions .changeset/iframe-memory-leak.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@posthog/rrweb': patch
'@posthog/rrweb-snapshot': patch
---

fix(record): release iframe documents and observers on iframe removal — same-origin iframes mounted and unmounted while session recording is active no longer leak their `Document`, every node serialized into the mirror, or one `MutationObserver` per mount. Closes five retainer chains: load-listener disposers, named pagehide handlers, the `recordCrossOriginIframes` cleanup gate (now applied to same-origin too), captured `Document` / `Window` sets that survive `iframe.src` swap-to-`about:blank` before removal, and the global `mutationBuffers[]` / `handlers[]` arrays which previously accumulated forever. Validated end-to-end: a host page that mounts/unmounts 5 blob-URL iframes every 2s for 110s went from +118 MB / +390 leaked `HTMLDocument`s to ~0 MB / 0.
98 changes: 73 additions & 25 deletions packages/rrweb/rrweb-snapshot/src/snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -339,54 +339,88 @@ export function needMaskingText(
return false;
}

// Returns a disposer that removes the load listener and clears any pending
// timer — call it if the iframe is detached before the listener fires.
// https://stackoverflow.com/a/36155560
function onceIframeLoaded(
iframeEl: HTMLIFrameElement,
listener: () => unknown,
iframeLoadTimeout: number,
) {
): () => void {
const noop = () => {};
const win = iframeEl.contentWindow;
if (!win) {
return;
return noop;
}
// document is loading
let fired = false;

let readyState: DocumentReadyState;
try {
readyState = win.document.readyState;
} catch (error) {
return;
return noop;
}

if (readyState !== 'complete') {
const timer = setTimeout(() => {
if (!fired) {
listener();
fired = true;
let fired = false;
let timer: ReturnType<typeof setTimeout> | null = null;
const onLoad = () => {
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.

Note

🤖 Automated comment by QA Swarm — not written by a human

[xp] 🟡 MEDIUM

The "still loading" branch's onLoad calls iframeEl.removeEventListener('load', onLoad) after firing, so subsequent loads no longer re-fire the listener. Compare to the "complete" branch right below this, which carefully wires onSubsequentLoad precisely to keep firing on later loads.

This asymmetry is either:

  • Intentional — then please leave a comment explaining why a still-loading iframe doesn't deserve subsequent-load handling, or
  • An oversight — the PR description says "Accumulate every contentDocument across loads", which strongly suggests this branch should also keep the listener attached after the first load (just behind the fired guard) so an iframe.src swap while the iframe is still loading the initial document gets its second contentDocument tracked too.

The latter feels more consistent with the rest of the diff's intent.

if (fired) return;
fired = true;
if (timer !== null) {
clearTimeout(timer);
timer = null;
}
}, iframeLoadTimeout);
iframeEl.addEventListener('load', () => {
clearTimeout(timer);
iframeEl.removeEventListener('load', onLoad);
listener();
};
timer = setTimeout(() => {
if (fired) return;
fired = true;
timer = null;
iframeEl.removeEventListener('load', onLoad);
listener();
});
return;
}, iframeLoadTimeout);
iframeEl.addEventListener('load', onLoad);
return () => {
if (fired) return;
fired = true;
if (timer !== null) {
clearTimeout(timer);
timer = null;
}
iframeEl.removeEventListener('load', onLoad);
};
}
// check blank frame for Chrome

// readyState === 'complete' but Chrome reports about:blank during the
// initial transition to a non-blank src; serializing now would emit the
// blank doc and re-emit when the real load completes.
const blankUrl = 'about:blank';
let winLocationHref: string;
try {
winLocationHref = win.location.href;
} catch {
return noop;
}
const onSubsequentLoad = () => listener();
if (
win.location.href !== blankUrl ||
winLocationHref !== blankUrl ||
iframeEl.src === blankUrl ||
iframeEl.src === ''
) {
// iframe was already loaded, make sure we wait to trigger the listener
// till _after_ the mutation that found this iframe has had time to process
setTimeout(listener, 0);

return iframeEl.addEventListener('load', listener); // keep listing for future loads
// Genuinely loaded — fire on the next tick so the host mutation that
// surfaced this iframe has time to commit; also re-fire on later loads.
const initialTimer = setTimeout(listener, 0);
iframeEl.addEventListener('load', onSubsequentLoad);
return () => {
clearTimeout(initialTimer);
iframeEl.removeEventListener('load', onSubsequentLoad);
};
}
// use default listener
iframeEl.addEventListener('load', listener);
// Transient blank during navigation — wait for the real load.
iframeEl.addEventListener('load', onSubsequentLoad);
return () => {
iframeEl.removeEventListener('load', onSubsequentLoad);
};
}

function onceStylesheetLoaded(
Expand Down Expand Up @@ -1023,6 +1057,10 @@ export function serializeNodeWithId(
node: serializedElementNodeWithId,
) => unknown;
iframeLoadTimeout?: number;
onIframeListenerRegistered?: (
iframeNode: HTMLIFrameElement,
disposer: () => void,
) => void;
onStylesheetLoad?: (
linkNode: HTMLLinkElement,
node: serializedElementNodeWithId,
Expand Down Expand Up @@ -1052,6 +1090,7 @@ export function serializeNodeWithId(
onSerialize,
onIframeLoad,
iframeLoadTimeout = 5000,
onIframeListenerRegistered,
onStylesheetLoad,
stylesheetLoadTimeout = 5000,
keepIframeSrcFn = () => false,
Expand Down Expand Up @@ -1179,6 +1218,7 @@ export function serializeNodeWithId(
onSerialize,
onIframeLoad,
iframeLoadTimeout,
onIframeListenerRegistered,
onStylesheetLoad,
stylesheetLoadTimeout,
keepIframeSrcFn,
Expand Down Expand Up @@ -1223,7 +1263,7 @@ export function serializeNodeWithId(
serializedNode.type === NodeType.Element &&
serializedNode.tagName === 'iframe'
) {
onceIframeLoaded(
const iframeDisposer = onceIframeLoaded(
n as HTMLIFrameElement,
() => {
const iframeDoc = (n as HTMLIFrameElement).contentDocument;
Expand All @@ -1249,6 +1289,7 @@ export function serializeNodeWithId(
onSerialize,
onIframeLoad,
iframeLoadTimeout,
onIframeListenerRegistered,
onStylesheetLoad,
stylesheetLoadTimeout,
keepIframeSrcFn,
Expand All @@ -1266,6 +1307,7 @@ export function serializeNodeWithId(
},
iframeLoadTimeout,
);
onIframeListenerRegistered?.(n as HTMLIFrameElement, iframeDisposer);
}

// <link rel=stylesheet href=...>
Expand Down Expand Up @@ -1372,6 +1414,10 @@ function snapshot(
node: serializedElementNodeWithId,
) => unknown;
iframeLoadTimeout?: number;
onIframeListenerRegistered?: (
iframeNode: HTMLIFrameElement,
disposer: () => void,
) => void;
onStylesheetLoad?: (
linkNode: HTMLLinkElement,
node: serializedElementNodeWithId,
Expand Down Expand Up @@ -1399,6 +1445,7 @@ function snapshot(
onSerialize,
onIframeLoad,
iframeLoadTimeout,
onIframeListenerRegistered,
onStylesheetLoad,
stylesheetLoadTimeout,
keepIframeSrcFn = () => false,
Expand Down Expand Up @@ -1450,6 +1497,7 @@ function snapshot(
onSerialize,
onIframeLoad,
iframeLoadTimeout,
onIframeListenerRegistered,
onStylesheetLoad,
stylesheetLoadTimeout,
keepIframeSrcFn,
Expand Down
Loading