Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
5 changes: 5 additions & 0 deletions .changeset/fix-wasmcontext-memory-leak.md
Original file line number Diff line number Diff line change
@@ -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
26 changes: 12 additions & 14 deletions packages/web-platform/web-core/ts/client/mainthread/LynxView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -405,27 +405,24 @@ export class LynxViewElement extends HTMLElement {
* @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.#disposeInstance();
}

This also fixes a secondary bug where lynxGroupId is referenced before declaration.
*/
async #disposeInstance() {
this.shadowRoot?.querySelector('[part="page"]')
?.setAttribute(
lynxDisposedAttribute,
'',
);
this.#instance?.[Symbol.asyncDispose]();
const oldInstance = this.#instance;
this.#instance = undefined;
if (oldInstance) {
await oldInstance[Symbol.asyncDispose]();
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}
if (this.shadowRoot) {
this.shadowRoot.innerHTML = '';
this.shadowRoot.adoptedStyleSheets = [];
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
this.#instance = undefined;
}

/**
Expand All @@ -436,14 +433,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();
await this.#disposeInstance();
}
const mtsRealmPromise = createIFrameRealm(this.shadowRoot!);
queueMicrotask(async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ export class LynxViewInstance implements AsyncDisposable {
}

async [Symbol.asyncDispose]() {
this.mtsWasmBinding.dispose();
await this.backgroundThread[Symbol.asyncDispose]();
Comment thread
PupilTong marked this conversation as resolved.
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ export type WASMJSBindingInjectedHandler = {

export class WASMJSBinding implements RustMainthreadContextBinding {
wasmContext: InstanceType<MainThreadWasmContext> | undefined;
disposeWasmContext?: () => void;
#addedEventListeners: Set<string> = new Set();
toBeEnabledElement: Set<HTMLElement> = new Set();
toBeDisabledElement: Set<HTMLElement> = new Set();

Expand Down Expand Up @@ -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,
Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +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;
mtsBinding.disposeWasmContext = () => {
if (wasmContext) {
wasmContext.free();
}
// @ts-expect-error parameter nullification for GC
wasmContext = null;
// @ts-expect-error parameter nullification for GC
rootDom = null;
// @ts-expect-error parameter nullification for GC
mtsBinding = null;
page = undefined;
timingFlags.length = 0;
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.
let page: DecoratedHTMLElement | undefined = undefined;
const timingFlags: string[] = [];

Expand Down
Loading