Skip to content

Commit

Permalink
Fixes #2961 (#2965)
Browse files Browse the repository at this point in the history
In the bundler target, there's a circular dependency between the bindings and the wasm module. That means that the wasm module's exports aren't available at the top level. In #2886, I didn't realise that and made the memory views be initialised at the top level, which resulted in an error from the wasm module's memory not being available yet.

This fixes that by lazily initialising the memory views like they were before #2886, except that they're reset to uninitialised in `init` to make sure they're updated if it's called multiple times (the reason I made them be immediately initialised in the first place).
  • Loading branch information
Liamolucko committed Jun 27, 2022
1 parent d4b372a commit 7f4663b
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 57 deletions.
64 changes: 23 additions & 41 deletions crates/cli-support/src/js/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use std::fmt;
use std::fmt::Write;
use std::fs;
use std::path::{Path, PathBuf};
use walrus::{ExportItem, FunctionId, ImportId, MemoryId, Module, TableId, ValType};
use walrus::{FunctionId, ImportId, MemoryId, Module, TableId, ValType};

mod binding;

Expand Down Expand Up @@ -403,8 +403,6 @@ impl<'a> Context<'a> {
))),
);

footer.push_str(&self.post_instantiate());

if needs_manual_start {
footer.push_str("\nwasm.__wbindgen_start();\n");
}
Expand All @@ -418,7 +416,6 @@ impl<'a> Context<'a> {
footer.push_str(&self.generate_deno_wasm_loading(module_name));

footer.push_str("\n\n");
footer.push_str(&self.post_instantiate());

if needs_manual_start {
footer.push_str("\nwasm.__wbindgen_start();\n");
Expand Down Expand Up @@ -454,9 +451,6 @@ impl<'a> Context<'a> {
}
}

footer.push('\n');
footer.push_str(&self.post_instantiate());

if needs_manual_start {
start = Some("\nwasm.__wbindgen_start();\n".to_string());
}
Expand Down Expand Up @@ -587,37 +581,6 @@ impl<'a> Context<'a> {
Ok(imports)
}

/// Returns JS to be run immediately after the wasm module is instantiated,
/// before the start function is called.
fn post_instantiate(&self) -> String {
let mut out = String::new();
// Initialise all the memory views.
for (&mem_id, &(num, ref views)) in &self.memories {
// We can't just use `export_name_of` because it takes `&mut self` and we've already borrowed `views`.
let mem = match self
.module
.exports
.iter()
.find(|export| matches!(export.item, ExportItem::Memory(id) if id == mem_id))
{
Some(export) => &export.name,
None => continue,
};

for kind in views {
writeln!(
out,
"cached{kind}Memory{num} = new {kind}Array(wasm.{mem}.buffer);",
kind = kind,
num = num,
mem = mem,
)
.unwrap()
}
}
out
}

fn ts_for_init_fn(
&self,
has_memory: bool,
Expand Down Expand Up @@ -797,6 +760,22 @@ impl<'a> Context<'a> {
imports_init.push_str(&format!("imports['{}'] = __wbg_star{};\n", extra, i));
}

let mut init_memviews = String::new();
for &(num, ref views) in self.memories.values() {
for kind in views {
writeln!(
init_memviews,
// Reset the memory views to empty in case `init` gets called multiple times.
// Without this, the `length = 0` check would never detect that the view was
// outdated.
"cached{kind}Memory{num} = new {kind}Array();",
kind = kind,
num = num,
)
.unwrap()
}
}

let js = format!(
"\
async function load(module, imports) {{
Expand Down Expand Up @@ -847,7 +826,7 @@ impl<'a> Context<'a> {
function finalizeInit(instance, module) {{
wasm = instance.exports;
init.__wbindgen_wasm_module = module;
{post_instantiate}
{init_memviews}
{start}
return wasm;
}}
Expand Down Expand Up @@ -881,7 +860,7 @@ impl<'a> Context<'a> {
init_memory_arg = init_memory_arg,
default_module_path = default_module_path,
init_memory = init_memory,
post_instantiate = self.post_instantiate(),
init_memviews = init_memviews,
start = if needs_manual_start {
"wasm.__wbindgen_start();"
} else {
Expand Down Expand Up @@ -1749,9 +1728,12 @@ impl<'a> Context<'a> {
format!("{cache}.byteLength === 0", cache = cache)
};

// Initialize the cache to an empty array, which will trigger the resized check
// on the first call and initialise the view.
self.global(&format!("let {cache} = new {kind}Array();\n"));

self.global(&format!(
"
let {cache};
function {name}() {{
if ({resized_check}) {{
{cache} = new {kind}Array(wasm.{mem}.buffer);
Expand Down
9 changes: 4 additions & 5 deletions crates/cli/tests/reference/anyref-import-catch.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ let cachedTextDecoder = new lTextDecoder('utf-8', { ignoreBOM: true, fatal: true

cachedTextDecoder.decode();

let cachedUint8Memory0;
let cachedUint8Memory0 = new Uint8Array();

function getUint8Memory0() {
if (cachedUint8Memory0.byteLength === 0) {
cachedUint8Memory0 = new Uint8Array(wasm.memory.buffer);
Expand All @@ -33,7 +34,8 @@ function handleError(f, args) {
}
}

let cachedInt32Memory0;
let cachedInt32Memory0 = new Int32Array();

function getInt32Memory0() {
if (cachedInt32Memory0.byteLength === 0) {
cachedInt32Memory0 = new Int32Array(wasm.memory.buffer);
Expand Down Expand Up @@ -81,6 +83,3 @@ export function __wbindgen_init_externref_table() {
;
};

cachedInt32Memory0 = new Int32Array(wasm.memory.buffer);
cachedUint8Memory0 = new Uint8Array(wasm.memory.buffer);

5 changes: 2 additions & 3 deletions crates/cli/tests/reference/import-catch.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ function handleError(f, args) {
}
}

let cachedInt32Memory0;
let cachedInt32Memory0 = new Int32Array();

function getInt32Memory0() {
if (cachedInt32Memory0.byteLength === 0) {
cachedInt32Memory0 = new Int32Array(wasm.memory.buffer);
Expand Down Expand Up @@ -64,5 +65,3 @@ export function __wbg_foo_8d66ddef0ff279d6() { return handleError(function () {
foo();
}, arguments) };

cachedInt32Memory0 = new Int32Array(wasm.memory.buffer);

9 changes: 4 additions & 5 deletions crates/cli/tests/reference/result-string.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ function addHeapObject(obj) {
return idx;
}

let cachedInt32Memory0;
let cachedInt32Memory0 = new Int32Array();

function getInt32Memory0() {
if (cachedInt32Memory0.byteLength === 0) {
cachedInt32Memory0 = new Int32Array(wasm.memory.buffer);
Expand Down Expand Up @@ -43,7 +44,8 @@ let cachedTextDecoder = new lTextDecoder('utf-8', { ignoreBOM: true, fatal: true

cachedTextDecoder.decode();

let cachedUint8Memory0;
let cachedUint8Memory0 = new Uint8Array();

function getUint8Memory0() {
if (cachedUint8Memory0.byteLength === 0) {
cachedUint8Memory0 = new Uint8Array(wasm.memory.buffer);
Expand Down Expand Up @@ -83,6 +85,3 @@ export function __wbindgen_number_new(arg0) {
return addHeapObject(ret);
};

cachedInt32Memory0 = new Int32Array(wasm.memory.buffer);
cachedUint8Memory0 = new Uint8Array(wasm.memory.buffer);

5 changes: 2 additions & 3 deletions crates/cli/tests/reference/string-arg.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ let cachedTextDecoder = new lTextDecoder('utf-8', { ignoreBOM: true, fatal: true

cachedTextDecoder.decode();

let cachedUint8Memory0;
let cachedUint8Memory0 = new Uint8Array();

function getUint8Memory0() {
if (cachedUint8Memory0.byteLength === 0) {
cachedUint8Memory0 = new Uint8Array(wasm.memory.buffer);
Expand Down Expand Up @@ -87,5 +88,3 @@ export function __wbindgen_throw(arg0, arg1) {
throw new Error(getStringFromWasm0(arg0, arg1));
};

cachedUint8Memory0 = new Uint8Array(wasm.memory.buffer);

0 comments on commit 7f4663b

Please sign in to comment.