Skip to content
Merged
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -284,15 +284,10 @@ export class BackgroundThread implements AsyncDisposable {
}

async [Symbol.asyncDispose](): Promise<void> {
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];
Expand Down
54 changes: 32 additions & 22 deletions packages/web-platform/web-core/ts/client/mainthread/LynxView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -401,31 +401,40 @@ export class LynxViewElement extends HTMLElement {

public injectStyleRules?: string[];

#disposePromise?: Promise<void>;

/**
* @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;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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;
}

/**
Expand All @@ -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 () => {
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,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;
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.

const __SetCSSId: SetCSSIdPAPI = (elements, cssId, entryName) => {
const uniqueIds = elements.map(
Expand Down Expand Up @@ -603,6 +614,7 @@ export function createElementAPI(
wasmContext.take_timing_flags(),
);
requestIdleCallbackImpl(() => {
if (disposed) return;
mtsBinding.postTimingFlags(
timingFlagsAll,
pipelineId,
Expand Down
Loading