diff --git a/lib/api/src/js/externals/table.rs b/lib/api/src/js/externals/table.rs index c16fee03230..f7c2cdda4e6 100644 --- a/lib/api/src/js/externals/table.rs +++ b/lib/api/src/js/externals/table.rs @@ -24,7 +24,7 @@ fn get_function(store: &mut impl AsStoreMut, val: Value) -> Result Ok(func.0.handle.function.clone().into()), + Value::FuncRef(Some(ref func)) => Ok(func.0.handle.function.clone().into_inner()), // Only funcrefs is supported by the spec atm _ => unimplemented!("The {val:?} is not yet supported"), } diff --git a/lib/api/src/js/instance.rs b/lib/api/src/js/instance.rs index 30ff0b89d9b..49726418d4c 100644 --- a/lib/api/src/js/instance.rs +++ b/lib/api/src/js/instance.rs @@ -1,4 +1,3 @@ -use crate::errors::InstantiationError; use crate::exports::Exports; use crate::imports::Imports; use crate::js::as_js::AsJs; @@ -6,11 +5,12 @@ use crate::js::vm::VMInstance; use crate::module::Module; use crate::store::AsStoreMut; use crate::Extern; +use crate::{errors::InstantiationError, js::js_handle::JsHandle}; use js_sys::WebAssembly; #[derive(Clone, PartialEq, Eq)] pub struct Instance { - pub(crate) _handle: VMInstance, + pub(crate) _handle: JsHandle, } // Instance can't be Send in js because it dosen't support `structuredClone` @@ -67,6 +67,10 @@ impl Instance { }) .collect::>()?; - Ok((Self { _handle: instance }, exports)) + let instance = Instance { + _handle: JsHandle::new(instance), + }; + + Ok((instance, exports)) } } diff --git a/lib/api/src/js/js_handle.rs b/lib/api/src/js/js_handle.rs new file mode 100644 index 00000000000..9a826a74823 --- /dev/null +++ b/lib/api/src/js/js_handle.rs @@ -0,0 +1,217 @@ +use std::ops::{Deref, DerefMut}; + +use wasm_bindgen::JsValue; + +use self::integrity_check::IntegrityCheck; + +/// A handle that lets you detect thread-safety issues when passing a +/// [`JsValue`] (or derived type) around. +#[derive(Debug, Clone)] +pub(crate) struct JsHandle { + value: T, + integrity: IntegrityCheck, +} + +impl JsHandle { + #[track_caller] + pub fn new(value: T) -> Self { + JsHandle { + value, + integrity: IntegrityCheck::new(std::any::type_name::()), + } + } + + #[track_caller] + pub fn into_inner(self) -> T { + self.integrity.check(); + self.value + } +} + +impl PartialEq for JsHandle { + fn eq(&self, other: &Self) -> bool { + let JsHandle { + value, + integrity: _, + } = self; + + *value == other.value + } +} + +impl Eq for JsHandle {} + +impl From for JsHandle { + #[track_caller] + fn from(value: T) -> Self { + JsHandle::new(value) + } +} + +impl> From> for JsValue { + fn from(value: JsHandle) -> Self { + value.into_inner().into() + } +} + +impl AsRef for JsHandle +where + T: AsRef, +{ + #[track_caller] + fn as_ref(&self) -> &A { + self.integrity.check(); + self.value.as_ref() + } +} + +impl Deref for JsHandle { + type Target = T; + + #[track_caller] + fn deref(&self) -> &Self::Target { + self.integrity.check(); + &self.value + } +} + +impl DerefMut for JsHandle { + #[track_caller] + fn deref_mut(&mut self) -> &mut Self::Target { + self.integrity.check(); + &mut self.value + } +} + +#[cfg(not(debug_assertions))] +mod integrity_check { + + #[derive(Debug, Clone, PartialEq)] + pub(crate) struct IntegrityCheck; + + impl IntegrityCheck { + #[track_caller] + pub(crate) fn new(_type_name: &'static str) -> Self { + IntegrityCheck + } + + pub(crate) fn check(&self) {} + } +} + +#[cfg(debug_assertions)] +mod integrity_check { + use std::{ + fmt::Write as _, + panic::Location, + sync::atomic::{AtomicU32, Ordering}, + }; + + use js_sys::{JsString, Symbol}; + use wasm_bindgen::JsValue; + + #[derive(Debug, Clone, PartialEq)] + pub(crate) struct IntegrityCheck { + original_thread: u32, + created: &'static Location<'static>, + type_name: &'static str, + backtrace: Option, + } + + impl IntegrityCheck { + #[track_caller] + pub(crate) fn new(type_name: &'static str) -> Self { + IntegrityCheck { + original_thread: current_thread_id(), + created: Location::caller(), + type_name, + backtrace: record_backtrace(), + } + } + + #[track_caller] + pub(crate) fn check(&self) { + let current_thread = current_thread_id(); + + if current_thread != self.original_thread { + let IntegrityCheck { + original_thread, + created, + type_name, + backtrace, + } = self; + let mut error_message = String::new(); + + writeln!( + error_message, + "Thread-safety integrity check for {type_name} failed." + ) + .unwrap(); + + writeln!( + error_message, + "Created at {created} on thread #{original_thread}" + ) + .unwrap(); + + if let Some(bt) = backtrace { + writeln!(error_message, "{bt}").unwrap(); + writeln!(error_message).unwrap(); + } + + let caller = Location::caller(); + + writeln!( + error_message, + "Accessed from {caller} on thread #{current_thread}" + ) + .unwrap(); + + if let Some(bt) = record_backtrace() { + writeln!(error_message, "{bt}").unwrap(); + writeln!(error_message).unwrap(); + } + + panic!("{error_message}"); + } + } + } + + /// Get a unique ID for the current "thread" (i.e. web worker or the main + /// thread). + /// + /// This works by creating a `$WASMER_THREAD_ID` symbol and setting it on + /// the global object. + fn current_thread_id() -> u32 { + static NEXT_ID: AtomicU32 = AtomicU32::new(0); + + let global = js_sys::global(); + let thread_id_symbol = Symbol::for_("$WASMER_THREAD_ID"); + + if let Some(v) = js_sys::Reflect::get(&global, &thread_id_symbol) + .ok() + .and_then(|v| v.as_f64()) + { + // Note: we use a symbol so we know for sure that nobody else created + // this field. + return v as u32; + } + + // Looks like we haven't set the thread ID yet. + let id = NEXT_ID.fetch_add(1, Ordering::Relaxed); + + js_sys::Reflect::set(&global, &thread_id_symbol, &JsValue::from(id)) + .expect("Setting a field on the global object should never fail"); + + id + } + + fn record_backtrace() -> Option { + let err = js_sys::Error::new(""); + let stack = JsString::from(wasm_bindgen::intern("stack")); + + js_sys::Reflect::get(&err, &stack) + .ok() + .and_then(|v| v.as_string()) + } +} diff --git a/lib/api/src/js/mod.rs b/lib/api/src/js/mod.rs index fe10e935af0..1ecc4b4c0fe 100644 --- a/lib/api/src/js/mod.rs +++ b/lib/api/src/js/mod.rs @@ -29,6 +29,7 @@ pub(crate) mod errors; pub(crate) mod extern_ref; pub(crate) mod externals; pub(crate) mod instance; +mod js_handle; pub(crate) mod mem_access; pub(crate) mod module; #[cfg(feature = "wasm-types-polyfill")] diff --git a/lib/api/src/js/module.rs b/lib/api/src/js/module.rs index 1a202def78f..2b5b2c1b0f5 100644 --- a/lib/api/src/js/module.rs +++ b/lib/api/src/js/module.rs @@ -1,4 +1,3 @@ -use crate::errors::InstantiationError; use crate::errors::RuntimeError; use crate::imports::Imports; use crate::js::AsJs; @@ -6,6 +5,7 @@ use crate::store::AsStoreMut; use crate::vm::VMInstance; use crate::Extern; use crate::IntoBytes; +use crate::{errors::InstantiationError, js::js_handle::JsHandle}; use crate::{AsEngineRef, ExportType, ImportType}; use bytes::Bytes; use js_sys::{Reflect, Uint8Array, WebAssembly}; @@ -37,7 +37,7 @@ pub struct ModuleTypeHints { #[derive(Clone, PartialEq, Eq)] pub struct Module { - module: WebAssembly::Module, + module: JsHandle, name: Option, // WebAssembly type hints type_hints: Option, @@ -81,12 +81,12 @@ impl Module { } /// Creates a new WebAssembly module skipping any kind of validation from a javascript module - /// pub(crate) unsafe fn from_js_module( module: WebAssembly::Module, binary: impl IntoBytes, ) -> Self { let binary = binary.into_bytes(); + // The module is now validated, so we can safely parse it's types #[cfg(feature = "wasm-types-polyfill")] let (type_hints, name) = { @@ -112,11 +112,11 @@ impl Module { let (type_hints, name) = (None, None); Self { - module, + module: JsHandle::new(module), type_hints, name, #[cfg(feature = "js-serializable-module")] - raw_bytes: Some(binary.into_bytes()), + raw_bytes: Some(binary), } } @@ -451,9 +451,10 @@ impl Module { } impl From for Module { + #[track_caller] fn from(module: WebAssembly::Module) -> Module { Module { - module, + module: JsHandle::new(module), name: None, type_hints: None, #[cfg(feature = "js-serializable-module")] diff --git a/lib/api/src/js/store.rs b/lib/api/src/js/store.rs index 86147f0e0f8..bd2ad83a207 100644 --- a/lib/api/src/js/store.rs +++ b/lib/api/src/js/store.rs @@ -2,10 +2,12 @@ pub(crate) use objects::{InternalStoreHandle, StoreObject}; pub use objects::{StoreHandle, StoreObjects}; mod objects { + use std::{fmt, marker::PhantomData, num::NonZeroUsize}; + use wasm_bindgen::JsValue; use crate::js::vm::{VMFunctionEnvironment, VMGlobal}; - use std::{fmt, marker::PhantomData, num::NonZeroUsize}; + pub use wasmer_types::StoreId; /// Trait to represent an object managed by a context. This is implemented on diff --git a/lib/api/src/js/vm.rs b/lib/api/src/js/vm.rs index b7cb94093d2..35a1a003157 100644 --- a/lib/api/src/js/vm.rs +++ b/lib/api/src/js/vm.rs @@ -4,7 +4,7 @@ /// This module should not be needed any longer (with the exception of the memory) /// once the type reflection is added to the WebAssembly JS API. /// https://github.com/WebAssembly/js-types/ -use crate::js::wasm_bindgen_polyfill::Global as JsGlobal; +use crate::js::{js_handle::JsHandle, wasm_bindgen_polyfill::Global as JsGlobal}; use js_sys::Function as JsFunction; use js_sys::WebAssembly; use js_sys::WebAssembly::{Memory as JsMemory, Table as JsTable}; @@ -22,7 +22,7 @@ use wasmer_types::{ /// Represents linear memory that is managed by the javascript runtime #[derive(Clone, Debug, PartialEq)] pub struct VMMemory { - pub(crate) memory: JsMemory, + pub(crate) memory: JsHandle, pub(crate) ty: MemoryType, } @@ -38,7 +38,10 @@ struct DummyBuffer { impl VMMemory { /// Creates a new memory directly from a WebAssembly javascript object pub fn new(memory: JsMemory, ty: MemoryType) -> Self { - Self { memory, ty } + Self { + memory: JsHandle::new(memory), + ty, + } } /// Returns the size of the memory buffer in pages @@ -106,7 +109,7 @@ impl VMMemory { trace!("memory copy finished (size={})", dst.size().bytes().0); Ok(Self { - memory: new_memory, + memory: JsHandle::new(new_memory), ty: self.ty.clone(), }) } @@ -167,7 +170,7 @@ impl VMTable { /// The VM Function type #[derive(Clone)] pub struct VMFunction { - pub(crate) function: JsFunction, + pub(crate) function: JsHandle, pub(crate) ty: FunctionType, } @@ -176,7 +179,10 @@ unsafe impl Sync for VMFunction {} impl VMFunction { pub(crate) fn new(function: JsFunction, ty: FunctionType) -> Self { - Self { function, ty } + Self { + function: JsHandle::new(function), + ty, + } } } diff --git a/tests/wasmer-web/src/lib.rs b/tests/wasmer-web/src/lib.rs index 49a1aef92ba..fa7fbf25faa 100644 --- a/tests/wasmer-web/src/lib.rs +++ b/tests/wasmer-web/src/lib.rs @@ -405,24 +405,39 @@ impl ClientExt for Client { prompt: &str, timeout: Duration, ) -> String { - let previous_output = self.read_terminal().await.unwrap(); + let previous_output = self + .read_terminal() + .await + .expect("Unable to read the terminal"); let stdin = self .find(fantoccini::Locator::Css("textarea.xterm-helper-textarea")) .await - .unwrap(); + .expect("Unable to find the xterm textarea"); - stdin.send_keys(cmd).await.unwrap(); - stdin.send_keys("\n").await.unwrap(); + stdin + .send_keys(cmd) + .await + .expect("Unable to send the command"); + stdin + .send_keys("\n") + .await + .expect("Unable to send a newline"); let terminal_contents = self .wait_for_xterm_with_timeout( timeout, predicates::function::function(|s: &str| { // First, trim away anything before/including the previous output - let (_, new_content) = s.split_once(&previous_output).unwrap(); + let Some((_, new_content)) = s.split_once(&previous_output) else { + return false; + }; + // Now, we want to get the content after the command - let (_before, after) = new_content.split_once(cmd).unwrap(); + let Some((_before, after)) = new_content.split_once(cmd) else { + return false; + }; + after.trim_start_matches('\n').contains(prompt) }), ) @@ -450,7 +465,7 @@ fn extract_command_output<'a>(terminal: &'a str, prompt: &str) -> &'a str { Some(first_newline) => &output[first_newline + 1..], _ => output, }, - _ => todo!(), + _ => panic!("Terminal output wasn't in the expected format"), } }