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-bundle-race-condition.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@lynx-js/web-core": patch
---

fix(web-core): avoid partial bundle loading and double fetching when fetchBundle is called concurrently for the same url.
79 changes: 69 additions & 10 deletions packages/web-platform/web-core/tests/template-manager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ describe('Template Manager', () => {
);

// Verify data using getCustomSection
const customSections = templateManager.getTemplate(templateUrl)
const customSections = templateManager.getBundle(templateUrl)
?.customSections;
const decoder = new TextDecoder('utf-16le');
const decodedCustomSections = JSON.parse(
Expand Down Expand Up @@ -130,16 +130,16 @@ describe('Template Manager', () => {
.rejects.toThrow('Unsupported version: 2');

// Verify template is removed
expect(templateManager.getTemplate(templateUrl)?.customSections)
expect(templateManager.getBundle(templateUrl)?.customSections)
.toBeUndefined();
});

/*
test('should throw error for create same template twice', () => {
const templateUrl = 'http://example.com/template_duplicate_url_test';
templateManager.createTemplate(templateUrl);
templateManager.createBundle(templateUrl);
expect(() => {
templateManager.createTemplate(templateUrl);
templateManager.createBundle(templateUrl);
}).toThrow();
});
*/
Expand Down Expand Up @@ -172,7 +172,7 @@ describe('Template Manager', () => {
);

// Verify data using getCustomSection
const customSections = templateManager.getTemplate(
const customSections = templateManager.getBundle(
'http://example.com/template',
)?.customSections;
const decoder = new TextDecoder('utf-16le');
Expand All @@ -185,17 +185,17 @@ describe('Template Manager', () => {
/*
test('should remove template correctly', () => {
const templateUrl = 'http://example.com/template_to_remove';
templateManager.createTemplate(templateUrl);
templateManager.createBundle(templateUrl);

// Manually set a custom section to verify existence
templateManager.setCustomSection(templateUrl, { test: 'data' });
expect(templateManager.getTemplate(templateUrl)?.customSections).toEqual({
expect(templateManager.getBundle(templateUrl)?.customSections).toEqual({
test: 'data',
});

templateManager.removeTemplate(templateUrl);
templateManager.removeBundle(templateUrl);

expect(templateManager.getTemplate(templateUrl)?.customSections)
expect(templateManager.getBundle(templateUrl)?.customSections)
.toBeUndefined();
});
*/
Expand Down Expand Up @@ -228,7 +228,7 @@ describe('Template Manager', () => {
),
).rejects.toThrow('Stream failed');

expect(templateManager.getTemplate(templateUrl)?.customSections)
expect(templateManager.getBundle(templateUrl)?.customSections)
.toBeUndefined();
});

Expand Down Expand Up @@ -394,4 +394,63 @@ describe('Template Manager', () => {
}),
);
});

test('should not result in partial bundle when fetchBundle is called twice concurrently', async () => {
const encoded = encode(sampleTasm);

const stream = new ReadableStream({
async start(controller) {
// Enqueue with a small delay so concurrent requests wait
await new Promise(resolve => setTimeout(resolve, 10));
controller.enqueue(encoded);
controller.close();
},
});

(globalThis.fetch as any).mockResolvedValue({
ok: true,
status: 200,
statusText: 'OK',
body: stream,
});

const instance1 = {
...mockLynxViewInstance,
onPageConfigReady: vi.fn(),
backgroundThread: { markTiming: vi.fn() },
};
const instance2 = {
...mockLynxViewInstance,
onPageConfigReady: vi.fn(),
backgroundThread: { markTiming: vi.fn() },
};

// Trigger both concurrently
await Promise.all([
templateManager.fetchBundle(
'http://example.com/template_concurrent',
Promise.resolve(instance1 as unknown as LynxViewInstance),
false,
false,
),
templateManager.fetchBundle(
'http://example.com/template_concurrent',
Promise.resolve(instance2 as unknown as LynxViewInstance),
false,
false,
),
]);

// Verify both finish correctly
const customSections = templateManager.getBundle(
'http://example.com/template_concurrent',
)?.customSections;
const decoder = new TextDecoder('utf-16le');
const decodedCustomSections = JSON.parse(
decoder.decode(customSections as unknown as Uint8Array),
);
expect(decodedCustomSections).toEqual(sampleTasm.customSections);
expect(instance1.onPageConfigReady).toHaveBeenCalled();
expect(instance2.onPageConfigReady).toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ export class LynxViewInstance implements AsyncDisposable {

async onMTSScriptsLoaded(currentUrl: string, isLazy: boolean) {
this.backgroundThread.markTiming('lepus_execute_start');
const urlMap = templateManager.getTemplate(currentUrl)
const urlMap = templateManager.getBundle(currentUrl)
?.lepusCode as Record<string, string>;
this.lepusCodeUrls.set(
currentUrl,
Expand All @@ -181,8 +181,8 @@ export class LynxViewInstance implements AsyncDisposable {
this.backgroundThread.startWebWorker(
processedData,
this.globalprops,
templateManager.getTemplate(this.templateUrl)!.config!.cardType,
templateManager.getTemplate(this.templateUrl)?.customSections as Record<
templateManager.getBundle(this.templateUrl)!.config!.cardType,
templateManager.getBundle(this.templateUrl)?.customSections as Record<
string,
Cloneable
>,
Expand All @@ -197,7 +197,7 @@ export class LynxViewInstance implements AsyncDisposable {
}

async onBTSScriptsLoaded(url: string) {
const btsUrls = templateManager.getTemplate(url)
const btsUrls = templateManager.getBundle(url)
?.backgroundCode as Record<
string,
string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ const wasm = import(
);

export class TemplateManager {
readonly #templates: Map<string, DecodedTemplate> = new Map();
readonly #bundles: Map<string, DecodedTemplate> = new Map();
readonly #loadingBundles: Map<string, DecodedTemplate> = new Map();
readonly #loadingPromises: Map<string, Promise<void>> = new Map();
readonly #lynxViewInstancesMap: Map<
string,
Promise<LynxViewInstance>
Expand All @@ -49,26 +51,39 @@ export class TemplateManager {
transformVH: boolean,
overrideConfig?: Record<string, string>,
): Promise<void> {
if (this.#templates.has(url) && !overrideConfig) {
if (this.#bundles.has(url) && !overrideConfig) {
return (async () => {
const template = this.#templates.get(url);
const config = (template?.config || {}) as PageConfig;
const bundle = this.#bundles.get(url);
const config = (bundle?.config || {}) as PageConfig;
const lynxViewInstance = await lynxViewInstancePromise;
lynxViewInstance.backgroundThread.markTiming('decode_start');
lynxViewInstance.onPageConfigReady(config);
lynxViewInstance.onStyleInfoReady(url);
lynxViewInstance.onMTSScriptsLoaded(url, config.isLazy === 'true');
lynxViewInstance.onBTSScriptsLoaded(url);
})();
} else if (this.#loadingPromises.has(url)) {
return this.#loadingPromises.get(url)!.then(async () => {
const bundle = this.#bundles.get(url);
const config = (bundle?.config || {}) as PageConfig;
const lynxViewInstance = await lynxViewInstancePromise;
lynxViewInstance.backgroundThread.markTiming('decode_start');
lynxViewInstance.onPageConfigReady(config);
lynxViewInstance.onStyleInfoReady(url);
lynxViewInstance.onMTSScriptsLoaded(url, config.isLazy === 'true');
lynxViewInstance.onBTSScriptsLoaded(url);
});
} else {
this.createTemplate(url);
return this.#load(
this.createBundle(url);
const promise = this.#load(
url,
lynxViewInstancePromise,
transformVW,
transformVH,
overrideConfig,
);
this.#loadingPromises.set(url, promise);
return promise;
}
}

Expand Down Expand Up @@ -100,7 +115,7 @@ export class TemplateManager {
overrideConfig,
};
this.#worker!.postMessage(msg);
return new Promise((resolve, reject) => {
return new Promise<void>((resolve, reject) => {
this.#pendingResolves.set(url, { resolve, reject });
});
}
Expand Down Expand Up @@ -174,20 +189,27 @@ export class TemplateManager {
this.#handleSection(msg, lynxViewInstancePromise);
break;
case 'error':
console.error(`Error decoding template ${url}:`, msg.error);
console.error(`Error decoding bundle ${url}:`, msg.error);
this.#cleanup(url);
this.#removeTemplate(url);
this.#removeBundle(url);
this.#rejectPromise(url, new Error(msg.error));
this.#loadingPromises.delete(url);
break;
case 'done':
this.#cleanup(url);
const bundle = this.#loadingBundles.get(url);
if (bundle) {
this.#bundles.set(url, bundle);
this.#loadingBundles.delete(url);
}
this.#resolvePromise(url);
this.#loadingPromises.delete(url);
/* TODO: The promise resolution is deferred inside .then() without error handling.
*
*/
lynxViewInstancePromise.then((instance) => {
instance.backgroundThread.markTiming('decode_end');
instance.backgroundThread.markTiming('load_template_start');
this.#resolvePromise(url);
});
break;
}
Expand Down Expand Up @@ -217,9 +239,9 @@ export class TemplateManager {
new Uint8Array(data as ArrayBuffer),
document,
);
const template = this.#templates.get(url);
if (template) {
template.styleSheet = resource;
const bundle = this.#loadingBundles.get(url);
if (bundle) {
bundle.styleSheet = resource;
}
instance.onStyleInfoReady(url);
break;
Expand Down Expand Up @@ -250,72 +272,90 @@ export class TemplateManager {
this.#lynxViewInstancesMap.delete(url);
}

createTemplate(url: string) {
if (this.#templates.has(url)) {
// remove the template and revoke URLs
const template = this.#templates.get(url);
if (template) {
if (template.lepusCode) {
for (const blobUrl of Object.values(template.lepusCode)) {
createBundle(url: string) {
if (this.#bundles.has(url)) {
const bundle = this.#bundles.get(url);
if (bundle) {
if (bundle.lepusCode) {
for (const blobUrl of Object.values(bundle.lepusCode)) {
URL.revokeObjectURL(blobUrl);
}
}
if (bundle.backgroundCode) {
for (const blobUrl of Object.values(bundle.backgroundCode)) {
URL.revokeObjectURL(blobUrl);
}
}
if (bundle.styleSheet) {
bundle.styleSheet.free();
}
}
this.#bundles.delete(url);
}
if (this.#loadingBundles.has(url)) {
const bundle = this.#loadingBundles.get(url);
if (bundle) {
if (bundle.lepusCode) {
for (const blobUrl of Object.values(bundle.lepusCode)) {
URL.revokeObjectURL(blobUrl);
}
}
if (template.backgroundCode) {
for (const blobUrl of Object.values(template.backgroundCode)) {
if (bundle.backgroundCode) {
for (const blobUrl of Object.values(bundle.backgroundCode)) {
URL.revokeObjectURL(blobUrl);
}
}
if (template.styleSheet) {
template.styleSheet.free();
if (bundle.styleSheet) {
bundle.styleSheet.free();
}
}
this.#templates.delete(url);
this.#loadingBundles.delete(url);
}
this.#templates.set(url, {});
this.#loadingBundles.set(url, {});
}

#removeTemplate(url: string) {
this.createTemplate(url); // This actually clears it in current logic
this.#templates.delete(url);
#removeBundle(url: string) {
this.createBundle(url); // This actually clears it in current logic
this.#loadingBundles.delete(url);
}

#setConfig(url: string, config: PageConfig) {
const template = this.#templates.get(url);
if (template) {
template.config = config;
const bundle = this.#loadingBundles.get(url);
if (bundle) {
bundle.config = config;
}
}

#setLepusCode(url: string, lepusCode: Record<string, string>) {
const template = this.#templates.get(url);
if (template) {
template.lepusCode = lepusCode;
const bundle = this.#loadingBundles.get(url);
if (bundle) {
bundle.lepusCode = lepusCode;
}
}

#setCustomSection(url: string, customSections: Record<string, any>) {
const template = this.#templates.get(url);
if (template) {
template.customSections = customSections;
const bundle = this.#loadingBundles.get(url);
if (bundle) {
bundle.customSections = customSections;
}
}

#setBackgroundCode(
url: string,
backgroundCode: Record<string, string>,
) {
const template = this.#templates.get(url);
if (template) {
template.backgroundCode = backgroundCode;
const bundle = this.#loadingBundles.get(url);
if (bundle) {
bundle.backgroundCode = backgroundCode;
}
}

public getTemplate(url: string): DecodedTemplate | undefined {
return this.#templates.get(url);
public getBundle(url: string): DecodedTemplate | undefined {
return this.#bundles.get(url) || this.#loadingBundles.get(url);
}

public getStyleSheet(url: string): any {
return this.#templates.get(url)?.styleSheet;
return this.getBundle(url)?.styleSheet;
}
}

Expand Down
Loading
Loading