Skip to content

Replace incorrect wasm compilation fallback logic. Fixes #42055 partly #42918

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
SteveSandersonMS merged 1 commit into from
Jul 29, 2022
Merged

Replace incorrect wasm compilation fallback logic. Fixes #42055 partly #42918

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 19 additions & 15 deletions src/Components/Web.JS/src/Platform/Mono/MonoPlatform.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -652,24 +652,28 @@ async function loadICUData(icuDataResource: LoadingResource): Promise<void> {
}

async function compileWasmModule(wasmResource: LoadingResource, imports: any): Promise<WebAssembly.Instance> {
// This is the same logic as used in emscripten's generated js. We can't use emscripten's js because
// it doesn't provide any method for supplying a custom response provider, and we want to integrate
// with our resource loader cache.
const wasmResourceResponse = await wasmResource.response;

if (typeof WebAssembly['instantiateStreaming'] === 'function') {
try {
const streamingResult = await WebAssembly['instantiateStreaming'](wasmResource.response, imports);
return streamingResult.instance;
} catch (ex) {
console.info('Streaming compilation failed. Falling back to ArrayBuffer instantiation. ', ex);
// The instantiateStreaming spec explicitly requires the following exact MIME type (with no trailing parameters, etc.)
// https://webassembly.github.io/spec/web-api/#dom-webassembly-instantiatestreaming
const hasWasmContentType = wasmResourceResponse.headers?.get('content-type') === 'application/wasm';

if (hasWasmContentType && typeof WebAssembly.instantiateStreaming === 'function') {
// We can use streaming compilation. We know this shouldn't fail due to the content-type header being wrong,
// as we already just checked that. So if this fails for some other reason we'll treat it as fatal.
const streamingResult = await WebAssembly.instantiateStreaming(wasmResourceResponse, imports);
return streamingResult.instance;
} else {
if (!hasWasmContentType) {
// In most cases the developer should fix this. It's unusual enough that we don't mind logging a warning each time.
console.warn('WebAssembly resource does not have the expected content type "application/wasm", so falling back to slower ArrayBuffer instantiation.');
}
Comment on lines +667 to 670
Copy link
Contributor

@TanayParikh TanayParikh Jul 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we only attempt to use instantiateStreaming when the content-type application/wasm is found (as per spec), then it should work, and if not there's no reason to think that instantiate would be more successful.

In that case, should we just throw an exception given we don't expect the subsequent array buffer instantiation to work if !hasWasmContentType?

Copy link
Member Author

@SteveSandersonMS SteveSandersonMS Jul 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-streaming compilation should work regardless of the content type. It’s only streaming compilation that has the content type prerequisite.

Copy link
Member Author

@SteveSandersonMS SteveSandersonMS Jul 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the way I phrased the PR description was ambiguous and perhaps misleading. Hopefully the code itself does a better job of explaining!

}

// If that's not available or fails (e.g., due to incorrect content-type header),
// fall back to ArrayBuffer instantiation
const arrayBuffer = await wasmResource.response.then(r => r.arrayBuffer());
const arrayBufferResult = await WebAssembly.instantiate(arrayBuffer, imports);
return arrayBufferResult.instance;
// Fall back on ArrayBuffer instantiation.
const arrayBuffer = await wasmResourceResponse.arrayBuffer();
const arrayBufferResult = await WebAssembly.instantiate(arrayBuffer, imports);
return arrayBufferResult.instance;
}
}

function changeExtension(filename: string, newExtensionWithLeadingDot: string) {
Expand Down