0
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2025-02-20 20:33:42 -05:00

chore(wasm): Don't await on the argument to handleWasmStreaming (#14000)

`handleWasmStreaming` is the function that provides the binding with
the `fetch` API needed for `WebAssembly.instantiateStreaming()` and
`WebAssembly.compileStreaming()`. When I implemented it in #11200, I
thought V8 was calling these functions with the argument of the
`WebAssembly` streaming functions, without doing any resolving, and so
`handleWasmStreaming` awaits for the parameter to resolve. However,
as discovered in
https://github.com/denoland/deno/issues/13917#issuecomment-1065805565,
V8 does in fact resolve the parameter if it's a promise (and handles
rejections arising from that).

This change removes the `async` IIFE inside `handleWasmStreaming`,
letting initial errors be handled synchronously (which will however
not throw synchronously from the `WebAssembly` namespace functions).
Awaiting is still necessary for reading the bytes of the response,
though, and so there is an `async` IIFE for that.
This commit is contained in:
Andreu Botella 2022-03-29 14:44:33 +02:00 committed by David Sherret
parent cdb18a8965
commit 542d9e7620
3 changed files with 50 additions and 46 deletions

View file

@ -1,2 +1,2 @@
error: Uncaught (in promise) TypeError: Invalid WebAssembly content type. error: Uncaught (in promise) TypeError: Invalid WebAssembly content type.
at deno:ext/fetch/26_fetch.js:[WILDCARD] at handleWasmStreaming (deno:ext/fetch/26_fetch.js:[WILDCARD])

View file

@ -45,11 +45,11 @@ Deno.test(
Deno.test( Deno.test(
async function wasmInstantiateStreamingNoContentType() { async function wasmInstantiateStreamingNoContentType() {
const response = new Response(simpleWasm);
// Rejects, not throws.
const wasmPromise = WebAssembly.instantiateStreaming(response);
await assertRejects( await assertRejects(
async () => { () => wasmPromise,
const response = Promise.resolve(new Response(simpleWasm));
await WebAssembly.instantiateStreaming(response);
},
TypeError, TypeError,
"Invalid WebAssembly content type.", "Invalid WebAssembly content type.",
); );

View file

@ -510,31 +510,29 @@
} }
/** /**
* Handle the Promise<Response> argument to the WebAssembly streaming * Handle the Response argument to the WebAssembly streaming APIs, after
* APIs. This function should be registered through * resolving if it was passed as a promise. This function should be registered
* `Deno.core.setWasmStreamingCallback`. * through `Deno.core.setWasmStreamingCallback`.
* *
* @param {any} source The source parameter that the WebAssembly * @param {any} source The source parameter that the WebAssembly streaming API
* streaming API was called with. * was called with. If it was called with a Promise, `source` is the resolved
* @param {number} rid An rid that represents the wasm streaming * value of that promise.
* resource. * @param {number} rid An rid that represents the wasm streaming resource.
*/ */
function handleWasmStreaming(source, rid) { function handleWasmStreaming(source, rid) {
// This implements part of // This implements part of
// https://webassembly.github.io/spec/web-api/#compile-a-potential-webassembly-response // https://webassembly.github.io/spec/web-api/#compile-a-potential-webassembly-response
(async () => {
try { try {
const res = webidl.converters["Response"](await source, { const res = webidl.converters["Response"](source, {
prefix: "Failed to call 'WebAssembly.compileStreaming'", prefix: "Failed to call 'WebAssembly.compileStreaming'",
context: "Argument 1", context: "Argument 1",
}); });
// 2.3. // 2.3.
// The spec is ambiguous here, see // The spec is ambiguous here, see
// https://github.com/WebAssembly/spec/issues/1138. The WPT tests // https://github.com/WebAssembly/spec/issues/1138. The WPT tests expect
// expect the raw value of the Content-Type attribute lowercased. // the raw value of the Content-Type attribute lowercased. We ignore this
// We ignore this for file:// because file fetches don't have a // for file:// because file fetches don't have a Content-Type.
// Content-Type.
if (!StringPrototypeStartsWith(res.url, "file://")) { if (!StringPrototypeStartsWith(res.url, "file://")) {
const contentType = res.headers.get("Content-Type"); const contentType = res.headers.get("Content-Type");
if ( if (
@ -553,25 +551,31 @@
// Pass the resolved URL to v8. // Pass the resolved URL to v8.
core.opSync("op_wasm_streaming_set_url", rid, res.url); core.opSync("op_wasm_streaming_set_url", rid, res.url);
if (res.body !== null) {
// 2.6. // 2.6.
// Rather than consuming the body as an ArrayBuffer, this passes each // Rather than consuming the body as an ArrayBuffer, this passes each
// chunk to the feed as soon as it's available. // chunk to the feed as soon as it's available.
if (res.body !== null) { (async () => {
const reader = res.body.getReader(); const reader = res.body.getReader();
while (true) { while (true) {
const { value: chunk, done } = await reader.read(); const { value: chunk, done } = await reader.read();
if (done) break; if (done) break;
core.opSync("op_wasm_streaming_feed", rid, chunk); core.opSync("op_wasm_streaming_feed", rid, chunk);
} }
} })().then(
// 2.7
// 2.7. () => core.close(rid),
// 2.8
(err) => core.abortWasmStreaming(rid, err),
);
} else {
// 2.7
core.close(rid); core.close(rid);
}
} catch (err) { } catch (err) {
// 2.8 and 3 // 2.8
core.abortWasmStreaming(rid, err); core.abortWasmStreaming(rid, err);
} }
})();
} }
window.__bootstrap.fetch ??= {}; window.__bootstrap.fetch ??= {};