diff --git a/.changeset/iframe-memory-leak.md b/.changeset/iframe-memory-leak.md new file mode 100644 index 0000000000..d6650decc1 --- /dev/null +++ b/.changeset/iframe-memory-leak.md @@ -0,0 +1,5 @@ +--- +'posthog-js': 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 eight 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. diff --git a/packages/rrweb/rrweb-snapshot/src/snapshot.ts b/packages/rrweb/rrweb-snapshot/src/snapshot.ts index 55e7db3eb8..aabf76a705 100644 --- a/packages/rrweb/rrweb-snapshot/src/snapshot.ts +++ b/packages/rrweb/rrweb-snapshot/src/snapshot.ts @@ -346,54 +346,90 @@ 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; } + + // Re-fire on any later load so iframe.src navigations get their fresh + // contentDocument serialized too. Used by both branches below. + const onSubsequentLoad = () => listener(); + if (readyState !== 'complete') { - const timer = setTimeout(() => { - if (!fired) { - listener(); - fired = true; - } - }, iframeLoadTimeout); - iframeEl.addEventListener('load', () => { - clearTimeout(timer); + let fired = false; + let timer: ReturnType | null = null; + const fireOnce = () => { + if (fired) return; fired = true; + if (timer !== null) { + clearTimeout(timer); + timer = null; + } + iframeEl.removeEventListener('load', onInitialLoad); + iframeEl.addEventListener('load', onSubsequentLoad); listener(); - }); - return; + }; + const onInitialLoad = () => fireOnce(); + timer = setTimeout(fireOnce, iframeLoadTimeout); + iframeEl.addEventListener('load', onInitialLoad); + return () => { + if (timer !== null) { + clearTimeout(timer); + timer = null; + } + if (fired) { + iframeEl.removeEventListener('load', onSubsequentLoad); + } else { + fired = true; + iframeEl.removeEventListener('load', onInitialLoad); + } + }; } - // 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; + } 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); + }; } const stylesheetLoadTracked = new Map(); @@ -1069,6 +1105,10 @@ export function serializeNodeWithId( node: serializedElementNodeWithId, ) => unknown; iframeLoadTimeout?: number; + onIframeListenerRegistered?: ( + iframeNode: HTMLIFrameElement, + disposer: () => void, + ) => void; onStylesheetLoad?: ( linkNode: HTMLLinkElement, node: serializedElementNodeWithId, @@ -1098,6 +1138,7 @@ export function serializeNodeWithId( onSerialize, onIframeLoad, iframeLoadTimeout = 5000, + onIframeListenerRegistered, onStylesheetLoad, stylesheetLoadTimeout = 5000, keepIframeSrcFn = () => false, @@ -1225,6 +1266,7 @@ export function serializeNodeWithId( onSerialize, onIframeLoad, iframeLoadTimeout, + onIframeListenerRegistered, onStylesheetLoad, stylesheetLoadTimeout, keepIframeSrcFn, @@ -1269,7 +1311,7 @@ export function serializeNodeWithId( serializedNode.type === NodeType.Element && serializedNode.tagName === 'iframe' ) { - onceIframeLoaded( + const iframeDisposer = onceIframeLoaded( n as HTMLIFrameElement, () => { const iframeDoc = (n as HTMLIFrameElement).contentDocument; @@ -1295,6 +1337,7 @@ export function serializeNodeWithId( onSerialize, onIframeLoad, iframeLoadTimeout, + onIframeListenerRegistered, onStylesheetLoad, stylesheetLoadTimeout, keepIframeSrcFn, @@ -1312,6 +1355,7 @@ export function serializeNodeWithId( }, iframeLoadTimeout, ); + onIframeListenerRegistered?.(n as HTMLIFrameElement, iframeDisposer); } // @@ -1414,6 +1458,10 @@ function snapshot( node: serializedElementNodeWithId, ) => unknown; iframeLoadTimeout?: number; + onIframeListenerRegistered?: ( + iframeNode: HTMLIFrameElement, + disposer: () => void, + ) => void; onStylesheetLoad?: ( linkNode: HTMLLinkElement, node: serializedElementNodeWithId, @@ -1441,6 +1489,7 @@ function snapshot( onSerialize, onIframeLoad, iframeLoadTimeout, + onIframeListenerRegistered, onStylesheetLoad, stylesheetLoadTimeout, keepIframeSrcFn = () => false, @@ -1492,6 +1541,7 @@ function snapshot( onSerialize, onIframeLoad, iframeLoadTimeout, + onIframeListenerRegistered, onStylesheetLoad, stylesheetLoadTimeout, keepIframeSrcFn, diff --git a/packages/rrweb/rrweb/src/record/iframe-manager.ts b/packages/rrweb/rrweb/src/record/iframe-manager.ts index c7a6d1953a..3370cdac76 100644 --- a/packages/rrweb/rrweb/src/record/iframe-manager.ts +++ b/packages/rrweb/rrweb/src/record/iframe-manager.ts @@ -3,6 +3,7 @@ import { genId } from '@posthog/rrweb-snapshot'; import type { CrossOriginIframeMessageEvent } from '../types'; import { callSafely } from '../utils'; import CrossOriginIframeMirror from './cross-origin-iframe-mirror'; +import { findAndRemoveIframeBuffer } from './observer'; import { EventType, NodeType, IncrementalSource } from '@posthog/rrweb-types'; import type { eventWithTime, @@ -28,13 +29,29 @@ export class IframeManager { private stylesheetManager: StylesheetManager; private recordCrossOriginIframes: boolean; private messageHandler: (message: MessageEvent) => void; - // Map window to handler for cleanup - windows are browser-owned and won't prevent GC + // Strong Map — keys pin Windows; every entry must be deleted on detach. private nestedIframeListeners: Map void> = new Map(); + // Originals captured per iframe so cleanup survives iframe.src swaps. + private attachedWindows: WeakMap> = + new WeakMap(); + private attachedDocuments: WeakMap> = + new WeakMap(); private attachedIframes: Map< number, { element: HTMLIFrameElement; content: serializedNodeWithId } > = new Map(); + // Set per element — one iframe collects multiple disposers across loads. + private loadListenerDisposers: WeakMap void>> = + new WeakMap(); + // Fallback for iframes removed before first load — mirror entry is gone + // by then, but we still need the element to dispose its load listener. + private iframeElementsById: Map = new Map(); + // Set per element — same multi-load reasoning as loadListenerDisposers. + private pageHideHandlers: WeakMap< + HTMLIFrameElement, + Set<{ win: Window; handler: () => void }> + > = new WeakMap(); constructor(options: { mirror: Mirror; @@ -65,6 +82,51 @@ export class IframeManager { this.crossOriginIframeMap.set(iframeEl.contentWindow, iframeEl); } + public registerLoadListenerDisposer( + iframeEl: HTMLIFrameElement, + disposer: () => void, + ) { + let bucket = this.loadListenerDisposers.get(iframeEl); + if (!bucket) { + bucket = new Set(); + this.loadListenerDisposers.set(iframeEl, bucket); + } + bucket.add(disposer); + const id = this.mirror.getId(iframeEl); + if (id !== -1) this.iframeElementsById.set(id, iframeEl); + } + + // Used by the record-loop to distinguish reparenting from removal. + public getIframeElementById(iframeId: number): HTMLIFrameElement | null { + return ( + this.attachedIframes.get(iframeId)?.element ?? + this.iframeElementsById.get(iframeId) ?? + null + ); + } + + // Drops the id mapping for a moved iframe; element-keyed state survives. + public forgetIframeId(iframeId: number) { + this.attachedIframes.delete(iframeId); + this.iframeElementsById.delete(iframeId); + } + + private disposeLoadListeners(iframeEl: HTMLIFrameElement) { + const bucket = this.loadListenerDisposers.get(iframeEl); + if (!bucket) return; + bucket.forEach((d) => callSafely(d)); + this.loadListenerDisposers.delete(iframeEl); + } + + private removePageHideListener(iframeEl: HTMLIFrameElement) { + const bucket = this.pageHideHandlers.get(iframeEl); + if (!bucket) return; + bucket.forEach(({ win, handler }) => { + callSafely(() => win.removeEventListener('pagehide', handler)); + }); + this.pageHideHandlers.delete(iframeEl); + } + public addLoadListener(cb: (iframeEl: HTMLIFrameElement) => unknown) { this.loadListener = cb; } @@ -91,6 +153,15 @@ export class IframeManager { childSn: serializedNodeWithId, ) { const iframeId = this.trackIframeContent(iframeEl, childSn); + // Accumulate every contentDocument across loads (blank → src → blank). + if (iframeEl.contentDocument) { + let docs = this.attachedDocuments.get(iframeEl); + if (!docs) { + docs = new Set(); + this.attachedDocuments.set(iframeEl, docs); + } + docs.add(iframeEl.contentDocument); + } this.mutationCb({ adds: [ { @@ -116,11 +187,28 @@ export class IframeManager { callSafely(() => { win.addEventListener('message', nestedHandler); this.nestedIframeListeners.set(win, nestedHandler); + // Track per-iframe so detach finds it even after a contentWindow swap. + let wins = this.attachedWindows.get(iframeEl); + if (!wins) { + wins = new Set(); + this.attachedWindows.set(iframeEl, wins); + } + wins.add(win); }); } - callSafely(() => - iframeEl.contentWindow?.addEventListener('pagehide', () => { + callSafely(() => { + const pageHideWindow = iframeEl.contentWindow; + if (!pageHideWindow) return; + let bucket = this.pageHideHandlers.get(iframeEl); + // Reparented / re-attached iframes call attachIframe again on the same + // Window; skip if we already registered a handler for it. + if (bucket) { + for (const entry of bucket) { + if (entry.win === pageHideWindow) return; + } + } + const handler = () => { this.pageHideListener?.(iframeEl); if (iframeEl.contentDocument) { this.mirror.removeNodeFromMap(iframeEl.contentDocument); @@ -128,8 +216,14 @@ export class IframeManager { if (iframeEl.contentWindow) { this.crossOriginIframeMap.delete(iframeEl.contentWindow); } - }), - ); + }; + pageHideWindow.addEventListener('pagehide', handler); + if (!bucket) { + bucket = new Set(); + this.pageHideHandlers.set(iframeEl, bucket); + } + bucket.add({ win: pageHideWindow, handler }); + }); this.loadListener?.(iframeEl); @@ -360,33 +454,80 @@ export class IframeManager { public removeIframeById(iframeId: number) { const entry = this.attachedIframes.get(iframeId); + // attachedIframes / mirror may both be empty for iframes removed + // before first load; iframeElementsById covers that case. const iframe = entry?.element || + this.iframeElementsById.get(iframeId) || (this.mirror.getNode(iframeId) as HTMLIFrameElement | null); + this.iframeElementsById.delete(iframeId); + if (iframe) { const win = iframe.contentWindow; - // Clean up nested iframe listeners if they exist + // Clear listeners for every Window this iframe ever held — host + // may have swapped iframe.src before removal. + const capturedWins = this.attachedWindows.get(iframe); + if (capturedWins) { + capturedWins.forEach((capturedWin) => { + const handler = this.nestedIframeListeners.get(capturedWin); + if (handler) { + callSafely(() => + capturedWin.removeEventListener('message', handler), + ); + this.nestedIframeListeners.delete(capturedWin); + } + this.crossOriginIframeMap.delete(capturedWin); + }); + this.attachedWindows.delete(iframe); + } + // Legacy/test path: nestedIframeListeners populated without + // attachIframe (preserves SecurityError handling from #163). if (win && this.nestedIframeListeners.has(win)) { const handler = this.nestedIframeListeners.get(win)!; callSafely(() => win.removeEventListener('message', handler)); this.nestedIframeListeners.delete(win); } - // Clean up WeakMaps to allow GC of the iframe element if (win) { this.crossOriginIframeMap.delete(win); } this.iframes.delete(iframe); + + this.disposeLoadListeners(iframe); + this.removePageHideListener(iframe); + + // Walk captured docs so mirror.idNodeMap drops them even after + // an iframe.src swap, then splice their MutationBuffers. + const capturedDocs = this.attachedDocuments.get(iframe); + if (capturedDocs) { + capturedDocs.forEach((doc) => { + callSafely(() => this.mirror.removeNodeFromMap(doc)); + }); + callSafely(() => findAndRemoveIframeBuffer(iframe, capturedDocs)); + this.attachedDocuments.delete(iframe); + } } - // Always clean up attachedIframes if entry exists if (entry) { this.attachedIframes.delete(iframeId); } } + // Catches iframes removed inside a removed subtree (only the + // ancestor's id appears in m.removes). + public cleanupDetachedIframes() { + if (this.attachedIframes.size === 0) return; + const orphaned: number[] = []; + this.attachedIframes.forEach((_entry, iframeId) => { + if (!this.mirror.has(iframeId)) { + orphaned.push(iframeId); + } + }); + orphaned.forEach((iframeId) => this.removeIframeById(iframeId)); + } + public reattachIframes() { this.attachedIframes.forEach(({ content }, iframeId) => { // Verify the iframe ID is still in the mirror (still being tracked by rrweb) @@ -423,6 +564,19 @@ export class IframeManager { }); this.nestedIframeListeners.clear(); + // WeakMaps aren't iterable, so enumerate tracked iframes via the + // id-keyed Maps and dispose pending load listeners + pagehide + // handlers before dropping the WeakMaps. Otherwise stopRecording() + // would leave DOM listeners + onceIframeLoaded timers live, allowing + // a late iframe load to call wrappedEmit after recording stopped. + const tracked = new Set(); + this.iframeElementsById.forEach((el) => tracked.add(el)); + this.attachedIframes.forEach(({ element }) => tracked.add(element)); + tracked.forEach((iframe) => { + this.disposeLoadListeners(iframe); + this.removePageHideListener(iframe); + }); + this.crossOriginIframeMirror.reset(); this.crossOriginIframeStyleMirror.reset(); this.attachedIframes.clear(); @@ -430,5 +584,10 @@ export class IframeManager { this.crossOriginIframeMap = new WeakMap(); this.iframes = new WeakMap(); this.crossOriginIframeRootIdMap = new WeakMap(); + this.loadListenerDisposers = new WeakMap(); + this.pageHideHandlers = new WeakMap(); + this.attachedDocuments = new WeakMap(); + this.attachedWindows = new WeakMap(); + this.iframeElementsById = new Map(); } } diff --git a/packages/rrweb/rrweb/src/record/index.ts b/packages/rrweb/rrweb/src/record/index.ts index 94aeb01dce..9d57feb795 100644 --- a/packages/rrweb/rrweb/src/record/index.ts +++ b/packages/rrweb/rrweb/src/record/index.ts @@ -180,32 +180,15 @@ function record( let lastFullSnapshotEvent: eventWithTime; let incrementalSnapshotCount = 0; - // Track observer cleanup functions for individual iframes to prevent memory leaks - const iframeObserverCleanups = new Map(); + // Set per id — one iframe id can collect several cleanups across loads. + const iframeObserverCleanups = new Map>(); - function cleanupDetachedIframeObservers() { - for (const [iframeId, cleanup] of iframeObserverCleanups) { - const iframe = mirror.getNode(iframeId) as HTMLIFrameElement | null; - - if (!iframe) { - cleanup(); - iframeObserverCleanups.delete(iframeId); - continue; - } - - try { - // Check if iframe is detached or its content is no longer accessible - if (!iframe.contentDocument || !iframe.contentDocument.defaultView) { - cleanup(); - iframeObserverCleanups.delete(iframeId); - } - } catch { - // Cross-origin: contentDocument access throws, cleanup anyway - cleanup(); - iframeObserverCleanups.delete(iframeId); - } - } - } + // Forward-declared; assigned inside the try{} block where `handlers` is + // in scope. Optional-typed so a premature call is a no-op rather than a + // silently-swallowed cleanup — the try-block runs synchronously after the + // managers are constructed, but the types make that invariant explicit. + let runAndDetachIframeCleanup: ((iframeId: number) => void) | undefined; + let cleanupDetachedIframeObservers: (() => void) | undefined; const eventProcessor = (e: eventWithTime): T => { for (const plugin of plugins || []) { @@ -275,28 +258,40 @@ function record( }; const wrappedMutationEmit = (m: mutationCallbackParam) => { - // Clean up removed iframes from the attachedIframes Map to prevent memory leaks - // Only clean up iframes that are actually being removed, not moved - // (moved iframes appear in both removes and adds) - if (recordCrossOriginIframes && m.removes && m.removes.length > 0) { - // Only create the Set if there are adds to check against + // Clean up removed iframes (same-origin too). Detect reparenting by id + // AND by element identity — MutationBuffer.emit clears mirror entries + // before re-serializing adds, so a moved iframe may have a fresh id. + if (m.removes && m.removes.length > 0) { const addedIds = m.adds.length > 0 ? new Set(m.adds.map((add) => add.node.id)) : null; - m.removes.forEach(({ id }) => { - // Only remove if not being re-added (i.e., actually removed, not moved) - if (!addedIds || !addedIds.has(id)) { - // Disconnect observers for this iframe to prevent memory leaks - const cleanup = iframeObserverCleanups.get(id); - if (cleanup) { - cleanup(); - iframeObserverCleanups.delete(id); + const addedIframeElements = new Set(); + if (m.adds.length > 0) { + for (const add of m.adds) { + const node = mirror.getNode(add.node.id); + if (node && (node as Element).nodeName === 'IFRAME') { + addedIframeElements.add(node as HTMLIFrameElement); } - iframeManager.removeIframeById(id); } + } + + m.removes.forEach(({ id }) => { + if (addedIds && addedIds.has(id)) return; + const removedIframe = iframeManager.getIframeElementById(id); + if (removedIframe && addedIframeElements.has(removedIframe)) { + // Reparent: keep observers/listeners; just drop stale id mapping. + iframeManager.forgetIframeId(id); + return; + } + runAndDetachIframeCleanup?.(id); + iframeManager.removeIframeById(id); }); - // Safety net: cleanup any iframes that have become detached or inaccessible - cleanupDetachedIframeObservers(); + // Catch iframes removed inside a removed subtree (only the ancestor's + // id appears in m.removes). Disconnect observers before iframeManager + // releases the buffers, matching the order of the direct-remove path + // above so a queued mutation can't land on a freed buffer. + cleanupDetachedIframeObservers?.(); + iframeManager.cleanupDetachedIframes(); } wrappedEmit({ @@ -450,6 +445,12 @@ function record( iframeManager.attachIframe(iframe, childSn); shadowDomManager.observeAttachShadow(iframe); }, + onIframeListenerRegistered: ( + iframe: HTMLIFrameElement, + disposer: () => void, + ) => { + iframeManager.registerLoadListenerDisposer(iframe, disposer); + }, onStylesheetLoad: (linkEl, childSn) => { stylesheetManager.attachLinkElement(linkEl, childSn); }, @@ -487,6 +488,35 @@ function record( try { const handlers: listenerHandler[] = []; + // Disposes per-iframe observer cleanups and unlinks them from `handlers`. + runAndDetachIframeCleanup = (iframeId: number) => { + const cleanups = iframeObserverCleanups.get(iframeId); + if (!cleanups) return; + cleanups.forEach((cleanup) => { + callSafely(cleanup); + const idx = handlers.indexOf(cleanup); + if (idx !== -1) handlers.splice(idx, 1); + }); + iframeObserverCleanups.delete(iframeId); + }; + + cleanupDetachedIframeObservers = () => { + for (const [iframeId] of iframeObserverCleanups) { + const iframe = mirror.getNode(iframeId) as HTMLIFrameElement | null; + if (!iframe) { + runAndDetachIframeCleanup?.(iframeId); + continue; + } + try { + if (!iframe.contentDocument || !iframe.contentDocument.defaultView) { + runAndDetachIframeCleanup?.(iframeId); + } + } catch { + runAndDetachIframeCleanup?.(iframeId); + } + } + }; + const observe = (doc: Document) => { return callbackWrapper(initObservers)( { @@ -627,9 +657,14 @@ function record( const iframeId = mirror.getId(iframeEl); const cleanup = observe(iframeEl.contentDocument!); handlers.push(cleanup); - // Store cleanup function so we can disconnect this iframe's observers when it's removed + // Accumulate cleanups across iframe navigations. if (iframeId !== -1) { - iframeObserverCleanups.set(iframeId, cleanup); + let bucket = iframeObserverCleanups.get(iframeId); + if (!bucket) { + bucket = new Set(); + iframeObserverCleanups.set(iframeId, bucket); + } + bucket.add(cleanup); } } catch (error) { // TODO: handle internal error @@ -640,11 +675,7 @@ function record( iframeManager.addPageHideListener((iframeEl) => { const iframeId = mirror.getId(iframeEl); - const cleanup = iframeObserverCleanups.get(iframeId); - if (cleanup) { - cleanup(); - iframeObserverCleanups.delete(iframeId); - } + runAndDetachIframeCleanup?.(iframeId); findAndRemoveIframeBuffer(iframeEl); }); diff --git a/packages/rrweb/rrweb/src/record/mutation.ts b/packages/rrweb/rrweb/src/record/mutation.ts index 9a3093d2c4..256dd148bd 100644 --- a/packages/rrweb/rrweb/src/record/mutation.ts +++ b/packages/rrweb/rrweb/src/record/mutation.ts @@ -254,6 +254,15 @@ export default class MutationBuffer { public reset() { this.shadowDomManager.reset(); this.canvasManager.reset(); + // Don't null `this.doc` here — a MutationObserver callback queued before + // the observer was disconnected can still drain through `emit`, which + // passes `this.doc` to `serializeNodeWithId`. Splicing the buffer out of + // `mutationBuffers[]` + removing the cleanup closure from `handlers[]` + // releases the only strong refs to the buffer; GC handles the rest. + } + + public bufferDoc(): Document { + return this.doc; } public destroy() { @@ -339,6 +348,12 @@ export default class MutationBuffer { this.iframeManager.attachIframe(iframe, childSn); this.shadowDomManager.observeAttachShadow(iframe); }, + onIframeListenerRegistered: ( + iframe: HTMLIFrameElement, + disposer: () => void, + ) => { + this.iframeManager.registerLoadListenerDisposer(iframe, disposer); + }, onStylesheetLoad: (link, childSn) => { this.stylesheetManager.attachLinkElement(link, childSn); }, @@ -353,6 +368,12 @@ export default class MutationBuffer { } }; + // Drain mapRemoves before pushAdd so the mirror only holds nodes that + // are still live. Reparent detection in record/index.ts depends on this + // order — it resolves an `add`'s fresh id back to an element via + // `mirror.getNode` and matches it against the iframe behind the removed + // id. Reorder this and iframe moves will look like remove+add to that + // path, tearing down observers on a still-live iframe. while (this.mapRemoves.length) { this.mirror.removeNodeFromMap(this.mapRemoves.shift()!); } diff --git a/packages/rrweb/rrweb/src/record/observer.ts b/packages/rrweb/rrweb/src/record/observer.ts index 7395fa451b..1243ebf1d3 100644 --- a/packages/rrweb/rrweb/src/record/observer.ts +++ b/packages/rrweb/rrweb/src/record/observer.ts @@ -376,10 +376,20 @@ function initViewportResizeObserver( return on('resize', updateDimension, win); } -export function findAndRemoveIframeBuffer(iframeEl: HTMLIFrameElement) { +// `knownDocs` matches buffers whose iframe.contentDocument has been swapped +// before removal — bufferBelongsToIframe alone misses them. +export function findAndRemoveIframeBuffer( + iframeEl: HTMLIFrameElement, + knownDocs?: Set, +) { for (let i = mutationBuffers.length - 1; i >= 0; i--) { const buf = mutationBuffers[i]; - if (buf?.bufferBelongsToIframe(iframeEl)) { + if (!buf) continue; + let match = buf.bufferBelongsToIframe(iframeEl); + if (!match && knownDocs && knownDocs.has(buf.bufferDoc())) { + match = true; + } + if (match) { buf.reset(); mutationBuffers.splice(i, 1); } diff --git a/packages/rrweb/rrweb/test/record/memory-leaks.test.ts b/packages/rrweb/rrweb/test/record/memory-leaks.test.ts index 7874029ee9..99c209333f 100644 --- a/packages/rrweb/rrweb/test/record/memory-leaks.test.ts +++ b/packages/rrweb/rrweb/test/record/memory-leaks.test.ts @@ -246,15 +246,14 @@ describe('memory leak prevention', () => { // Stop recording - this should call destroy() which creates new WeakMaps stopRecording?.(); - // Verify that WeakMaps were created during cleanup - // The IframeManager destroy() creates: - // - 3 WeakMaps for IframeManager (crossOriginIframeMap, iframes, crossOriginIframeRootIdMap) - // - 4 WeakMaps from CrossOriginIframeMirror.reset() calls (2 mirrors × 2 WeakMaps each) - // - 1 WeakMap from other cleanup - // Total: 8 new WeakMaps + // 7 WeakMaps from IframeManager (crossOriginIframeMap, iframes, + // crossOriginIframeRootIdMap, loadListenerDisposers, pageHideHandlers, + // attachedDocuments, attachedWindows) + // + 4 from CrossOriginIframeMirror.reset() (2 mirrors × 2 WeakMaps each) + // + 1 from other cleanup const newWeakMapsCreated = weakMapConstructorCalls - weakMapCallsAfterRecord; - expect(newWeakMapsCreated).toBe(8); + expect(newWeakMapsCreated).toBe(12); // Restore original WeakMap global.WeakMap = originalWeakMap; @@ -842,5 +841,438 @@ describe('memory leak prevention', () => { stopRecording?.(); }); + + // Regression test for the same-origin iframe leak: previously the + // wrappedMutationEmit cleanup was gated on `recordCrossOriginIframes`, + // which left observers and iframe-manager state retained for every + // mounted/unmounted same-origin iframe. + it('should disconnect iframe observers even when recordCrossOriginIframes is false', async () => { + const emit = (event: eventWithTime) => { + events.push(event); + }; + + const stopRecording = record({ + emit, + // explicitly default — this is the case that used to leak + recordCrossOriginIframes: false, + }); + + const iframe = document.createElement('iframe'); + document.body.appendChild(iframe); + + await new Promise((resolve) => setTimeout(resolve, 50)); + + const buffersBeforeRemoval = mutationBuffers.length; + expect(buffersBeforeRemoval).toBeGreaterThan(1); // main doc + iframe + + document.body.removeChild(iframe); + + await new Promise((resolve) => setTimeout(resolve, 50)); + + expect(mutationBuffers.length).toBe(buffersBeforeRemoval - 1); + + stopRecording?.(); + }); + + it('should clean up nested iframes that are removed via parent removal', async () => { + const emit = (event: eventWithTime) => { + events.push(event); + }; + + const stopRecording = record({ + emit, + recordCrossOriginIframes: false, + }); + + const container = document.createElement('div'); + const iframe = document.createElement('iframe'); + container.appendChild(iframe); + document.body.appendChild(container); + + await new Promise((resolve) => setTimeout(resolve, 50)); + + const buffersBeforeRemoval = mutationBuffers.length; + expect(buffersBeforeRemoval).toBeGreaterThan(1); + + // Remove the parent — the mutation buffer only reports the parent in + // m.removes, so cleanup must walk the subtree (or fall back via the + // mirror) to find the nested iframe. + document.body.removeChild(container); + + await new Promise((resolve) => setTimeout(resolve, 50)); + + expect(mutationBuffers.length).toBe(buffersBeforeRemoval - 1); + + stopRecording?.(); + }); + }); + + // Regression for the "src swap before removal" leak: React-style cleanup + // sets iframe.src = 'about:blank' immediately before unmounting the iframe. + // By the time rrweb sees the removal, iframe.contentDocument is the new + // about:blank doc, so the recursive mirror cleanup walks an empty document + // and the original document's nodes stay pinned in mirror.idNodeMap. + // IframeManager must capture the original contentDocument at attach time + // and release it explicitly on detach. + describe('IframeManager.attachedDocuments', () => { + function makeIframeManager() { + const mirror = createMirror(); + const iframeManager = new IframeManager({ + mirror, + mutationCb: vi.fn(), + stylesheetManager: { + styleMirror: { generateId: vi.fn(() => 1) }, + adoptStyleSheets: vi.fn(), + } as any, + recordCrossOriginIframes: false, + wrappedEmit: vi.fn(), + }); + return { iframeManager, mirror }; + } + + it('walks the original contentDocument on detach, not the current one', () => { + const { iframeManager, mirror } = makeIframeManager(); + + const iframe = document.createElement('iframe'); + document.body.appendChild(iframe); + + const iframeId = 700; + mirror.add(iframe, { + type: 2, + tagName: 'iframe', + attributes: {}, + childNodes: [], + id: iframeId, + }); + + // Simulate the original (pre-swap) contentDocument with serialized nodes + // tracked in the mirror. + const originalDoc = iframe.contentDocument!; + const trackedChild = originalDoc.createElement('div'); + originalDoc.body.appendChild(trackedChild); + mirror.add(trackedChild, { + type: 2, + tagName: 'div', + attributes: {}, + childNodes: [], + id: 701, + }); + + iframeManager.addIframe(iframe); + iframeManager.attachIframe(iframe, { + type: 0, + childNodes: [], + id: 702, + } as any); + + expect(mirror.has(701)).toBe(true); + + // Swap contentDocument to a fresh document — this is what the host does + // when it sets iframe.src = 'about:blank' before unmounting. + const swappedDoc = document.implementation.createHTMLDocument(); + Object.defineProperty(iframe, 'contentDocument', { + configurable: true, + get: () => swappedDoc, + }); + + iframeManager.removeIframeById(iframeId); + + // The captured original-doc walk must release the tracked child node + // even though iframe.contentDocument no longer points at the original. + expect(mirror.has(701)).toBe(false); + + document.body.removeChild(iframe); + iframeManager.destroy(); + }); + + // Regression for v3 — a single iframe element lives through multiple + // documents (initial about:blank → loaded src → cleanup about:blank). + // attachIframe runs once per load event, so we must accumulate every + // doc; tracking only the most recent leaks every prior doc. + it('walks every contentDocument that has been attached, not just the last one', () => { + const { iframeManager, mirror } = makeIframeManager(); + + const iframe = document.createElement('iframe'); + document.body.appendChild(iframe); + + const iframeId = 800; + mirror.add(iframe, { + type: 2, + tagName: 'iframe', + attributes: {}, + childNodes: [], + id: iframeId, + }); + + // Doc #1 — initial about:blank with a tracked node. + const doc1 = iframe.contentDocument!; + const child1 = doc1.createElement('div'); + doc1.body.appendChild(child1); + mirror.add(child1, { + type: 2, + tagName: 'div', + attributes: {}, + childNodes: [], + id: 801, + }); + iframeManager.addIframe(iframe); + iframeManager.attachIframe(iframe, { + type: 0, + childNodes: [], + id: 802, + } as any); + + // Doc #2 — host swaps contentDocument (e.g. after iframe.src loads). + const doc2 = document.implementation.createHTMLDocument(); + const child2 = doc2.createElement('span'); + doc2.body.appendChild(child2); + mirror.add(child2, { + type: 2, + tagName: 'span', + attributes: {}, + childNodes: [], + id: 803, + }); + Object.defineProperty(iframe, 'contentDocument', { + configurable: true, + get: () => doc2, + }); + iframeManager.attachIframe(iframe, { + type: 0, + childNodes: [], + id: 804, + } as any); + + // Doc #3 — host swaps to about:blank just before unmount. No tracked + // nodes, but it shouldn't make us forget docs 1 and 2. + const doc3 = document.implementation.createHTMLDocument(); + Object.defineProperty(iframe, 'contentDocument', { + configurable: true, + get: () => doc3, + }); + iframeManager.attachIframe(iframe, { + type: 0, + childNodes: [], + id: 805, + } as any); + + expect(mirror.has(801)).toBe(true); + expect(mirror.has(803)).toBe(true); + + iframeManager.removeIframeById(iframeId); + + // Both prior docs' tracked nodes must be released — not just the last. + expect(mirror.has(801)).toBe(false); + expect(mirror.has(803)).toBe(false); + + document.body.removeChild(iframe); + iframeManager.destroy(); + }); + + // Regression: an iframe can be removed before its first load fires — + // attachIframe never runs, so attachedIframes has no entry, and by the + // time wrappedMutationEmit calls removeIframeById the mirror entry has + // typically been cleared by MutationBuffer.emit. We must still find the + // iframe element by id (via `iframeElementsById`, populated when the + // load-listener disposer was registered) so the disposer fires and the + // listener closure does not pin the iframe. + it('disposes the load listener for an iframe removed before first load', () => { + const mirror = createMirror(); + const mutationCb = vi.fn(); + const wrappedEmit = vi.fn(); + + const iframeManager = new IframeManager({ + mirror, + mutationCb, + stylesheetManager: { + styleMirror: { generateId: vi.fn(() => 1) }, + adoptStyleSheets: vi.fn(), + } as any, + recordCrossOriginIframes: false, + wrappedEmit, + }); + + const iframe = document.createElement('iframe'); + document.body.appendChild(iframe); + + const iframeId = 900; + mirror.add(iframe, { + type: 2, + tagName: 'iframe', + attributes: {}, + childNodes: [], + id: iframeId, + }); + + const disposer = vi.fn(); + iframeManager.registerLoadListenerDisposer(iframe, disposer); + + // Simulate the host removing the iframe before any load event fires — + // by mutation emit time the mirror entry is typically gone too. + mirror.removeNodeFromMap(iframe); + iframeManager.removeIframeById(iframeId); + + // The fallback id→element map must have let removeIframeById find the + // iframe and run the disposer; without that the load listener would + // stay registered and pin the iframe via its closure. + expect(disposer).toHaveBeenCalledTimes(1); + + document.body.removeChild(iframe); + iframeManager.destroy(); + }); + + // Regression: a moved (reparented) iframe shows up in MutationBuffer + // as remove(oldId) + add(newId) because mirror entries are cleared + // before adds re-serialize. The cleanup path must NOT dispose the + // (still-active) load listener / observers — only drop the stale id + // bookkeeping. forgetIframeId is the API the record-loop uses for + // that, and disposeLoadListeners must NOT be called on the element. + it('forgetIframeId drops id bookkeeping but preserves listener disposers', () => { + const mirror = createMirror(); + const mutationCb = vi.fn(); + const wrappedEmit = vi.fn(); + + const iframeManager = new IframeManager({ + mirror, + mutationCb, + stylesheetManager: { + styleMirror: { generateId: vi.fn(() => 1) }, + adoptStyleSheets: vi.fn(), + } as any, + recordCrossOriginIframes: false, + wrappedEmit, + }); + + const iframe = document.createElement('iframe'); + document.body.appendChild(iframe); + + const oldId = 1100; + mirror.add(iframe, { + type: 2, + tagName: 'iframe', + attributes: {}, + childNodes: [], + id: oldId, + }); + + const disposer = vi.fn(); + iframeManager.registerLoadListenerDisposer(iframe, disposer); + iframeManager.attachIframe(iframe, { + type: 0, + childNodes: [], + id: 1101, + } as any); + + // Sanity: the manager knows about this iframe under oldId. + expect(iframeManager.getIframeElementById(oldId)).toBe(iframe); + + // Simulate the move: forget the old id but keep all element-keyed + // bookkeeping for the same iframe. Disposer must not fire. + iframeManager.forgetIframeId(oldId); + + expect(disposer).not.toHaveBeenCalled(); + expect(iframeManager.getIframeElementById(oldId)).toBeNull(); + + // Re-registering under a new id (as the move's add path would) and + // then a real removal must still dispose the listener exactly once. + const newId = 1102; + mirror.removeNodeFromMap(iframe); + mirror.add(iframe, { + type: 2, + tagName: 'iframe', + attributes: {}, + childNodes: [], + id: newId, + }); + iframeManager.registerLoadListenerDisposer(iframe, disposer); + iframeManager.removeIframeById(newId); + // disposer is in the Set once, and callSafely calls it once. + expect(disposer).toHaveBeenCalledTimes(1); + + document.body.removeChild(iframe); + iframeManager.destroy(); + }); + + // Same move scenario, exercised through the real record() pipeline: + // an iframe reparented from one container to another should not lose + // its mutation observer (i.e. the per-iframe MutationBuffer count + // must not decrease as a side effect of the move). + it('moving an iframe to a new parent does not tear down its observers', async () => { + const events: eventWithTime[] = []; + const stopRecording = record({ + emit: (e) => events.push(e), + }); + + const c1 = document.createElement('div'); + const c2 = document.createElement('div'); + document.body.appendChild(c1); + document.body.appendChild(c2); + + const iframe = document.createElement('iframe'); + c1.appendChild(iframe); + + // Let the load listener register and any synchronous mutation + // observer setup complete. + await new Promise((r) => setTimeout(r, 50)); + const buffersBeforeMove = mutationBuffers.length; + + // Move (remove + add in one atomic mutation). + c2.appendChild(iframe); + await new Promise((r) => setTimeout(r, 50)); + + // Reparent must not splice any per-iframe MutationBuffer entries. + // (The count can grow because the add side re-serializes the iframe + // and `attachIframe` registers a fresh observer alongside the kept + // old one; that's a known limitation, separate from this leak.) + expect(mutationBuffers.length).toBeGreaterThanOrEqual(buffersBeforeMove); + + stopRecording?.(); + document.body.removeChild(c1); + document.body.removeChild(c2); + }); + + // destroy() must run pending iframe load-listener disposers and + // pagehide-handler removers; reassigning the WeakMaps without + // iterating leaves the DOM listeners alive and the + // onceIframeLoaded timer scheduled. + it('destroy() disposes pending iframe load listeners', () => { + const mirror = createMirror(); + const mutationCb = vi.fn(); + const wrappedEmit = vi.fn(); + + const iframeManager = new IframeManager({ + mirror, + mutationCb, + stylesheetManager: { + styleMirror: { generateId: vi.fn(() => 1) }, + adoptStyleSheets: vi.fn(), + } as any, + recordCrossOriginIframes: false, + wrappedEmit, + }); + + const iframe = document.createElement('iframe'); + document.body.appendChild(iframe); + const iframeId = 1300; + mirror.add(iframe, { + type: 2, + tagName: 'iframe', + attributes: {}, + childNodes: [], + id: iframeId, + }); + + const disposer = vi.fn(); + iframeManager.registerLoadListenerDisposer(iframe, disposer); + + iframeManager.destroy(); + + // Without the iterate-and-dispose step in destroy(), the disposer + // would be silently dropped and the DOM listener / timeout would + // outlive recording. + expect(disposer).toHaveBeenCalledTimes(1); + + document.body.removeChild(iframe); + }); }); });