From 09735ba256a92311f64998d0a9734422f9a73863 Mon Sep 17 00:00:00 2001 From: Noa Date: Tue, 28 Oct 2025 12:53:45 -0500 Subject: [PATCH 1/4] Add proper syscall versioning --- crates/core/src/host/v8/error.rs | 8 - crates/core/src/host/v8/mod.rs | 26 ++- crates/core/src/host/v8/string.rs | 4 + crates/core/src/host/v8/syscall/hooks.rs | 103 +++++++++++ crates/core/src/host/v8/syscall/mod.rs | 88 +++++++++ .../src/host/v8/{syscall.rs => syscall/v1.rs} | 173 ++++++------------ 6 files changed, 268 insertions(+), 134 deletions(-) create mode 100644 crates/core/src/host/v8/syscall/hooks.rs create mode 100644 crates/core/src/host/v8/syscall/mod.rs rename crates/core/src/host/v8/{syscall.rs => syscall/v1.rs} (90%) diff --git a/crates/core/src/host/v8/error.rs b/crates/core/src/host/v8/error.rs index ec389a48b2f..7156c29aa31 100644 --- a/crates/core/src/host/v8/error.rs +++ b/crates/core/src/host/v8/error.rs @@ -1,6 +1,5 @@ //! Utilities for error handling when dealing with V8. -use super::de::scratch_buf; use super::serialize_to_js; use super::string::IntoJsString; use crate::{ @@ -46,13 +45,6 @@ impl<'scope, M: IntoJsString> IntoException<'scope> for TypeError { } } -/// Returns a "module not found" exception to be thrown. -pub fn module_exception(scope: &mut PinScope<'_, '_>, spec: Local<'_, v8::String>) -> TypeError { - let mut buf = scratch_buf::<32>(); - let spec = spec.to_rust_cow_lossy(scope, &mut buf); - TypeError(format!("Could not find module {spec:?}")) -} - /// A type converting into a JS `RangeError` exception. #[derive(Copy, Clone)] pub struct RangeError(pub M); diff --git a/crates/core/src/host/v8/mod.rs b/crates/core/src/host/v8/mod.rs index 73b1ad9d76a..af9059be3bf 100644 --- a/crates/core/src/host/v8/mod.rs +++ b/crates/core/src/host/v8/mod.rs @@ -5,7 +5,9 @@ use self::error::{ }; use self::ser::serialize_to_js; use self::string::{str_from_ident, IntoJsString}; -use self::syscall::{call_call_reducer, call_describe_module, call_reducer_fun, resolve_sys_module, FnRet}; +use self::syscall::{ + call_call_reducer, call_describe_module, get_hook, resolve_sys_module, FnRet, HookFunction, ModuleHook, +}; use super::module_common::{build_common_module_from_raw, run_describer, ModuleCommon}; use super::module_host::{CallProcedureParams, CallReducerParams, Module, ModuleInfo, ModuleRuntime}; use super::UpdateDatabaseResult; @@ -18,6 +20,7 @@ use crate::host::{ReducerCallResult, Scheduler}; use crate::module_host_context::{ModuleCreationContext, ModuleCreationContextLimited}; use crate::replica_context::ReplicaContext; use crate::util::asyncify; +use anyhow::Context as _; use core::str; use itertools::Either; use spacetimedb_datastore::locking_tx_datastore::MutTxId; @@ -329,12 +332,13 @@ fn startup_instance_worker<'scope>( scope: &mut PinScope<'scope, '_>, program: Arc, module_or_mcc: Either, -) -> anyhow::Result<(Local<'scope, Function>, Either)> { +) -> anyhow::Result<(HookFunction<'scope>, Either)> { // Start-up the user's module. eval_user_module_catch(scope, &program).map_err(DescribeError::Setup)?; // Find the `__call_reducer__` function. - let call_reducer_fun = catch_exception(scope, |scope| Ok(call_reducer_fun(scope)?)).map_err(|(e, _)| e)?; + let call_reducer_fun = + get_hook(scope, ModuleHook::CallReducer).context("The `spacetimedb/server` module was never imported")?; // If we don't have a module, make one. let module_common = match module_or_mcc { @@ -578,7 +582,7 @@ fn call_reducer<'scope>( instance_common: &mut InstanceCommon, replica_ctx: &ReplicaContext, scope: &mut PinScope<'scope, '_>, - fun: Local<'scope, Function>, + fun: HookFunction<'_>, tx: Option, params: CallReducerParams, ) -> (super::ReducerCallResult, bool) { @@ -655,7 +659,10 @@ fn extract_description<'scope>( |a, b, c| log_traceback(replica_ctx, a, b, c), || { catch_exception(scope, |scope| { - let def = call_describe_module(scope)?; + let Some(describe_module) = get_hook(scope, ModuleHook::DescribeModule) else { + return Ok(RawModuleDef::V9(Default::default())); + }; + let def = call_describe_module(scope, describe_module)?; Ok(def) }) .map_err(|(e, _)| e) @@ -663,6 +670,7 @@ fn extract_description<'scope>( }, ) } + #[cfg(test)] mod test { use super::to_value::test::with_scope; @@ -691,7 +699,7 @@ mod test { fn call_call_reducer_works() { let call = |code| { with_module_catch(code, |scope| { - let fun = call_reducer_fun(scope)?; + let fun = get_hook(scope, ModuleHook::CallReducer).unwrap(); let op = ReducerOp { id: ReducerId(42), name: "foobar", @@ -769,7 +777,11 @@ js error Uncaught Error: foobar }, }) "#; - let raw_mod = with_module_catch(code, call_describe_module).map_err(|e| e.to_string()); + let raw_mod = with_module_catch(code, |scope| { + let describe_module = get_hook(scope, ModuleHook::DescribeModule).unwrap(); + call_describe_module(scope, describe_module) + }) + .map_err(|e| e.to_string()); assert_eq!(raw_mod, Ok(RawModuleDef::V9(<_>::default()))); } } diff --git a/crates/core/src/host/v8/string.rs b/crates/core/src/host/v8/string.rs index ca8696ac1b3..872df8c3155 100644 --- a/crates/core/src/host/v8/string.rs +++ b/crates/core/src/host/v8/string.rs @@ -36,6 +36,10 @@ impl StringConst { v8::String::new_from_onebyte_const(scope, &self.0) .expect("`create_external_onebyte_const` should've asserted `.len() < kMaxLength`") } + + pub(super) fn as_str(&'static self) -> &'static str { + self.0.as_str() + } } /// Converts an identifier to a compile-time ASCII string. diff --git a/crates/core/src/host/v8/syscall/hooks.rs b/crates/core/src/host/v8/syscall/hooks.rs new file mode 100644 index 00000000000..a12d6421ad5 --- /dev/null +++ b/crates/core/src/host/v8/syscall/hooks.rs @@ -0,0 +1,103 @@ +use std::cell::OnceCell; +use std::rc::Rc; + +use enum_map::EnumMap; +use v8::{Context, Function, Local, Object, PinScope}; + +use super::AbiVersion; +use crate::host::v8::de::property; +use crate::host::v8::error::ExcResult; +use crate::host::v8::error::Throwable; +use crate::host::v8::error::TypeError; +use crate::host::v8::from_value::cast; +use crate::host::v8::string::StringConst; + +pub(super) fn get_hook_function<'s>( + scope: &mut PinScope<'s, '_>, + hooks_obj: Local<'_, Object>, + name: &'static StringConst, +) -> ExcResult> { + let key = name.string(scope); + let object = property(scope, hooks_obj, key)?; + cast!(scope, object, Function, "module function hook `{}`", name.as_str()).map_err(|e| e.throw(scope)) +} + +pub(super) fn set_hook_slots( + scope: &mut PinScope<'_, '_>, + abi: AbiVersion, + hooks: &[(ModuleHook, Local<'_, Function>)], +) -> ExcResult<()> { + // Make sure to call `set_slot` first, as it creates the annex + // and `set_embedder_data` is currently buggy. + let ctx = scope.get_current_context(); + let hooks_info = HooksInfo::get_or_create(&ctx); + for &(hook, func) in hooks { + hooks_info + .register(hook, abi) + .map_err(|_| TypeError("cannot call register_hooks multiple times").throw(scope))?; + ctx.set_embedder_data(hook.to_slot_index(), func.into()); + } + Ok(()) +} + +#[derive(enum_map::Enum, Copy, Clone)] +pub(in crate::host::v8) enum ModuleHook { + DescribeModule, + CallReducer, +} + +impl ModuleHook { + /// Get the `v8::Context::{get,set}_embedder_data` slot that holds this hook. + fn to_slot_index(self) -> i32 { + match self { + ModuleHook::DescribeModule => 20, + ModuleHook::CallReducer => 21, + } + } +} + +#[derive(Default)] +struct HooksInfo { + abi: OnceCell, + registered: EnumMap>, +} + +impl HooksInfo { + fn get_or_create(ctx: &Context) -> Rc { + ctx.get_slot().unwrap_or_else(|| { + let this = Rc::::default(); + ctx.set_slot(this.clone()); + this + }) + } + + fn register(&self, hook: ModuleHook, abi: AbiVersion) -> Result<(), ()> { + if *self.abi.get_or_init(|| abi) != abi { + return Err(()); + } + self.registered[hook].set(()) + } + + fn get(&self, hook: ModuleHook) -> Option { + self.registered[hook].get().and(self.abi.get().copied()) + } +} + +#[derive(Copy, Clone)] +pub(in crate::host::v8) struct HookFunction<'s>(pub AbiVersion, pub Local<'s, Function>); + +/// Returns the hook function previously registered in [`register_hooks`]. +pub(in crate::host::v8) fn get_hook<'scope>( + scope: &mut PinScope<'scope, '_>, + hook: ModuleHook, +) -> Option> { + let ctx = scope.get_current_context(); + let hooks = ctx.get_slot::()?; + + let abi_version = hooks.get(hook)?; + + let hooks = ctx + .get_embedder_data(scope, hook.to_slot_index()) + .expect("if `AbiVersion` is set the hook must be set"); + Some(HookFunction(abi_version, hooks.cast())) +} diff --git a/crates/core/src/host/v8/syscall/mod.rs b/crates/core/src/host/v8/syscall/mod.rs new file mode 100644 index 00000000000..ba4884b1a5c --- /dev/null +++ b/crates/core/src/host/v8/syscall/mod.rs @@ -0,0 +1,88 @@ +use spacetimedb_lib::RawModuleDef; +use v8::{callback_scope, Context, FixedArray, Local, Module, PinScope}; + +use crate::host::v8::de::scratch_buf; +use crate::host::v8::error::ExcResult; +use crate::host::v8::error::Throwable; +use crate::host::v8::error::TypeError; +use crate::host::wasm_common::module_host_actor::{ReducerOp, ReducerResult}; + +mod hooks; +mod v1; + +pub(super) use self::hooks::{get_hook, HookFunction, ModuleHook}; + +/// The return type of a module -> host syscall. +pub(super) type FnRet<'scope> = ExcResult>; + +/// The version of the ABI that is exposed to V8. +#[derive(Copy, Clone, PartialEq, Eq)] +pub enum AbiVersion { + V1, +} + +/// A dependency resolver for the user's module +/// that will resolve `spacetimedb_sys` to a module that exposes the ABI. +pub(super) fn resolve_sys_module<'s>( + context: Local<'s, Context>, + spec: Local<'s, v8::String>, + _attrs: Local<'s, FixedArray>, + _referrer: Local<'s, Module>, +) -> Option> { + callback_scope!(unsafe scope, context); + resolve_sys_module_inner(scope, spec).ok() +} + +fn resolve_sys_module_inner<'s>( + scope: &mut PinScope<'s, '_>, + spec: Local<'s, v8::String>, +) -> ExcResult> { + let scratch = &mut scratch_buf::<32>(); + let spec = spec.to_rust_cow_lossy(scope, scratch); + + let generic_error = || TypeError(format!("Could not find module {spec:?}")); + + let (module, ver) = spec + .strip_prefix("spacetime:") + .and_then(|spec| spec.split_once('@')) + .ok_or_else(|| generic_error().throw(scope))?; + + let (maj, min) = ver + .split_once('.') + .and_then(|(maj, min)| Option::zip(maj.parse::().ok(), min.parse::().ok())) + .ok_or_else(|| TypeError(format!("Invalid version in module spec {spec:?}")).throw(scope))?; + + match module { + "sys" => match (maj, min) { + (1, 0) => Ok(v1::sys_v1_0(scope)), + _ => Err(TypeError(format!( + "Could not import {spec:?}, likely because this module was built for a newer version of SpacetimeDB.\n\ + It requires sys module v{maj}.{min}, but that version is not supported by the database." + )) + .throw(scope)), + }, + _ => Err(generic_error().throw(scope)), + } +} + +pub(super) fn call_call_reducer( + scope: &mut PinScope<'_, '_>, + fun: HookFunction<'_>, + op: ReducerOp<'_>, +) -> ExcResult { + let HookFunction(ver, fun) = fun; + match ver { + AbiVersion::V1 => v1::call_call_reducer(scope, fun, op), + } +} + +/// Calls the registered `__describe_module__` function hook. +pub(super) fn call_describe_module<'scope>( + scope: &mut PinScope<'scope, '_>, + fun: HookFunction<'_>, +) -> ExcResult { + let HookFunction(ver, fun) = fun; + match ver { + AbiVersion::V1 => v1::call_describe_module(scope, fun), + } +} diff --git a/crates/core/src/host/v8/syscall.rs b/crates/core/src/host/v8/syscall/v1.rs similarity index 90% rename from crates/core/src/host/v8/syscall.rs rename to crates/core/src/host/v8/syscall/v1.rs index 98eacd38b43..e22bd6d0a21 100644 --- a/crates/core/src/host/v8/syscall.rs +++ b/crates/core/src/host/v8/syscall/v1.rs @@ -1,17 +1,17 @@ -use std::rc::Rc; - -use super::de::{deserialize_js, property, scratch_buf}; -use super::error::{module_exception, ExcResult, ExceptionThrown, TypeError}; -use super::from_value::cast; -use super::ser::serialize_to_js; -use super::string::{str_from_ident, StringConst}; -use super::{ - call_free_fun, env_on_isolate, exception_already_thrown, BufferTooSmall, CodeError, JsInstanceEnv, JsStackTrace, - TerminationError, Throwable, -}; +use super::hooks::{get_hook_function, set_hook_slots}; +use super::{AbiVersion, FnRet, ModuleHook}; use crate::database_logger::{LogLevel, Record}; use crate::error::NodesError; use crate::host::instance_env::InstanceEnv; +use crate::host::v8::de::{deserialize_js, scratch_buf}; +use crate::host::v8::error::{ExcResult, ExceptionThrown, TypeError}; +use crate::host::v8::from_value::cast; +use crate::host::v8::ser::serialize_to_js; +use crate::host::v8::string::{str_from_ident, StringConst}; +use crate::host::v8::{ + call_free_fun, env_on_isolate, exception_already_thrown, BufferTooSmall, CodeError, JsInstanceEnv, JsStackTrace, + TerminationError, Throwable, +}; use crate::host::wasm_common::instrumentation::span; use crate::host::wasm_common::module_host_actor::{ReducerOp, ReducerResult}; use crate::host::wasm_common::{err_to_errno_and_log, RowIterIdx, TimingSpan, TimingSpanIdx}; @@ -20,51 +20,39 @@ use spacetimedb_lib::{bsatn, ConnectionId, Identity, RawModuleDef}; use spacetimedb_primitives::{errno, ColId, IndexId, ReducerId, TableId}; use spacetimedb_sats::Serialize; use v8::{ - callback_scope, ConstructorBehavior, Context, FixedArray, Function, FunctionCallbackArguments, Isolate, Local, - Module, Object, PinCallbackScope, PinScope, Value, + callback_scope, ConstructorBehavior, Function, FunctionCallbackArguments, Isolate, Local, Module, Object, + PinCallbackScope, PinScope, }; -/// A dependency resolver for the user's module -/// that will resolve `spacetimedb_sys` to a module that exposes the ABI. -pub(super) fn resolve_sys_module<'s>( - context: Local<'s, Context>, - spec: Local<'s, v8::String>, - _attrs: Local<'s, FixedArray>, - _referrer: Local<'s, Module>, -) -> Option> { - callback_scope!(unsafe scope, context); - - if spec == SYS_MODULE_NAME.string(scope) { - Some(register_sys_module(scope)) - } else { - module_exception(scope, spec).throw(scope); - None - } +macro_rules! create_synthetic_module { + ($scope:expr, $module_name:expr, $(($wrapper:ident, $abi_call:expr, $fun:ident),)*) => {{ + let export_names = &[$(str_from_ident!($fun).string($scope)),*]; + let eval_steps = |context, module| { + callback_scope!(unsafe scope, context); + $( + register_module_fun(scope, &module, str_from_ident!($fun), |s, a| { + $wrapper($abi_call, s, a, $fun) + })?; + )* + + Some(v8::undefined(scope).into()) + }; + + Module::create_synthetic_module( + $scope, + const { StringConst::new($module_name) }.string($scope), + export_names, + eval_steps, + ) + }} } /// Registers all module -> host syscalls in the JS module `spacetimedb_sys`. -fn register_sys_module<'scope>(scope: &mut PinScope<'scope, '_>) -> Local<'scope, Module> { - let module_name = SYS_MODULE_NAME.string(scope); - - macro_rules! create_synthetic_module { - ($(($wrapper:ident, $abi_call:expr, $fun:ident),)*) => {{ - let export_names = &[$(str_from_ident!($fun).string(scope)),*]; - let eval_steps = |context, module| { - callback_scope!(unsafe scope, context); - $( - register_module_fun(scope, &module, str_from_ident!($fun), |s, a| { - $wrapper($abi_call, s, a, $fun) - })?; - )* - - Some(v8::undefined(scope).into()) - }; - - Module::create_synthetic_module(scope, module_name, export_names, eval_steps) - }} - } - +pub(super) fn sys_v1_0<'scope>(scope: &mut PinScope<'scope, '_>) -> Local<'scope, Module> { + use register_hooks_v1_0 as register_hooks; create_synthetic_module!( + scope, + "spacetime:sys@1.0", (with_nothing, (), register_hooks), (with_sys_result_ret, AbiCall::TableIdFromName, table_id_from_name), (with_sys_result_ret, AbiCall::IndexIdFromName, index_id_from_name), @@ -122,11 +110,6 @@ fn register_sys_module<'scope>(scope: &mut PinScope<'scope, '_>) -> Local<'scope ) } -const SYS_MODULE_NAME: &StringConst = &StringConst::new("spacetime:sys@1.0"); - -/// The return type of a module -> host syscall. -pub(super) type FnRet<'scope> = ExcResult>; - /// Registers a function in `module` /// where the function has `name` and does `body`. fn register_module_fun( @@ -328,63 +311,30 @@ fn with_span<'scope, T, E: From>( /// /// Throws a `TypeError` if: /// - `hooks` is not an object that has functions `__describe_module__` and `__call_reducer__`. -fn register_hooks<'scope>(scope: &mut PinScope<'scope, '_>, args: FunctionCallbackArguments<'_>) -> FnRet<'scope> { +fn register_hooks_v1_0<'scope>(scope: &mut PinScope<'scope, '_>, args: FunctionCallbackArguments<'_>) -> FnRet<'scope> { // Convert `hooks` to an object. let hooks = cast!(scope, args.get(0), Object, "hooks object").map_err(|e| e.throw(scope))?; - // Set the hook. - let ctx = scope.get_current_context(); - // Call `set_slot` first, as it creates the annex - // and `set_embedder_data` is currently buggy. - ctx.set_slot(Rc::new(AbiVersion::V1)); - ctx.set_embedder_data(HOOKS_SLOT, hooks.into()); + let describe_module = get_hook_function(scope, hooks, str_from_ident!(__describe_module__))?; + let call_reducer = get_hook_function(scope, hooks, str_from_ident!(__call_reducer__))?; - // Validate that `__call_reducer__` + `__describe_module__` are functions. - let _ = describe_module_fun(scope)?; - let _ = call_reducer_fun(scope)?; + // Set the hooks. + set_hook_slots( + scope, + AbiVersion::V1, + &[ + (ModuleHook::DescribeModule, describe_module), + (ModuleHook::CallReducer, call_reducer), + ], + )?; Ok(v8::undefined(scope).into()) } -/// The `v8::Context::{get,set}_embedder_data` slot that holds the hooks object. -const HOOKS_SLOT: i32 = 20; - -/// The version of the ABI that is exposed to V8. -#[derive(Copy, Clone)] -enum AbiVersion { - V1, -} - -/// Returns the, in [`register_hooks`], -/// previously registered object with hooks. -fn get_hooks<'scope>(scope: &mut PinScope<'scope, '_>) -> ExcResult<(AbiVersion, Local<'scope, Object>)> { - let ctx = scope.get_current_context(); - let abi_version = *ctx - .get_slot::() - .ok_or_else(|| TypeError("module hooks were never registered").throw(scope))?; - - let hooks = ctx - .get_embedder_data(scope, HOOKS_SLOT) - .expect("if `AbiVersion` is set hooks must be set"); - Ok((abi_version, hooks.cast())) -} - -/// Gets a handle to the registered `__call_reducer__` function hook. -pub(super) fn call_reducer_fun<'scope>(scope: &mut PinScope<'scope, '_>) -> ExcResult> { - let (abi_ver, hooks_obj) = get_hooks(scope)?; - let AbiVersion::V1 = abi_ver; - - let key = str_from_ident!(__call_reducer__).string(scope); - let object = property(scope, hooks_obj, key)?; - let fun = cast!(scope, object, Function, "module function hook `__call_reducer__`").map_err(|e| e.throw(scope))?; - - Ok(fun) -} - /// Calls the `__call_reducer__` function `fun`. -pub(super) fn call_call_reducer<'scope>( - scope: &mut PinScope<'scope, '_>, - fun: Local<'scope, Function>, +pub(super) fn call_call_reducer( + scope: &mut PinScope<'_, '_>, + fun: Local<'_, Function>, op: ReducerOp<'_>, ) -> ExcResult { let ReducerOp { @@ -412,23 +362,8 @@ pub(super) fn call_call_reducer<'scope>( Ok(user_res) } -/// Gets a handle to the registered `__describe_module__` function hook. on `object`. -fn describe_module_fun<'scope>(scope: &mut PinScope<'scope, '_>) -> ExcResult> { - let (abi_ver, hooks_obj) = get_hooks(scope)?; - let AbiVersion::V1 = abi_ver; - - let key = str_from_ident!(__describe_module__).string(scope); - let object = property(scope, hooks_obj, key)?; - let fun = - cast!(scope, object, Function, "module function hook `__describe_module__`").map_err(|e| e.throw(scope))?; - Ok(fun) -} - /// Calls the registered `__describe_module__` function hook. -pub(super) fn call_describe_module<'scope>(scope: &mut PinScope<'scope, '_>) -> ExcResult { - // Get the registered function hook. - let fun = describe_module_fun(scope)?; - +pub(super) fn call_describe_module(scope: &mut PinScope<'_, '_>, fun: Local<'_, Function>) -> ExcResult { // Call the function. let raw_mod_js = call_free_fun(scope, fun, &[])?; From 827afd98040d3078ac9d43ab3271f8160dfad739 Mon Sep 17 00:00:00 2001 From: Noa Date: Thu, 30 Oct 2025 12:48:10 -0500 Subject: [PATCH 2/4] Apply suggestions from code review Co-authored-by: Mazdak Farrokhzad Signed-off-by: Noa --- crates/core/src/host/v8/string.rs | 1 + crates/core/src/host/v8/syscall/hooks.rs | 24 ++++++++++++++++------- crates/core/src/host/v8/syscall/mod.rs | 25 ++++++++++++++---------- 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/crates/core/src/host/v8/string.rs b/crates/core/src/host/v8/string.rs index 872df8c3155..d245ab5ca50 100644 --- a/crates/core/src/host/v8/string.rs +++ b/crates/core/src/host/v8/string.rs @@ -37,6 +37,7 @@ impl StringConst { .expect("`create_external_onebyte_const` should've asserted `.len() < kMaxLength`") } + /// Returns the backing string slice. pub(super) fn as_str(&'static self) -> &'static str { self.0.as_str() } diff --git a/crates/core/src/host/v8/syscall/hooks.rs b/crates/core/src/host/v8/syscall/hooks.rs index a12d6421ad5..68d73597fb6 100644 --- a/crates/core/src/host/v8/syscall/hooks.rs +++ b/crates/core/src/host/v8/syscall/hooks.rs @@ -12,16 +12,19 @@ use crate::host::v8::error::TypeError; use crate::host::v8::from_value::cast; use crate::host::v8::string::StringConst; -pub(super) fn get_hook_function<'s>( - scope: &mut PinScope<'s, '_>, +/// Returns the hook function `name` on `hooks_obj`. +pub(super) fn get_hook_function<'scope>( + scope: &mut PinScope<'scope, '_>, hooks_obj: Local<'_, Object>, name: &'static StringConst, -) -> ExcResult> { +) -> ExcResult> { let key = name.string(scope); let object = property(scope, hooks_obj, key)?; cast!(scope, object, Function, "module function hook `{}`", name.as_str()).map_err(|e| e.throw(scope)) } +/// Registers all the module function `hooks` +/// and sets the given `AbiVersion` to `abi`. pub(super) fn set_hook_slots( scope: &mut PinScope<'_, '_>, abi: AbiVersion, @@ -34,7 +37,7 @@ pub(super) fn set_hook_slots( for &(hook, func) in hooks { hooks_info .register(hook, abi) - .map_err(|_| TypeError("cannot call register_hooks multiple times").throw(scope))?; + .map_err(|_| TypeError("cannot call `register_hooks` multiple times").throw(scope))?; ctx.set_embedder_data(hook.to_slot_index(), func.into()); } Ok(()) @@ -47,7 +50,8 @@ pub(in crate::host::v8) enum ModuleHook { } impl ModuleHook { - /// Get the `v8::Context::{get,set}_embedder_data` slot that holds this hook. + /// Returns the index for the slot that holds the module function hook. + /// The index is passed to `v8::Context::{get,set}_embedder_data`. fn to_slot_index(self) -> i32 { match self { ModuleHook::DescribeModule => 20, @@ -56,6 +60,9 @@ impl ModuleHook { } } +/// Holds the `AbiVersion` used by the module +/// and the module hooks registered by the module +/// for that version. #[derive(Default)] struct HooksInfo { abi: OnceCell, @@ -63,6 +70,7 @@ struct HooksInfo { } impl HooksInfo { + /// Returns, and possibly creates, the [`HooksInfo`] stored in `ctx`. fn get_or_create(ctx: &Context) -> Rc { ctx.get_slot().unwrap_or_else(|| { let this = Rc::::default(); @@ -78,16 +86,18 @@ impl HooksInfo { self.registered[hook].set(()) } + /// Returns the `AbiVersion` for the given `hook`, if any. fn get(&self, hook: ModuleHook) -> Option { self.registered[hook].get().and(self.abi.get().copied()) } } #[derive(Copy, Clone)] -pub(in crate::host::v8) struct HookFunction<'s>(pub AbiVersion, pub Local<'s, Function>); +/// The actual callable module hook function and its abi version. +pub(in super::super) struct HookFunction<'scope>(pub AbiVersion, pub Local<'scope, Function>); /// Returns the hook function previously registered in [`register_hooks`]. -pub(in crate::host::v8) fn get_hook<'scope>( +pub(in super::super) fn get_hook<'scope>( scope: &mut PinScope<'scope, '_>, hook: ModuleHook, ) -> Option> { diff --git a/crates/core/src/host/v8/syscall/mod.rs b/crates/core/src/host/v8/syscall/mod.rs index ba4884b1a5c..d3bf634c7f1 100644 --- a/crates/core/src/host/v8/syscall/mod.rs +++ b/crates/core/src/host/v8/syscall/mod.rs @@ -23,20 +23,20 @@ pub enum AbiVersion { /// A dependency resolver for the user's module /// that will resolve `spacetimedb_sys` to a module that exposes the ABI. -pub(super) fn resolve_sys_module<'s>( - context: Local<'s, Context>, - spec: Local<'s, v8::String>, - _attrs: Local<'s, FixedArray>, - _referrer: Local<'s, Module>, -) -> Option> { +pub(super) fn resolve_sys_module<'scope>( + context: Local<'scope, Context>, + spec: Local<'scope, v8::String>, + _attrs: Local<'scope, FixedArray>, + _referrer: Local<'scope, Module>, +) -> Option> { callback_scope!(unsafe scope, context); resolve_sys_module_inner(scope, spec).ok() } -fn resolve_sys_module_inner<'s>( - scope: &mut PinScope<'s, '_>, - spec: Local<'s, v8::String>, -) -> ExcResult> { +fn resolve_sys_module_inner<'scope>( + scope: &mut PinScope<'scope, '_>, + spec: Local<'scope, v8::String>, +) -> ExcResult> { let scratch = &mut scratch_buf::<32>(); let spec = spec.to_rust_cow_lossy(scope, scratch); @@ -65,6 +65,9 @@ fn resolve_sys_module_inner<'s>( } } +/// Calls the registered `__call_reducer__` function hook. +/// +/// This handles any (future) ABI version differences. pub(super) fn call_call_reducer( scope: &mut PinScope<'_, '_>, fun: HookFunction<'_>, @@ -77,6 +80,8 @@ pub(super) fn call_call_reducer( } /// Calls the registered `__describe_module__` function hook. +/// +/// This handles any (future) ABI version differences. pub(super) fn call_describe_module<'scope>( scope: &mut PinScope<'scope, '_>, fun: HookFunction<'_>, From d3116659686a359d540d9033db6772e26c9ee9b3 Mon Sep 17 00:00:00 2001 From: Noa Date: Thu, 30 Oct 2025 13:00:26 -0500 Subject: [PATCH 3/4] Address comments --- crates/core/src/host/v8/mod.rs | 10 ++--- crates/core/src/host/v8/syscall/hooks.rs | 56 ++++++++++++++---------- crates/core/src/host/v8/syscall/mod.rs | 12 ++--- crates/core/src/host/v8/syscall/v1.rs | 6 +-- crates/core/src/host/wasm_common/abi.rs | 14 +++--- 5 files changed, 55 insertions(+), 43 deletions(-) diff --git a/crates/core/src/host/v8/mod.rs b/crates/core/src/host/v8/mod.rs index af9059be3bf..1604abc9131 100644 --- a/crates/core/src/host/v8/mod.rs +++ b/crates/core/src/host/v8/mod.rs @@ -6,7 +6,7 @@ use self::error::{ use self::ser::serialize_to_js; use self::string::{str_from_ident, IntoJsString}; use self::syscall::{ - call_call_reducer, call_describe_module, get_hook, resolve_sys_module, FnRet, HookFunction, ModuleHook, + call_call_reducer, call_describe_module, get_hook, resolve_sys_module, FnRet, HookFunction, ModuleHookKey, }; use super::module_common::{build_common_module_from_raw, run_describer, ModuleCommon}; use super::module_host::{CallProcedureParams, CallReducerParams, Module, ModuleInfo, ModuleRuntime}; @@ -338,7 +338,7 @@ fn startup_instance_worker<'scope>( // Find the `__call_reducer__` function. let call_reducer_fun = - get_hook(scope, ModuleHook::CallReducer).context("The `spacetimedb/server` module was never imported")?; + get_hook(scope, ModuleHookKey::CallReducer).context("The `spacetimedb/server` module was never imported")?; // If we don't have a module, make one. let module_common = match module_or_mcc { @@ -659,7 +659,7 @@ fn extract_description<'scope>( |a, b, c| log_traceback(replica_ctx, a, b, c), || { catch_exception(scope, |scope| { - let Some(describe_module) = get_hook(scope, ModuleHook::DescribeModule) else { + let Some(describe_module) = get_hook(scope, ModuleHookKey::DescribeModule) else { return Ok(RawModuleDef::V9(Default::default())); }; let def = call_describe_module(scope, describe_module)?; @@ -699,7 +699,7 @@ mod test { fn call_call_reducer_works() { let call = |code| { with_module_catch(code, |scope| { - let fun = get_hook(scope, ModuleHook::CallReducer).unwrap(); + let fun = get_hook(scope, ModuleHookKey::CallReducer).unwrap(); let op = ReducerOp { id: ReducerId(42), name: "foobar", @@ -778,7 +778,7 @@ js error Uncaught Error: foobar }) "#; let raw_mod = with_module_catch(code, |scope| { - let describe_module = get_hook(scope, ModuleHook::DescribeModule).unwrap(); + let describe_module = get_hook(scope, ModuleHookKey::DescribeModule).unwrap(); call_describe_module(scope, describe_module) }) .map_err(|e| e.to_string()); diff --git a/crates/core/src/host/v8/syscall/hooks.rs b/crates/core/src/host/v8/syscall/hooks.rs index 68d73597fb6..05acaf48e04 100644 --- a/crates/core/src/host/v8/syscall/hooks.rs +++ b/crates/core/src/host/v8/syscall/hooks.rs @@ -28,15 +28,16 @@ pub(super) fn get_hook_function<'scope>( pub(super) fn set_hook_slots( scope: &mut PinScope<'_, '_>, abi: AbiVersion, - hooks: &[(ModuleHook, Local<'_, Function>)], + hooks: &[(ModuleHookKey, Local<'_, Function>)], ) -> ExcResult<()> { // Make sure to call `set_slot` first, as it creates the annex // and `set_embedder_data` is currently buggy. let ctx = scope.get_current_context(); - let hooks_info = HooksInfo::get_or_create(&ctx); + let hooks_info = HooksInfo::get_or_create(&ctx, abi) + .map_err(|_| TypeError("cannot call `register_hooks` from different versions").throw(scope))?; for &(hook, func) in hooks { hooks_info - .register(hook, abi) + .register(hook) .map_err(|_| TypeError("cannot call `register_hooks` multiple times").throw(scope))?; ctx.set_embedder_data(hook.to_slot_index(), func.into()); } @@ -44,18 +45,20 @@ pub(super) fn set_hook_slots( } #[derive(enum_map::Enum, Copy, Clone)] -pub(in crate::host::v8) enum ModuleHook { +pub(in super::super) enum ModuleHookKey { DescribeModule, CallReducer, } -impl ModuleHook { +impl ModuleHookKey { /// Returns the index for the slot that holds the module function hook. /// The index is passed to `v8::Context::{get,set}_embedder_data`. fn to_slot_index(self) -> i32 { match self { - ModuleHook::DescribeModule => 20, - ModuleHook::CallReducer => 21, + // high numbers to avoid overlapping with rusty_v8 - can be + // reverted to just 0, 1... once denoland/rusty_v8#1868 merges + ModuleHookKey::DescribeModule => 20, + ModuleHookKey::CallReducer => 21, } } } @@ -63,32 +66,39 @@ impl ModuleHook { /// Holds the `AbiVersion` used by the module /// and the module hooks registered by the module /// for that version. -#[derive(Default)] struct HooksInfo { - abi: OnceCell, - registered: EnumMap>, + abi: AbiVersion, + registered: EnumMap>, } impl HooksInfo { /// Returns, and possibly creates, the [`HooksInfo`] stored in `ctx`. - fn get_or_create(ctx: &Context) -> Rc { - ctx.get_slot().unwrap_or_else(|| { - let this = Rc::::default(); - ctx.set_slot(this.clone()); - this - }) + /// + /// Returns an error if `abi` doesn't match the abi version in the + /// already existing `HooksInfo`. + fn get_or_create(ctx: &Context, abi: AbiVersion) -> Result, ()> { + match ctx.get_slot::() { + Some(this) if this.abi == abi => Ok(this), + Some(_) => Err(()), + None => { + let this = Rc::new(Self { + abi, + registered: EnumMap::default(), + }); + ctx.set_slot(this.clone()); + Ok(this) + } + } } - fn register(&self, hook: ModuleHook, abi: AbiVersion) -> Result<(), ()> { - if *self.abi.get_or_init(|| abi) != abi { - return Err(()); - } + /// Mark down the given `hook` as registered, returning an error if it already was. + fn register(&self, hook: ModuleHookKey) -> Result<(), ()> { self.registered[hook].set(()) } /// Returns the `AbiVersion` for the given `hook`, if any. - fn get(&self, hook: ModuleHook) -> Option { - self.registered[hook].get().and(self.abi.get().copied()) + fn get(&self, hook: ModuleHookKey) -> Option { + self.registered[hook].get().map(|_| self.abi) } } @@ -99,7 +109,7 @@ pub(in super::super) struct HookFunction<'scope>(pub AbiVersion, pub Local<'scop /// Returns the hook function previously registered in [`register_hooks`]. pub(in super::super) fn get_hook<'scope>( scope: &mut PinScope<'scope, '_>, - hook: ModuleHook, + hook: ModuleHookKey, ) -> Option> { let ctx = scope.get_current_context(); let hooks = ctx.get_slot::()?; diff --git a/crates/core/src/host/v8/syscall/mod.rs b/crates/core/src/host/v8/syscall/mod.rs index d3bf634c7f1..3230b2868a3 100644 --- a/crates/core/src/host/v8/syscall/mod.rs +++ b/crates/core/src/host/v8/syscall/mod.rs @@ -1,16 +1,18 @@ use spacetimedb_lib::RawModuleDef; +use spacetimedb_lib::VersionTuple; use v8::{callback_scope, Context, FixedArray, Local, Module, PinScope}; use crate::host::v8::de::scratch_buf; use crate::host::v8::error::ExcResult; use crate::host::v8::error::Throwable; use crate::host::v8::error::TypeError; +use crate::host::wasm_common::abi::parse_abi_version; use crate::host::wasm_common::module_host_actor::{ReducerOp, ReducerResult}; mod hooks; mod v1; -pub(super) use self::hooks::{get_hook, HookFunction, ModuleHook}; +pub(super) use self::hooks::{get_hook, HookFunction, ModuleHookKey}; /// The return type of a module -> host syscall. pub(super) type FnRet<'scope> = ExcResult>; @@ -47,17 +49,15 @@ fn resolve_sys_module_inner<'scope>( .and_then(|spec| spec.split_once('@')) .ok_or_else(|| generic_error().throw(scope))?; - let (maj, min) = ver - .split_once('.') - .and_then(|(maj, min)| Option::zip(maj.parse::().ok(), min.parse::().ok())) + let VersionTuple { major, minor } = parse_abi_version(ver) .ok_or_else(|| TypeError(format!("Invalid version in module spec {spec:?}")).throw(scope))?; match module { - "sys" => match (maj, min) { + "sys" => match (major, minor) { (1, 0) => Ok(v1::sys_v1_0(scope)), _ => Err(TypeError(format!( "Could not import {spec:?}, likely because this module was built for a newer version of SpacetimeDB.\n\ - It requires sys module v{maj}.{min}, but that version is not supported by the database." + It requires sys module v{major}.{minor}, but that version is not supported by the database." )) .throw(scope)), }, diff --git a/crates/core/src/host/v8/syscall/v1.rs b/crates/core/src/host/v8/syscall/v1.rs index e22bd6d0a21..043df27a8ae 100644 --- a/crates/core/src/host/v8/syscall/v1.rs +++ b/crates/core/src/host/v8/syscall/v1.rs @@ -1,5 +1,5 @@ use super::hooks::{get_hook_function, set_hook_slots}; -use super::{AbiVersion, FnRet, ModuleHook}; +use super::{AbiVersion, FnRet, ModuleHookKey}; use crate::database_logger::{LogLevel, Record}; use crate::error::NodesError; use crate::host::instance_env::InstanceEnv; @@ -323,8 +323,8 @@ fn register_hooks_v1_0<'scope>(scope: &mut PinScope<'scope, '_>, args: FunctionC scope, AbiVersion::V1, &[ - (ModuleHook::DescribeModule, describe_module), - (ModuleHook::CallReducer, call_reducer), + (ModuleHookKey::DescribeModule, describe_module), + (ModuleHookKey::CallReducer, call_reducer), ], )?; diff --git a/crates/core/src/host/wasm_common/abi.rs b/crates/core/src/host/wasm_common/abi.rs index ff189283f18..5440b014f45 100644 --- a/crates/core/src/host/wasm_common/abi.rs +++ b/crates/core/src/host/wasm_common/abi.rs @@ -8,12 +8,8 @@ pub fn determine_spacetime_abi( ) -> Result { let it = imports.into_iter().filter_map(|imp| { let s = get_module(&imp); - let err = || AbiVersionError::Parse { module: s.to_owned() }; - s.strip_prefix(MODULE_PREFIX).map(|ver| { - let (major, minor) = ver.split_once('.').ok_or_else(err)?; - let (major, minor) = Option::zip(major.parse().ok(), minor.parse().ok()).ok_or_else(err)?; - Ok(VersionTuple { major, minor }) - }) + s.strip_prefix(MODULE_PREFIX) + .map(|ver| parse_abi_version(ver).ok_or_else(|| AbiVersionError::Parse { module: s.to_owned() })) }); itertools::process_results(it, |mut it| { let first = it.next().ok_or(AbiVersionError::NotDetected)?; @@ -21,6 +17,12 @@ pub fn determine_spacetime_abi( })? } +pub fn parse_abi_version(ver: &str) -> Option { + let (major, minor) = ver.split_once('.')?; + let (major, minor) = Option::zip(major.parse().ok(), minor.parse().ok())?; + Some(VersionTuple { major, minor }) +} + fn refine_ver_req(ver: VersionTuple, new: VersionTuple) -> Result { if ver.major != new.major { Err(AbiVersionError::MultipleMajor(ver.major, new.major)) From 9d715487324ee988af91b44675dca71b7f3bb57d Mon Sep 17 00:00:00 2001 From: Noa Date: Mon, 3 Nov 2025 11:54:28 -0600 Subject: [PATCH 4/4] Error if no describe_module --- crates/core/src/host/v8/mod.rs | 10 +++++----- crates/core/src/host/v8/syscall/mod.rs | 9 +++------ crates/core/src/host/v8/syscall/v1.rs | 11 +++++++---- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/crates/core/src/host/v8/mod.rs b/crates/core/src/host/v8/mod.rs index 1604abc9131..8a7f6b58d0b 100644 --- a/crates/core/src/host/v8/mod.rs +++ b/crates/core/src/host/v8/mod.rs @@ -659,9 +659,8 @@ fn extract_description<'scope>( |a, b, c| log_traceback(replica_ctx, a, b, c), || { catch_exception(scope, |scope| { - let Some(describe_module) = get_hook(scope, ModuleHookKey::DescribeModule) else { - return Ok(RawModuleDef::V9(Default::default())); - }; + let describe_module = get_hook(scope, ModuleHookKey::DescribeModule) + .context("The `spacetimedb/server` package was never imported into the module")?; let def = call_describe_module(scope, describe_module)?; Ok(def) }) @@ -675,6 +674,7 @@ fn extract_description<'scope>( mod test { use super::to_value::test::with_scope; use super::*; + use crate::host::v8::error::{ErrorOrException, ExceptionThrown}; use crate::host::wasm_common::module_host_actor::ReducerOp; use crate::host::ArgsTuple; use spacetimedb_lib::{ConnectionId, Identity}; @@ -682,7 +682,7 @@ mod test { fn with_module_catch( code: &str, - logic: impl for<'scope> FnOnce(&mut PinScope<'scope, '_>) -> ExcResult, + logic: impl for<'scope> FnOnce(&mut PinScope<'scope, '_>) -> Result>, ) -> anyhow::Result { with_scope(|scope| { eval_user_module_catch(scope, code).unwrap(); @@ -708,7 +708,7 @@ mod test { timestamp: Timestamp::from_micros_since_unix_epoch(24), args: &ArgsTuple::nullary(), }; - call_call_reducer(scope, fun, op) + Ok(call_call_reducer(scope, fun, op)?) }) }; diff --git a/crates/core/src/host/v8/syscall/mod.rs b/crates/core/src/host/v8/syscall/mod.rs index 3230b2868a3..caa58ceb33c 100644 --- a/crates/core/src/host/v8/syscall/mod.rs +++ b/crates/core/src/host/v8/syscall/mod.rs @@ -1,11 +1,8 @@ -use spacetimedb_lib::RawModuleDef; -use spacetimedb_lib::VersionTuple; +use spacetimedb_lib::{RawModuleDef, VersionTuple}; use v8::{callback_scope, Context, FixedArray, Local, Module, PinScope}; use crate::host::v8::de::scratch_buf; -use crate::host::v8::error::ExcResult; -use crate::host::v8::error::Throwable; -use crate::host::v8::error::TypeError; +use crate::host::v8::error::{ErrorOrException, ExcResult, ExceptionThrown, Throwable, TypeError}; use crate::host::wasm_common::abi::parse_abi_version; use crate::host::wasm_common::module_host_actor::{ReducerOp, ReducerResult}; @@ -85,7 +82,7 @@ pub(super) fn call_call_reducer( pub(super) fn call_describe_module<'scope>( scope: &mut PinScope<'scope, '_>, fun: HookFunction<'_>, -) -> ExcResult { +) -> Result> { let HookFunction(ver, fun) = fun; match ver { AbiVersion::V1 => v1::call_describe_module(scope, fun), diff --git a/crates/core/src/host/v8/syscall/v1.rs b/crates/core/src/host/v8/syscall/v1.rs index 043df27a8ae..c7f666e258f 100644 --- a/crates/core/src/host/v8/syscall/v1.rs +++ b/crates/core/src/host/v8/syscall/v1.rs @@ -4,7 +4,7 @@ use crate::database_logger::{LogLevel, Record}; use crate::error::NodesError; use crate::host::instance_env::InstanceEnv; use crate::host::v8::de::{deserialize_js, scratch_buf}; -use crate::host::v8::error::{ExcResult, ExceptionThrown, TypeError}; +use crate::host::v8::error::{ErrorOrException, ExcResult, ExceptionThrown}; use crate::host::v8::from_value::cast; use crate::host::v8::ser::serialize_to_js; use crate::host::v8::string::{str_from_ident, StringConst}; @@ -16,6 +16,7 @@ use crate::host::wasm_common::instrumentation::span; use crate::host::wasm_common::module_host_actor::{ReducerOp, ReducerResult}; use crate::host::wasm_common::{err_to_errno_and_log, RowIterIdx, TimingSpan, TimingSpanIdx}; use crate::host::AbiCall; +use anyhow::Context; use spacetimedb_lib::{bsatn, ConnectionId, Identity, RawModuleDef}; use spacetimedb_primitives::{errno, ColId, IndexId, ReducerId, TableId}; use spacetimedb_sats::Serialize; @@ -363,7 +364,10 @@ pub(super) fn call_call_reducer( } /// Calls the registered `__describe_module__` function hook. -pub(super) fn call_describe_module(scope: &mut PinScope<'_, '_>, fun: Local<'_, Function>) -> ExcResult { +pub(super) fn call_describe_module( + scope: &mut PinScope<'_, '_>, + fun: Local<'_, Function>, +) -> Result> { // Call the function. let raw_mod_js = call_free_fun(scope, fun, &[])?; @@ -377,8 +381,7 @@ pub(super) fn call_describe_module(scope: &mut PinScope<'_, '_>, fun: Local<'_, .map_err(|e| e.throw(scope))?; let bytes = raw_mod.get_contents(&mut []); - let module = - bsatn::from_slice::(bytes).map_err(|_e| TypeError("invalid bsatn module def").throw(scope))?; + let module = bsatn::from_slice::(bytes).context("invalid bsatn module def")?; Ok(module) }