diff --git a/.changeset/fix-wasmcontext-memory-leak.md b/.changeset/fix-wasmcontext-memory-leak.md new file mode 100644 index 0000000000..5436abaf4c --- /dev/null +++ b/.changeset/fix-wasmcontext-memory-leak.md @@ -0,0 +1,5 @@ +--- +"@lynx-js/web-core": patch +--- + +fix(web-platform): completely detach event listeners and forcefully free `MainThreadWasmContext` pointer alongside strict FIFO async component disposal to ensure total memory reclamation without use-after-free risks diff --git a/packages/web-platform/web-core/ts/client/mainthread/Background.ts b/packages/web-platform/web-core/ts/client/mainthread/Background.ts index 45c2331f8a..3151838fdc 100644 --- a/packages/web-platform/web-core/ts/client/mainthread/Background.ts +++ b/packages/web-platform/web-core/ts/client/mainthread/Background.ts @@ -284,15 +284,10 @@ export class BackgroundThread implements AsyncDisposable { } async [Symbol.asyncDispose](): Promise { - await this.#btsReady; - /* - * TODO: - * Potential deadlock if startBTS() was never called. - * If [Symbol.asyncDispose]() is invoked on a BackgroundThread instance where startBTS() was never called, - * #btsReady will never resolve, causing the disposal to hang indefinitely. - * Consider guarding with the existing #btsStarted flag. - */ - await this.#rpc.invoke(disposeEndpoint, []); + if (this.#btsStarted) { + await this.#btsReady; + await this.#rpc.invoke(disposeEndpoint, []); + } if (this.#lynxGroupId !== undefined) { const group = BackgroundThread.contextIdToBackgroundWorker[this.#lynxGroupId]; diff --git a/packages/web-platform/web-core/ts/client/mainthread/LynxView.ts b/packages/web-platform/web-core/ts/client/mainthread/LynxView.ts index d58c376f21..2b8270058c 100644 --- a/packages/web-platform/web-core/ts/client/mainthread/LynxView.ts +++ b/packages/web-platform/web-core/ts/client/mainthread/LynxView.ts @@ -401,31 +401,40 @@ export class LynxViewElement extends HTMLElement { public injectStyleRules?: string[]; + #disposePromise?: Promise; + /** * @private */ disconnectedCallback() { - /* TODO: - * Await async disposal before re-rendering to prevent concurrent instance mutations. - - Currently disconnectedCallback() triggers asyncDispose() without awaiting, allowing #render() to immediately create a new instance while the old one is still cleaning up on the background thread. This causes both instances to render into the shadowRoot concurrently, producing multiple page elements. - - The basic-reload-page-only-one test confirms this issue by checking that exactly one page element exists after reload. The disposal must complete before the new instance begins rendering. - - Extract an async #disposeInstance() method that marks the old page as disposed, awaits the instance cleanup, clears the shadowRoot, and resets adoptedStyleSheets to prevent stylesheet accumulation. Then await this in the microtask before instantiating the new LynxViewInstance. + this.#connected = false; + this.#disposeInstance(); + } - This also fixes a secondary bug where lynxGroupId is referenced before declaration. - */ - this.shadowRoot?.querySelector('[part="page"]') - ?.setAttribute( - lynxDisposedAttribute, - '', - ); - this.#instance?.[Symbol.asyncDispose](); - if (this.shadowRoot) { - this.shadowRoot.innerHTML = ''; + async #disposeInstance() { + if (this.#disposePromise) { + return this.#disposePromise; } - this.#instance = undefined; + const dispose = async () => { + this.shadowRoot?.querySelector('[part="page"]') + ?.setAttribute( + lynxDisposedAttribute, + '', + ); + const oldInstance = this.#instance; + this.#instance = undefined; + if (oldInstance) { + await oldInstance[Symbol.asyncDispose](); + } + if (this.shadowRoot) { + this.shadowRoot.innerHTML = ''; + this.shadowRoot.adoptedStyleSheets = []; + } + }; + + this.#disposePromise = dispose(); + await this.#disposePromise; + this.#disposePromise = undefined; } /** @@ -436,14 +445,15 @@ export class LynxViewElement extends HTMLElement { /** * @private */ - #render() { + async #render() { if (!this.#rendering && this.#connected) { this.#rendering = true; if (!this.shadowRoot) { this.attachShadow({ mode: 'open' }); } - if (this.#instance) { - this.disconnectedCallback(); + + if (this.#instance || this.#disposePromise) { + await this.#disposeInstance(); } const mtsRealmPromise = createIFrameRealm(this.shadowRoot!); queueMicrotask(async () => { diff --git a/packages/web-platform/web-core/ts/client/mainthread/LynxViewInstance.ts b/packages/web-platform/web-core/ts/client/mainthread/LynxViewInstance.ts index ab3a7eabaa..0b5ca3a2c5 100644 --- a/packages/web-platform/web-core/ts/client/mainthread/LynxViewInstance.ts +++ b/packages/web-platform/web-core/ts/client/mainthread/LynxViewInstance.ts @@ -297,6 +297,7 @@ export class LynxViewInstance implements AsyncDisposable { } async [Symbol.asyncDispose]() { + this.mtsWasmBinding.dispose(); await this.backgroundThread[Symbol.asyncDispose](); } } diff --git a/packages/web-platform/web-core/ts/client/mainthread/elementAPIs/WASMJSBinding.ts b/packages/web-platform/web-core/ts/client/mainthread/elementAPIs/WASMJSBinding.ts index 2d169e5cbd..e42416a7d9 100644 --- a/packages/web-platform/web-core/ts/client/mainthread/elementAPIs/WASMJSBinding.ts +++ b/packages/web-platform/web-core/ts/client/mainthread/elementAPIs/WASMJSBinding.ts @@ -25,6 +25,8 @@ export type WASMJSBindingInjectedHandler = { export class WASMJSBinding implements RustMainthreadContextBinding { wasmContext: InstanceType | undefined; + disposeWasmContext?: () => void; + #addedEventListeners: Set = new Set(); toBeEnabledElement: Set = new Set(); toBeDisabledElement: Set = new Set(); @@ -185,8 +187,11 @@ export class WASMJSBinding implements RustMainthreadContextBinding { }; addEventListener(eventName: string) { + const w3cEventName = LynxEventNameToW3cCommon[eventName] ?? eventName; + if (this.#addedEventListeners.has(w3cEventName)) return; + this.#addedEventListeners.add(w3cEventName); this.lynxViewInstance.rootDom.addEventListener( - LynxEventNameToW3cCommon[eventName] ?? eventName, + w3cEventName, this.#commonEventHandler, { passive: true, @@ -195,6 +200,26 @@ export class WASMJSBinding implements RustMainthreadContextBinding { ); } + dispose() { + for (const eventName of this.#addedEventListeners) { + this.lynxViewInstance.rootDom.removeEventListener( + eventName, + this.#commonEventHandler, + { + passive: true, + capture: true, + } as any, + ); + } + this.#addedEventListeners.clear(); + + this.toBeEnabledElement.clear(); + this.toBeDisabledElement.clear(); + + this.disposeWasmContext?.(); + this.wasmContext = undefined; + } + postTimingFlags(flags: string[], pipelineId?: string) { this.lynxViewInstance.backgroundThread.postTimingFlags(flags, pipelineId); } diff --git a/packages/web-platform/web-core/ts/client/mainthread/elementAPIs/createElementAPI.ts b/packages/web-platform/web-core/ts/client/mainthread/elementAPIs/createElementAPI.ts index a5b3bc549f..e3ed99fc33 100644 --- a/packages/web-platform/web-core/ts/client/mainthread/elementAPIs/createElementAPI.ts +++ b/packages/web-platform/web-core/ts/client/mainthread/elementAPIs/createElementAPI.ts @@ -68,14 +68,25 @@ export function createElementAPI( transform_vh: boolean, transform_rem: boolean, ): ElementPAPIs { - const wasmContext = new MainThreadWasmContext( + let wasmContext = new MainThreadWasmContext( rootDom, mtsBinding, config_enable_css_selector, ); - mtsBinding.wasmContext = wasmContext; let page: DecoratedHTMLElement | undefined = undefined; const timingFlags: string[] = []; + let disposed = false; + + mtsBinding.wasmContext = wasmContext; + mtsBinding.disposeWasmContext = () => { + if (disposed) return; + disposed = true; + if (wasmContext) { + wasmContext.free(); + } + page = undefined; + timingFlags.length = 0; + }; const __SetCSSId: SetCSSIdPAPI = (elements, cssId, entryName) => { const uniqueIds = elements.map( @@ -603,6 +614,7 @@ export function createElementAPI( wasmContext.take_timing_flags(), ); requestIdleCallbackImpl(() => { + if (disposed) return; mtsBinding.postTimingFlags( timingFlagsAll, pipelineId,