From c0d1d8920ca2fda95b9d7fed53ed3e314e3f9fc4 Mon Sep 17 00:00:00 2001 From: ocnc Date: Wed, 31 Dec 2025 16:18:01 -0500 Subject: [PATCH 1/2] fix: memory leak from detached snap iframes --- .../services/iframe/IframeExecutionService.ts | 12 ++++++- .../src/common/BaseSnapExecutor.ts | 32 +++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/packages/snaps-controllers/src/services/iframe/IframeExecutionService.ts b/packages/snaps-controllers/src/services/iframe/IframeExecutionService.ts index 71812eb559..1986de58d2 100644 --- a/packages/snaps-controllers/src/services/iframe/IframeExecutionService.ts +++ b/packages/snaps-controllers/src/services/iframe/IframeExecutionService.ts @@ -30,7 +30,17 @@ export class IframeExecutionService extends AbstractExecutionService { } protected terminateJob(jobWrapper: TerminateJobArgs): void { - document.getElementById(jobWrapper.id)?.remove(); + const iframe = document.getElementById( + jobWrapper.id, + ) as HTMLIFrameElement | null; + if (iframe) { + // Navigate to about:blank before removing to ensure the browser + // unloads the previous document and cleans up its event listeners. + // Without this, Firefox may keep the detached iframe's document alive + // with all its event listeners, causing a memory leak. + iframe.src = 'about:blank'; + iframe.remove(); + } } protected async initEnvStream(snapId: string): Promise<{ diff --git a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts index 3818439f46..4e7c7b278f 100644 --- a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts +++ b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts @@ -486,6 +486,38 @@ export class BaseSnapExecutor { data.runningEvaluations.forEach((evaluation) => evaluation.stop()), ); this.snapData.clear(); + + // Clean up global error handlers to prevent memory leaks. + // Without this, the event listeners keep references to the executor + // and prevent the iframe from being garbage collected. + if (this.snapPromiseErrorHandler) { + removeEventListener('unhandledrejection', this.snapPromiseErrorHandler); + this.snapPromiseErrorHandler = undefined; + } + + if (this.snapErrorHandler) { + removeEventListener('error', this.snapErrorHandler); + this.snapErrorHandler = undefined; + } + + // Destroy streams to clean up their internal event listeners. + // This is important for Firefox where detached iframes with active + // listeners are not garbage collected. + try { + if (!this.commandStream.destroyed) { + this.commandStream.destroy(); + } + } catch { + // Ignore errors during cleanup + } + + try { + if (!this.rpcStream.destroyed) { + this.rpcStream.destroy(); + } + } catch { + // Ignore errors during cleanup + } } // TODO: Either fix this lint violation or explain why it's necessary to From 4e1b218e77d095b1e211fc3b7329c4b1dc121241 Mon Sep 17 00:00:00 2001 From: ocnc Date: Wed, 31 Dec 2025 16:32:54 -0500 Subject: [PATCH 2/2] not needed. already handled by gc --- .../src/common/BaseSnapExecutor.ts | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts index 4e7c7b278f..3feb6edac2 100644 --- a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts +++ b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts @@ -499,25 +499,6 @@ export class BaseSnapExecutor { removeEventListener('error', this.snapErrorHandler); this.snapErrorHandler = undefined; } - - // Destroy streams to clean up their internal event listeners. - // This is important for Firefox where detached iframes with active - // listeners are not garbage collected. - try { - if (!this.commandStream.destroyed) { - this.commandStream.destroy(); - } - } catch { - // Ignore errors during cleanup - } - - try { - if (!this.rpcStream.destroyed) { - this.rpcStream.destroy(); - } - } catch { - // Ignore errors during cleanup - } } // TODO: Either fix this lint violation or explain why it's necessary to