From 84870e91ea411fe31567589bef7a420030acd215 Mon Sep 17 00:00:00 2001 From: Pete Matsyburka Date: Sat, 30 May 2026 16:55:40 +0300 Subject: [PATCH 1/4] Allow releasing GVL when calling exported Wasm functions --- ext/src/helpers/mod.rs | 2 +- ext/src/helpers/nogvl.rs | 61 ++++++++---- ext/src/ruby_api/externals.rs | 25 +++-- ext/src/ruby_api/func.rs | 171 +++++++++++++++++++--------------- ext/src/ruby_api/instance.rs | 2 +- spec/unit/func_spec.rb | 143 ++++++++++++++++++++++++++++ 6 files changed, 306 insertions(+), 98 deletions(-) diff --git a/ext/src/helpers/mod.rs b/ext/src/helpers/mod.rs index 5a531371..c5cc5b05 100644 --- a/ext/src/helpers/mod.rs +++ b/ext/src/helpers/mod.rs @@ -5,7 +5,7 @@ mod static_id; mod symbol_enum; mod tmplock; -pub use nogvl::nogvl; +pub use nogvl::{nogvl, with_gvl}; pub use output_limited_buffer::OutputLimitedBuffer; pub use static_id::StaticId; pub use symbol_enum::SymbolEnum; diff --git a/ext/src/helpers/nogvl.rs b/ext/src/helpers/nogvl.rs index bb7ba712..8f93502c 100644 --- a/ext/src/helpers/nogvl.rs +++ b/ext/src/helpers/nogvl.rs @@ -1,29 +1,58 @@ -use std::{ffi::c_void, mem::MaybeUninit, ptr::null_mut}; +use std::{ + ffi::c_void, + mem::MaybeUninit, + panic::{self, AssertUnwindSafe}, + ptr::null_mut, + thread, +}; -use rb_sys::rb_thread_call_without_gvl; +use rb_sys::{rb_thread_call_with_gvl, rb_thread_call_without_gvl, ruby_thread_has_gvl_p}; -unsafe extern "C" fn call_without_gvl(arg: *mut c_void) -> *mut c_void +#[inline] +fn has_gvl() -> bool { + unsafe { ruby_thread_has_gvl_p() != 0 } +} + +unsafe extern "C" fn call_trampoline(arg: *mut c_void) -> *mut c_void where - F: FnMut() -> R, - R: Sized, + F: FnOnce() -> R, { - let arg = arg as *mut (&mut F, &mut MaybeUninit); - let (func, result) = unsafe { &mut *arg }; - result.write(func()); - + let data = unsafe { &mut *(arg as *mut (Option, MaybeUninit>)) }; + let func = data.0.take().expect("closure called more than once"); + data.1.write(panic::catch_unwind(AssertUnwindSafe(func))); null_mut() } -pub fn nogvl(mut func: F) -> R +pub fn nogvl(func: F) -> R +where + F: FnOnce() -> R, +{ + let mut data: (Option, MaybeUninit>) = (Some(func), MaybeUninit::uninit()); + let arg = &mut data as *mut _ as *mut c_void; + + unsafe { + rb_thread_call_without_gvl(Some(call_trampoline::), arg, None, null_mut()); + data.1 + .assume_init() + .unwrap_or_else(|e| panic::resume_unwind(e)) + } +} + +pub fn with_gvl(func: F) -> R where - F: FnMut() -> R, - R: Sized, + F: FnOnce() -> R, { - let result = MaybeUninit::uninit(); - let arg_ptr = &(&mut func, &result) as *const _ as *mut c_void; + if has_gvl() { + return func(); + } + + let mut data: (Option, MaybeUninit>) = (Some(func), MaybeUninit::uninit()); + let arg = &mut data as *mut _ as *mut c_void; unsafe { - rb_thread_call_without_gvl(Some(call_without_gvl::), arg_ptr, None, null_mut()); - result.assume_init() + rb_thread_call_with_gvl(Some(call_trampoline::), arg); + data.1 + .assume_init() + .unwrap_or_else(|e| panic::resume_unwind(e)) } } diff --git a/ext/src/ruby_api/externals.rs b/ext/src/ruby_api/externals.rs index f21c2723..3f5c9da8 100644 --- a/ext/src/ruby_api/externals.rs +++ b/ext/src/ruby_api/externals.rs @@ -7,12 +7,16 @@ use super::{ store::StoreContextValue, table::{Table, TableType}, }; -use crate::{conversion_err, not_implemented}; +use crate::{conversion_err, define_rb_intern, not_implemented}; use magnus::{ - class, gc::Marker, method, prelude::*, rb_sys::AsRawValue, typed_data::Obj, DataTypeFunctions, - Error, Module, RClass, Ruby, TypedData, Value, + class, gc::Marker, method, prelude::*, rb_sys::AsRawValue, scan_args, typed_data::Obj, + DataTypeFunctions, Error, Module, RClass, Ruby, TypedData, Value, }; +define_rb_intern!( + GVL => "gvl", +); + #[derive(TypedData)] #[magnus( class = "Wasmtime::ExternType", @@ -128,10 +132,19 @@ impl Extern<'_> { /// @yard /// Returns the exported function or raises a `{ConversionError}` when the export is not a /// function. + /// + /// @def to_func(gvl: true) + /// @param gvl [Boolean] When +false+, releases the GVL during the call so other Ruby threads run in parallel (each thread must use its own {Store}). Defaults to +true+. /// @return [Func] The exported function. - pub fn to_func(ruby: &Ruby, rb_self: Obj) -> Result { + pub fn to_func(ruby: &Ruby, rb_self: Obj, args: &[Value]) -> Result { + let args = scan_args::scan_args::<(), (), (), (), _, ()>(args)?; + let kw = scan_args::get_kwargs::<_, (), (Option,), ()>(args.keywords, &[], &[*GVL])?; + match *rb_self { - Extern::Func(f) => Ok(f.as_value()), + Extern::Func(f) => match kw.optional.0 { + Some(false) => Ok(ruby.obj_wrap(f.without_gvl()).as_value()), + _ => Ok(f.as_value()), + }, _ => conversion_err!(Self::inner_class(rb_self), Func::class(ruby)), } } @@ -252,7 +265,7 @@ pub fn init(ruby: &Ruby) -> Result<(), Error> { let class = root().define_class("Extern", ruby.class_object())?; - class.define_method("to_func", method!(Extern::to_func, 0))?; + class.define_method("to_func", method!(Extern::to_func, -1))?; class.define_method("to_global", method!(Extern::to_global, 0))?; class.define_method("to_memory", method!(Extern::to_memory, 0))?; class.define_method("to_table", method!(Extern::to_table, 0))?; diff --git a/ext/src/ruby_api/func.rs b/ext/src/ruby_api/func.rs index d95b6ba6..8fd77df2 100644 --- a/ext/src/ruby_api/func.rs +++ b/ext/src/ruby_api/func.rs @@ -6,7 +6,11 @@ use super::{ root, store::{Store, StoreContextValue, StoreData}, }; -use crate::{error, Caller}; +use crate::{ + error, + helpers::{nogvl, with_gvl}, + Caller, +}; use magnus::{ block::Proc, class, function, gc::Marker, method, prelude::*, scan_args::scan_args, typed_data::Obj, value::Opaque, DataTypeFunctions, Error, IntoValue, Object, RArray, Ruby, @@ -81,6 +85,7 @@ impl From<&FuncType> for wasmtime::ExternType { pub struct Func<'a> { store: StoreContextValue<'a>, inner: FuncImpl, + gvl: bool, } impl DataTypeFunctions for Func<'_> { @@ -141,14 +146,23 @@ impl<'a> Func<'a> { let func_closure = make_func_closure(&ty, callable.into()); let inner = wasmtime::Func::new(context, ty, func_closure); - Ok(Self { - store: store.into(), - inner, - }) + Ok(Self::from_inner(store.into(), inner)) } pub fn from_inner(store: StoreContextValue<'a>, inner: FuncImpl) -> Self { - Self { store, inner } + Self { + store, + inner, + gvl: true, + } + } + + pub fn without_gvl(&self) -> Self { + Self { + store: self.store, + inner: self.inner, + gvl: false, + } } pub fn get(&self) -> FuncImpl { @@ -176,7 +190,7 @@ impl<'a> Func<'a> { /// func.call(1, 2) # => [2, 3] pub fn call(&self, args: &[Value]) -> Result { let ruby = Ruby::get().unwrap(); - Self::invoke(&ruby, &self.store, &self.inner, args) + Self::invoke(&ruby, &self.store, &self.inner, self.gvl, args) } pub fn inner(&self) -> &FuncImpl { @@ -211,6 +225,7 @@ impl<'a> Func<'a> { ruby: &Ruby, store: &StoreContextValue, func: &wasmtime::Func, + gvl: bool, args: &[Value], ) -> Result { let mut context = store.context_mut()?; @@ -218,8 +233,12 @@ impl<'a> Func<'a> { let params = Params::new(ruby, &func_ty, args)?.to_vec(ruby, store)?; let mut results = vec![Val::null_func_ref(); func_ty.results().len()]; - func.call(context, ¶ms, &mut results) - .map_err(|e| store.handle_wasm_error(ruby, e))?; + let call_result = if gvl { + func.call(&mut context, ¶ms, &mut results) + } else { + nogvl(|| func.call(&mut context, ¶ms, &mut results)) + }; + call_result.map_err(|e| store.handle_wasm_error(ruby, e))?; // Check for any errors stored during execution (e.g., from socket checks) if let Some(error) = store.take_last_error()? { @@ -275,79 +294,83 @@ pub fn make_func_closure( // We then return a generic error here. The caller will check for a stored error // and raise it if it exists. move |caller_impl: CallerImpl<'_, StoreData>, params: &[Val], results: &mut [Val]| { - let ruby = Ruby::get().unwrap(); - let wrapped_caller = ruby.obj_wrap(Caller::new(caller_impl)); - let store_context = StoreContextValue::from(wrapped_caller); - - let rparams = ruby.ary_new_capa(params.len() + 1); - rparams - .push(wrapped_caller.as_value()) - .map_err(|e| wasmtime::Error::msg(format!("failed to push caller: {e}")))?; - - for (i, param) in params.iter().enumerate() { - let val = param - .to_ruby_value(&ruby, &store_context) - .map_err(|e| wasmtime::Error::msg(format!("invalid argument at index {i}: {e}")))?; - rparams.push(val).map_err(|e| { - wasmtime::Error::msg(format!("failed to push argument at index {i}: {e}")) - })?; - } + // Borrow `ty` so the `move` closure captures a reference, not the owned value. + let ty = &ty; + with_gvl(move || { + let ruby = Ruby::get().unwrap(); + let wrapped_caller = ruby.obj_wrap(Caller::new(caller_impl)); + let store_context = StoreContextValue::from(wrapped_caller); + + let rparams = ruby.ary_new_capa(params.len() + 1); + rparams + .push(wrapped_caller.as_value()) + .map_err(|e| wasmtime::Error::msg(format!("failed to push caller: {e}")))?; + + for (i, param) in params.iter().enumerate() { + let val = param.to_ruby_value(&ruby, &store_context).map_err(|e| { + wasmtime::Error::msg(format!("invalid argument at index {i}: {e}")) + })?; + rparams.push(val).map_err(|e| { + wasmtime::Error::msg(format!("failed to push argument at index {i}: {e}")) + })?; + } - let callable = ruby.get_inner(callable); + let callable = ruby.get_inner(callable); - match (callable.call(rparams), results.len()) { - (Ok(_proc_result), 0) => { - wrapped_caller.expire(); - Ok(()) - } - (Ok(proc_result), n) => { - // For len=1, accept both `val` and `[val]` - let Ok(proc_result) = RArray::to_ary(proc_result) else { - return result_error!( - store_context, - wrapped_caller, - format!("could not convert {} to results array", callable) - ); - }; - - if proc_result.len() != results.len() { - return result_error!( - store_context, - wrapped_caller, - format!( - "wrong number of results (given {}, expected {}) in {}", - proc_result.len(), - n, - callable - ) - ); + match (callable.call(rparams), results.len()) { + (Ok(_proc_result), 0) => { + wrapped_caller.expire(); + Ok(()) } + (Ok(proc_result), n) => { + // For len=1, accept both `val` and `[val]` + let Ok(proc_result) = RArray::to_ary(proc_result) else { + return result_error!( + store_context, + wrapped_caller, + format!("could not convert {} to results array", callable) + ); + }; + + if proc_result.len() != results.len() { + return result_error!( + store_context, + wrapped_caller, + format!( + "wrong number of results (given {}, expected {}) in {}", + proc_result.len(), + n, + callable + ) + ); + } - for (i, ((rb_val, wasm_val), ty)) in unsafe { proc_result.as_slice() } - .iter() - .zip(results.iter_mut()) - .zip(ty.results()) - .enumerate() - { - match rb_val.to_wasm_val(&store_context, ty) { - Ok(val) => *wasm_val = val, - Err(e) => { - return result_error!( - store_context, - wrapped_caller, - format!("invalid result at index {i}: {e} in {callable}") - ); + for (i, ((rb_val, wasm_val), ty)) in unsafe { proc_result.as_slice() } + .iter() + .zip(results.iter_mut()) + .zip(ty.results()) + .enumerate() + { + match rb_val.to_wasm_val(&store_context, ty) { + Ok(val) => *wasm_val = val, + Err(e) => { + return result_error!( + store_context, + wrapped_caller, + format!("invalid result at index {i}: {e} in {callable}") + ); + } } } - } - wrapped_caller.expire(); - Ok(()) - } - (Err(e), _) => { - caller_error!(store_context, wrapped_caller, e) + wrapped_caller.expire(); + Ok(()) + } + (Err(e), _) => { + caller_error!(store_context, wrapped_caller, e) + } } - } + }) } } diff --git a/ext/src/ruby_api/instance.rs b/ext/src/ruby_api/instance.rs index 64434463..46a699e6 100644 --- a/ext/src/ruby_api/instance.rs +++ b/ext/src/ruby_api/instance.rs @@ -139,7 +139,7 @@ impl Instance { })?)?; let func = rb_self.get_func(rb_self.store.context_mut(), unsafe { name.as_str()? })?; - Func::invoke(ruby, &rb_self.store.into(), &func, &args[1..]) + Func::invoke(ruby, &rb_self.store.into(), &func, true, &args[1..]) } fn get_func( diff --git a/spec/unit/func_spec.rb b/spec/unit/func_spec.rb index 54165374..c5cd63c6 100644 --- a/spec/unit/func_spec.rb +++ b/spec/unit/func_spec.rb @@ -161,11 +161,154 @@ module Wasmtime end end + describe "#call with gvl: false" do + it "still returns correct results" do + store = Store.new(engine) + mod = Module.new(engine, <<~WAT) + (module (func (export "add") (param i32 i32) (result i32) + (i32.add (local.get 0) (local.get 1)))) + WAT + func = Instance.new(store, mod).export("add").to_func(gvl: false) + expect(func.call(2, 3)).to eq(5) + end + + it "releases the GVL so other Ruby threads run during the call" do + mod = Module.new(engine, <<~WAT) + (module + (import "env" "mark" (func $mark)) + (func (export "spin") (param $n i64) (result i64) + (local $i i64) + (call $mark) + (block $done + (loop $loop + (br_if $done (i64.ge_u (local.get $i) (local.get $n))) + (local.set $i (i64.add (local.get $i) (i64.const 1))) + (br $loop))) + (call $mark) + (local.get $i))) + WAT + + counter = 0 + running = true + sibling = Thread.new { counter += 1 while running } + + marks = [] + store = Store.new(engine) + mark = Func.new(store, [], []) { marks << counter } + linker = Linker.new(engine) + linker.define(store, "env", "mark", mark) + instance = linker.instantiate(store, mod) + + instance.export("spin").to_func(gvl: false).call(500_000_000) + + running = false + sibling.join + + expect(marks.length).to eq(2) + expect(marks.last).to be > marks.first + end + + it "re-acquires the GVL for Ruby host callbacks during a released call" do + expect(run_released_with_callback(host_callback_module, spin: 1_000)).to eq(42) + end + + it "stays correct with host callbacks across threads under GC stress" do + mod = host_callback_module + + results = with_gc_stress do + 10.times.map do + Thread.new { run_released_with_callback(mod, spin: 50_000) } + end.map(&:value) + end + + expect(results).to eq(Array.new(10, 42)) + end + + it "bubbles a host-callback exception raised during a released call" do + error_class = Class.new(StandardError) + store = Store.new(engine) + raising = Func.new(store, [:i32], [:i32]) { raise error_class, "boom" } + linker = Linker.new(engine) + linker.define(store, "env", "host_add", raising) + func = linker.instantiate(store, host_callback_module).export("run").to_func(gvl: false) + + expect { func.call(1_000) }.to raise_error(error_class, "boom") + end + + it "raises ResultError for a wrong-arity callback result during a released call" do + store = Store.new(engine) + bad = Func.new(store, [:i32], [:i32]) { |_caller, _x| [1, 2] } + linker = Linker.new(engine) + linker.define(store, "env", "host_add", bad) + func = linker.instantiate(store, host_callback_module).export("run").to_func(gvl: false) + + expect { func.call(1_000) }.to raise_error(Wasmtime::ResultError, /wrong number of results/) + end + + it "raises Trap for a Wasm trap during a released call" do + store = Store.new(engine) + mod = Module.new(engine, <<~WAT) + (module (func (export "boom") unreachable)) + WAT + func = Instance.new(store, mod).export("boom").to_func(gvl: false) + + expect { func.call }.to raise_error(Trap) + end + + it "bubbles host-callback exceptions across threads under GC stress" do + error_class = Class.new(StandardError) + + results = with_gc_stress do + 10.times.map do + Thread.new do + store = Store.new(engine) + raising = Func.new(store, [:i32], [:i32]) { raise error_class, "boom" } + linker = Linker.new(engine) + linker.define(store, "env", "host_add", raising) + func = linker.instantiate(store, host_callback_module).export("run").to_func(gvl: false) + begin + func.call(10_000) + :no_error + rescue error_class + :raised + end + end.value + end + end + + expect(results).to eq(Array.new(10, :raised)) + end + end + private def build_func(params, results, &block) store = Store.new(engine, Object.new) Func.new(store, params, results, &block) end + + def host_callback_module + Module.new(engine, <<~WAT) + (module + (import "env" "host_add" (func $host_add (param i32) (result i32))) + (func (export "run") (param $n i64) (result i32) + (local $i i64) + (block $done + (loop $loop + (br_if $done (i64.ge_u (local.get $i) (local.get $n))) + (local.set $i (i64.add (local.get $i) (i64.const 1))) + (br $loop))) + (call $host_add (i32.const 41)))) + WAT + end + + def run_released_with_callback(mod, spin:) + store = Store.new(engine) + host_add = Func.new(store, [:i32], [:i32]) { |_caller, x| x.to_s.to_i + 1 } + linker = Linker.new(engine) + linker.define(store, "env", "host_add", host_add) + instance = linker.instantiate(store, mod) + instance.export("run").to_func(gvl: false).call(spin) + end end end From 2520975b5e54cd45aaad31733016cb188978d246 Mon Sep 17 00:00:00 2001 From: Pete Matsyburka Date: Sat, 30 May 2026 18:37:04 +0300 Subject: [PATCH 2/4] Fix ruby 3.3 3.4 thread_has_gvl build --- ext/src/helpers/nogvl.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ext/src/helpers/nogvl.rs b/ext/src/helpers/nogvl.rs index 8f93502c..08fe2e01 100644 --- a/ext/src/helpers/nogvl.rs +++ b/ext/src/helpers/nogvl.rs @@ -1,12 +1,16 @@ use std::{ - ffi::c_void, + ffi::{c_int, c_void}, mem::MaybeUninit, panic::{self, AssertUnwindSafe}, ptr::null_mut, thread, }; -use rb_sys::{rb_thread_call_with_gvl, rb_thread_call_without_gvl, ruby_thread_has_gvl_p}; +use rb_sys::{rb_thread_call_with_gvl, rb_thread_call_without_gvl}; + +extern "C" { + fn ruby_thread_has_gvl_p() -> c_int; +} #[inline] fn has_gvl() -> bool { From 2a837cb0be93d587264afccca1865b6e548ca516 Mon Sep 17 00:00:00 2001 From: Pete Matsyburka Date: Tue, 9 Jun 2026 20:05:02 +0300 Subject: [PATCH 3/4] Update Func gvl: false doc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Saúl Cabrera --- ext/src/ruby_api/externals.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ext/src/ruby_api/externals.rs b/ext/src/ruby_api/externals.rs index 3f5c9da8..fc1d6a3c 100644 --- a/ext/src/ruby_api/externals.rs +++ b/ext/src/ruby_api/externals.rs @@ -135,6 +135,8 @@ impl Extern<'_> { /// /// @def to_func(gvl: true) /// @param gvl [Boolean] When +false+, releases the GVL during the call so other Ruby threads run in parallel (each thread must use its own {Store}). Defaults to +true+. + + /// Failing to respect the {Store}-per-thread requirement, when using `gvl: false` is highly unsafe and will result in undefined behavior. /// @return [Func] The exported function. pub fn to_func(ruby: &Ruby, rb_self: Obj, args: &[Value]) -> Result { let args = scan_args::scan_args::<(), (), (), (), _, ()>(args)?; From 42f6aa62d9310e304ecf16e7917b978601d53fab Mon Sep 17 00:00:00 2001 From: Pete Matsyburka Date: Tue, 9 Jun 2026 20:50:07 +0300 Subject: [PATCH 4/4] fix doc comment --- ext/src/ruby_api/externals.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/src/ruby_api/externals.rs b/ext/src/ruby_api/externals.rs index fc1d6a3c..afbe07dc 100644 --- a/ext/src/ruby_api/externals.rs +++ b/ext/src/ruby_api/externals.rs @@ -135,7 +135,7 @@ impl Extern<'_> { /// /// @def to_func(gvl: true) /// @param gvl [Boolean] When +false+, releases the GVL during the call so other Ruby threads run in parallel (each thread must use its own {Store}). Defaults to +true+. - + /// /// Failing to respect the {Store}-per-thread requirement, when using `gvl: false` is highly unsafe and will result in undefined behavior. /// @return [Func] The exported function. pub fn to_func(ruby: &Ruby, rb_self: Obj, args: &[Value]) -> Result {