From dc114658897f4879a3b314283e7d2a87788eacfd Mon Sep 17 00:00:00 2001 From: Xin Liu Date: Tue, 31 Oct 2023 00:58:46 +0800 Subject: [PATCH 01/15] feat(rust-sys): drop host_data in `Function::drop` Signed-off-by: Xin Liu --- crates/wasmedge-sys/src/instance/function.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/wasmedge-sys/src/instance/function.rs b/crates/wasmedge-sys/src/instance/function.rs index f0b861dcb..dd31a2df4 100644 --- a/crates/wasmedge-sys/src/instance/function.rs +++ b/crates/wasmedge-sys/src/instance/function.rs @@ -512,7 +512,14 @@ impl Drop for Function { // delete the function instance if !self.inner.lock().0.is_null() { - unsafe { ffi::WasmEdge_FunctionInstanceDelete(self.inner.lock().0) }; + unsafe { + // drop host data + let _ = + Box::from_raw(ffi::WasmEdge_FunctionInstanceGetData(self.inner.lock().0) + as *mut c_void); + + ffi::WasmEdge_FunctionInstanceDelete(self.inner.lock().0) + }; } } } From ac89b2a574b42c16d9d2907ae369ba0188cf2cce Mon Sep 17 00:00:00 2001 From: Xin Liu Date: Thu, 2 Nov 2023 03:29:06 +0000 Subject: [PATCH 02/15] chore(rust-sys): update `drop` of `ImportModule` and `Function` Signed-off-by: Xin Liu --- crates/wasmedge-sys/src/instance/function.rs | 98 ++++++++++++++++++-- crates/wasmedge-sys/src/instance/module.rs | 42 +++++++-- 2 files changed, 126 insertions(+), 14 deletions(-) diff --git a/crates/wasmedge-sys/src/instance/function.rs b/crates/wasmedge-sys/src/instance/function.rs index dd31a2df4..bf9741ab4 100644 --- a/crates/wasmedge-sys/src/instance/function.rs +++ b/crates/wasmedge-sys/src/instance/function.rs @@ -506,8 +506,6 @@ impl Drop for Function { ); } } - - self.inner.lock().0 = std::ptr::null_mut(); } // delete the function instance @@ -518,7 +516,7 @@ impl Drop for Function { Box::from_raw(ffi::WasmEdge_FunctionInstanceGetData(self.inner.lock().0) as *mut c_void); - ffi::WasmEdge_FunctionInstanceDelete(self.inner.lock().0) + ffi::WasmEdge_FunctionInstanceDelete(self.inner.lock().0); }; } } @@ -837,7 +835,8 @@ mod tests { ) -> Result, HostFuncError> { println!("Rust: Entering Rust function real_add"); - let host_data = unsafe { Box::from_raw(data as *mut T) }; + // Do not use `Box::from_raw`: let host_data = unsafe { Box::from_raw(data as *mut T) }; + let host_data = unsafe { &mut *(data as *mut T) }; println!("host_data: {:?}", host_data); if input.len() != 2 { @@ -1167,7 +1166,7 @@ mod tests { } #[test] - fn test_func_drop() -> Result<(), Box> { + fn test_func_drop_v1() -> Result<(), Box> { // create a host function let real_add = |_: CallingFrame, input: Vec, @@ -1314,6 +1313,80 @@ mod tests { Ok(()) } + #[test] + fn test_func_drop_v2() { + #[derive(Debug)] + struct Data { + _x: i32, + _y: String, + _v: Vec, + _s: Vec, + } + let data: Data = Data { + _x: 12, + _y: "hello".to_string(), + _v: vec![1, 2, 3], + _s: vec!["macos", "linux", "windows"], + }; + + fn real_add( + _frame: CallingFrame, + input: Vec, + data: *mut std::ffi::c_void, + ) -> Result, HostFuncError> { + println!("Rust: Entering Rust function real_add"); + + // Do not use `Box::from_raw`: let host_data = unsafe { Box::from_raw(data as *mut T) }; + let host_data = unsafe { &mut *(data as *mut T) }; + println!("host_data: {:?}", host_data); + + if input.len() != 2 { + return Err(HostFuncError::User(1)); + } + + let a = if input[0].ty() == ValType::I32 { + input[0].to_i32() + } else { + return Err(HostFuncError::User(2)); + }; + + let b = if input[1].ty() == ValType::I32 { + input[1].to_i32() + } else { + return Err(HostFuncError::User(3)); + }; + + let c = a + b; + println!("Rust: calcuating in real_add c: {c:?}"); + + println!("Rust: Leaving Rust function real_add"); + Ok(vec![WasmValue::from_i32(c)]) + } + + assert_eq!(HOST_FUNCS.read().len(), 0); + assert_eq!(HOST_FUNC_FOOTPRINTS.lock().len(), 0); + + // create a FuncType + let result = FuncType::create(vec![ValType::I32; 2], vec![ValType::I32]); + assert!(result.is_ok()); + let func_ty = result.unwrap(); + // create a host function + let result = Function::create_sync_func( + &func_ty, + Box::new(real_add::>), + Some(Box::new(data)), + 0, + ); + assert!(result.is_ok()); + let host_func = result.unwrap(); + + let host_func_cloned = host_func.clone(); + + drop(host_func); + + drop(host_func_cloned); + } + #[cfg(all(feature = "async", target_os = "linux"))] #[tokio::test] async fn test_func_async_closure() -> Result<(), Box> { @@ -1345,8 +1418,8 @@ mod tests { -> Box< (dyn std::future::Future, HostFuncError>> + Send), > { - // let host_data = unsafe { &mut *(data as *mut Data) }; - let host_data = unsafe { Box::from_raw(data as *mut Data) }; + // Do not use `Box::from_raw`: let host_data = unsafe { Box::from_raw(data as *mut Data) }; + let host_data = unsafe { &mut *(data as *mut Data) }; Box::new(async move { for _ in 0..10 { @@ -1450,7 +1523,8 @@ mod tests { data: *mut std::ffi::c_void, ) -> Box<(dyn std::future::Future, HostFuncError>> + Send)> { - let data = unsafe { Box::from_raw(data as *mut T) }; + // Do not use `Box::from_raw`: let data = unsafe { Box::from_raw(data as *mut T) }; + let data = unsafe { &mut *(data as *mut T) }; Box::new(async move { for _ in 0..10 { @@ -1480,6 +1554,9 @@ mod tests { assert!(result.is_ok()); let async_hello_func = result.unwrap(); + assert_eq!(ASYNC_HOST_FUNCS.read().len(), 1); + assert_eq!(HOST_FUNC_FOOTPRINTS.lock().len(), 1); + // create an Executor let result = Executor::create(None, None); assert!(result.is_ok()); @@ -1524,6 +1601,11 @@ mod tests { let _ = executor .call_func_async(&async_state, &async_hello, []) .await?; + + assert_eq!(ASYNC_HOST_FUNCS.read().len(), 1); + assert_eq!(HOST_FUNC_FOOTPRINTS.lock().len(), 1); + + drop(import); } assert_eq!(ASYNC_HOST_FUNCS.read().len(), 0); diff --git a/crates/wasmedge-sys/src/instance/module.rs b/crates/wasmedge-sys/src/instance/module.rs index 925b2ea5d..d1152ea64 100644 --- a/crates/wasmedge-sys/src/instance/module.rs +++ b/crates/wasmedge-sys/src/instance/module.rs @@ -1,13 +1,13 @@ //! Defines WasmEdge Instance and other relevant types. -#[cfg(all(feature = "async", target_os = "linux"))] -use crate::r#async::AsyncWasiModule; use crate::{ ffi, instance::{function::InnerFunc, global::InnerGlobal, memory::InnerMemory, table::InnerTable}, types::WasmEdgeString, - Function, Global, Memory, Table, WasmEdgeResult, + Function, Global, Memory, Table, WasmEdgeResult, HOST_FUNCS, HOST_FUNC_FOOTPRINTS, }; +#[cfg(all(feature = "async", target_os = "linux"))] +use crate::{r#async::AsyncWasiModule, ASYNC_HOST_FUNCS}; use parking_lot::Mutex; use std::sync::Arc; use wasmedge_types::error::{InstanceError, WasmEdgeError}; @@ -400,7 +400,38 @@ impl Drop for ImportModule { } // drop the registered host functions - self.funcs.drain(..); + for func in self.funcs.iter() { + if func.registered && Arc::strong_count(&func.inner) == 1 { + // remove the real_func from HOST_FUNCS + let footprint = func.inner.lock().0 as usize; + if let Some(key) = HOST_FUNC_FOOTPRINTS.lock().remove(&footprint) { + let mut map_host_func = HOST_FUNCS.write(); + if map_host_func.contains_key(&key) { + map_host_func.remove(&key).expect( + "[wasmedge-sys] Failed to remove the host function from HOST_FUNCS_NEW container", + ); + } + + #[cfg(all(feature = "async", target_os = "linux"))] + { + let mut map_host_func = ASYNC_HOST_FUNCS.write(); + if map_host_func.contains_key(&key) { + map_host_func.remove(&key).expect( + "[wasmedge-sys] Failed to remove the host function from ASYNC_HOST_FUNCS container", + ); + } + } + + unsafe { + // drop host data + let _ = Box::from_raw(ffi::WasmEdge_FunctionInstanceGetData( + func.inner.lock().0, + ) + as *mut std::ffi::c_void); + } + } + } + } } } } @@ -461,6 +492,7 @@ impl AsImport for ImportModule { fn add_func(&mut self, name: impl AsRef, func: Function) { self.funcs.push(func); let f = self.funcs.last_mut().unwrap(); + f.registered = true; let func_name: WasmEdgeString = name.into(); unsafe { @@ -470,8 +502,6 @@ impl AsImport for ImportModule { f.inner.lock().0, ); } - - // ! Notice that, `f.inner.lock().0` is not set to null here as the pointer will be used in `Function::drop`. } fn add_table(&mut self, name: impl AsRef, table: Table) { From 150b0bbe62c9147c50db494203927a5a2887f873 Mon Sep 17 00:00:00 2001 From: Xin Liu Date: Thu, 2 Nov 2023 03:29:42 +0000 Subject: [PATCH 03/15] chore(rust-sdk): update test code Signed-off-by: Xin Liu --- src/externals/function.rs | 3 ++- src/vm.rs | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/externals/function.rs b/src/externals/function.rs index 6e9f894e8..c9d635194 100644 --- a/src/externals/function.rs +++ b/src/externals/function.rs @@ -671,7 +671,8 @@ mod tests { -> Box< (dyn std::future::Future, HostFuncError>> + Send), > { - let data = unsafe { Box::from_raw(data as *mut Data) }; + // Do not use `Box::from_raw`: let data = unsafe { Box::from_raw(data as *mut Data) }; + let data = unsafe { &mut *(data as *mut Data) }; Box::new(async move { for _ in 0..10 { diff --git a/src/vm.rs b/src/vm.rs index 6e9fa3a7e..ba18993b3 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -1003,8 +1003,10 @@ mod tests { Mutability, NeverType, RefType, Table, TableType, ValType, WasmValue, }; - #[cfg(target_os = "linux")] + #[ignore] #[test] + #[cfg(target_os = "linux")] + // To enable this test function, please install `wasi_crypto` plugin first. fn test_vmbuilder() -> Result<(), Box> { use crate::{ config::{CommonConfigOptions, ConfigBuilder, HostRegistrationConfigOptions}, From 68a8abdaf3e020f8bd9f34973bdbdf5cf757c166 Mon Sep 17 00:00:00 2001 From: Xin Liu Date: Mon, 6 Nov 2023 04:54:28 +0000 Subject: [PATCH 04/15] chore(rust-sys): update `drop` of `ImportModule` and `Function` Signed-off-by: Xin Liu --- crates/wasmedge-sys/src/instance/function.rs | 52 ++++++++++----- crates/wasmedge-sys/src/instance/module.rs | 66 +++++++++++++------- crates/wasmedge-sys/src/lib.rs | 5 +- src/vm.rs | 11 +++- 4 files changed, 89 insertions(+), 45 deletions(-) diff --git a/crates/wasmedge-sys/src/instance/function.rs b/crates/wasmedge-sys/src/instance/function.rs index bf9741ab4..968eaf08b 100644 --- a/crates/wasmedge-sys/src/instance/function.rs +++ b/crates/wasmedge-sys/src/instance/function.rs @@ -166,6 +166,7 @@ extern "C" fn wrap_async_fn( pub struct Function { pub(crate) inner: Arc>, pub(crate) registered: bool, + pub(crate) data_owner: bool, } impl Function { /// Creates a [host function](crate::Function) with the given function type. @@ -231,12 +232,12 @@ impl Function { data: Option>, cost: u64, ) -> WasmEdgeResult { - let data = match data { - Some(d) => Box::into_raw(d) as *mut std::ffi::c_void, - None => std::ptr::null_mut(), + let (data, data_owner) = match data { + Some(d) => (Box::into_raw(d) as *mut std::ffi::c_void, true), + None => (std::ptr::null_mut(), false), }; - unsafe { Self::create_with_data(ty, real_fn, data, cost) } + unsafe { Self::create_with_data(ty, real_fn, data, data_owner, cost) } } /// Creates a [host function](crate::Function) with the given function type. @@ -251,6 +252,8 @@ impl Function { /// /// * `data` - The pointer to the host context data used in this function. /// + /// * `data_owner` - Whether the host context data is owned by the host function. + /// /// * `cost` - The function cost in the [Statistics](crate::Statistics). Pass 0 if the calculation is not needed. /// /// # Error @@ -261,6 +264,7 @@ impl Function { ty: &FuncType, real_fn: BoxedFn, data: *mut c_void, + data_owner: bool, cost: u64, ) -> WasmEdgeResult { let mut map_host_func = HOST_FUNCS.write(); @@ -292,6 +296,7 @@ impl Function { false => Ok(Self { inner: Arc::new(Mutex::new(InnerFunc(ctx))), registered: false, + data_owner, }), } } @@ -320,9 +325,9 @@ impl Function { data: Option>, cost: u64, ) -> WasmEdgeResult { - let data = match data { - Some(d) => Box::into_raw(d) as *mut std::ffi::c_void, - None => std::ptr::null_mut(), + let (data, data_owner) = match data { + Some(d) => (Box::into_raw(d) as *mut std::ffi::c_void, true), + None => (std::ptr::null_mut(), false), }; let mut map_host_func = ASYNC_HOST_FUNCS.write(); @@ -356,6 +361,7 @@ impl Function { false => Ok(Self { inner: Arc::new(Mutex::new(InnerFunc(ctx))), registered: false, + data_owner, }), } } @@ -372,6 +378,8 @@ impl Function { /// /// * `data` - The pointer to the host context data used in this function. /// + /// * `data_owner` - Whether the host context data is owned by the host function. + /// /// * `cost` - The function cost in the [Statistics](crate::Statistics). Pass 0 if the calculation is not needed. /// /// # Error @@ -387,6 +395,7 @@ impl Function { fn_wrapper: CustomFnWrapper, real_fn: *mut c_void, data: *mut c_void, + data_owner: bool, cost: u64, ) -> WasmEdgeResult { let ctx = ffi::WasmEdge_FunctionInstanceCreateBinding( @@ -402,6 +411,7 @@ impl Function { false => Ok(Self { inner: Arc::new(Mutex::new(InnerFunc(ctx))), registered: false, + data_owner, }), } } @@ -486,6 +496,7 @@ impl Function { } impl Drop for Function { fn drop(&mut self) { + dbg!("func registered: {}", self.registered); if !self.registered && Arc::strong_count(&self.inner) == 1 { // remove the real_func from HOST_FUNCS let footprint = self.inner.lock().0 as usize; @@ -493,8 +504,8 @@ impl Drop for Function { let mut map_host_func = HOST_FUNCS.write(); if map_host_func.contains_key(&key) { map_host_func.remove(&key).expect( - "[wasmedge-sys] Failed to remove the host function from HOST_FUNCS_NEW container", - ); + "[wasmedge-sys] Failed to remove the host function from HOST_FUNCS_NEW container", + ); } #[cfg(all(feature = "async", target_os = "linux"))] @@ -502,20 +513,26 @@ impl Drop for Function { let mut map_host_func = ASYNC_HOST_FUNCS.write(); if map_host_func.contains_key(&key) { map_host_func.remove(&key).expect( - "[wasmedge-sys] Failed to remove the host function from ASYNC_HOST_FUNCS container", - ); + "[wasmedge-sys] Failed to remove the host function from ASYNC_HOST_FUNCS container", + ); } } + } else { + panic!("[wasmedge-sys] Failed to remove the host function from HOST_FUNC_FOOTPRINTS container"); + } + + // drop host data + if self.data_owner { + let _ = unsafe { + Box::from_raw( + ffi::WasmEdge_FunctionInstanceGetData(self.inner.lock().0) as *mut c_void + ) + }; } // delete the function instance if !self.inner.lock().0.is_null() { unsafe { - // drop host data - let _ = - Box::from_raw(ffi::WasmEdge_FunctionInstanceGetData(self.inner.lock().0) - as *mut c_void); - ffi::WasmEdge_FunctionInstanceDelete(self.inner.lock().0); }; } @@ -527,6 +544,7 @@ impl Clone for Function { Self { inner: self.inner.clone(), registered: self.registered, + data_owner: self.data_owner, } } } @@ -1291,12 +1309,14 @@ mod tests { assert_eq!(HOST_FUNC_FOOTPRINTS.lock().len(), 1); // ! notice that `add_again` should be dropped before or not be used after dropping `import` + dbg!("drop add_again"); drop(add_again); assert_eq!(HOST_FUNCS.read().len(), 1); assert_eq!(HOST_FUNC_FOOTPRINTS.lock().len(), 1); // drop the import object + dbg!("drop import"); drop(import); assert!(store.module("extern").is_err()); diff --git a/crates/wasmedge-sys/src/instance/module.rs b/crates/wasmedge-sys/src/instance/module.rs index d1152ea64..482d949e5 100644 --- a/crates/wasmedge-sys/src/instance/module.rs +++ b/crates/wasmedge-sys/src/instance/module.rs @@ -69,6 +69,7 @@ impl Instance { false => Ok(Function { inner: Arc::new(Mutex::new(InnerFunc(func_ctx))), registered: true, + data_owner: false, // it doesn't matter to set this field to false }), } } @@ -393,41 +394,59 @@ pub struct ImportModule { } impl Drop for ImportModule { fn drop(&mut self) { + dbg!("import module registered: {}", self.registered); + dbg!( + "import module strong count: {}", + Arc::strong_count(&self.inner) + ); + if !self.registered && Arc::strong_count(&self.inner) == 1 && !self.inner.0.is_null() { // free the module instance unsafe { ffi::WasmEdge_ModuleInstanceDelete(self.inner.0); } - // drop the registered host functions + dbg!("len of funcs: {}", self.funcs.len()); + + // cleanup the stuff belonging to each host function before really dropping the host function for func in self.funcs.iter() { - if func.registered && Arc::strong_count(&func.inner) == 1 { - // remove the real_func from HOST_FUNCS - let footprint = func.inner.lock().0 as usize; - if let Some(key) = HOST_FUNC_FOOTPRINTS.lock().remove(&footprint) { - let mut map_host_func = HOST_FUNCS.write(); - if map_host_func.contains_key(&key) { - map_host_func.remove(&key).expect( + assert_eq!(Arc::strong_count(&func.inner), 1, "[wasmedge-sys] The host function is still in use while dropping the import module"); + assert!(func.registered, "[wasmedge-sys] Trying to drop a non-registered host function while dropping the import module"); + + // remove the real_func from HOST_FUNCS + let footprint = func.inner.lock().0 as usize; + + dbg!("footprint: {}", footprint); + + if let Some(key) = HOST_FUNC_FOOTPRINTS.lock().remove(&footprint) { + dbg!("remove the host function from HOST_FUNCS container"); + + if func.data_owner { + dbg!(func.inner.lock().0); + unsafe { + // drop host data + dbg!("drop host data while dropping the import module"); + let p = ffi::WasmEdge_FunctionInstanceGetData(func.inner.lock().0); + dbg!("start box::from_raw"); + let _ = Box::from_raw(p as *mut std::ffi::c_void); + dbg!("Done! drop host data while dropping the import module"); + } + } + + let mut map_host_func = HOST_FUNCS.write(); + if map_host_func.contains_key(&key) { + map_host_func.remove(&key).expect( "[wasmedge-sys] Failed to remove the host function from HOST_FUNCS_NEW container", ); - } + } - #[cfg(all(feature = "async", target_os = "linux"))] - { - let mut map_host_func = ASYNC_HOST_FUNCS.write(); - if map_host_func.contains_key(&key) { - map_host_func.remove(&key).expect( + #[cfg(all(feature = "async", target_os = "linux"))] + { + let mut map_host_func = ASYNC_HOST_FUNCS.write(); + if map_host_func.contains_key(&key) { + map_host_func.remove(&key).expect( "[wasmedge-sys] Failed to remove the host function from ASYNC_HOST_FUNCS container", ); - } - } - - unsafe { - // drop host data - let _ = Box::from_raw(ffi::WasmEdge_FunctionInstanceGetData( - func.inner.lock().0, - ) - as *mut std::ffi::c_void); } } } @@ -493,6 +512,7 @@ impl AsImport for ImportModule { self.funcs.push(func); let f = self.funcs.last_mut().unwrap(); f.registered = true; + dbg!(f.data_owner); let func_name: WasmEdgeString = name.into(); unsafe { diff --git a/crates/wasmedge-sys/src/lib.rs b/crates/wasmedge-sys/src/lib.rs index b4b696a62..6298e9583 100644 --- a/crates/wasmedge-sys/src/lib.rs +++ b/crates/wasmedge-sys/src/lib.rs @@ -142,7 +142,7 @@ pub(crate) type BoxedFn = Box< >; lazy_static! { - pub(crate) static ref HOST_FUNCS: RwLock>>> = + pub static ref HOST_FUNCS: RwLock>>> = RwLock::new(HashMap::new()); } @@ -165,8 +165,7 @@ lazy_static! { // Stores the mapping from the address of each host function pointer to the key of the `HOST_FUNCS`. lazy_static! { - pub(crate) static ref HOST_FUNC_FOOTPRINTS: Mutex> = - Mutex::new(HashMap::new()); + pub static ref HOST_FUNC_FOOTPRINTS: Mutex> = Mutex::new(HashMap::new()); } /// The object that is used to perform a [host function](crate::Function) is required to implement this trait. diff --git a/src/vm.rs b/src/vm.rs index ba18993b3..23a3eb669 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -502,9 +502,14 @@ impl Vm { ) -> WasmEdgeResult> { match mod_name { Some(mod_name) => match self.named_instances.get(mod_name) { - Some(named_instance) => named_instance - .func(func_name.as_ref())? - .run(self.executor(), args), + Some(named_instance) => { + let named_func = named_instance.func(func_name.as_ref())?; + let res = named_func.run(self.executor(), args); + dbg!("drop named_func"); + drop(named_func); + dbg!("DONE! drop anmed_func"); + res + } None => Err(Box::new(WasmEdgeError::Vm(VmError::NotFoundModule( mod_name.into(), )))), From 1078cdbcf57cc74a7352a43a63a9521a0da79c3a Mon Sep 17 00:00:00 2001 From: Xin Liu Date: Tue, 7 Nov 2023 03:56:37 +0000 Subject: [PATCH 05/15] refactor(rust-sys): update `ImportModule::drop` Signed-off-by: Xin Liu --- crates/wasmedge-sys/src/instance/module.rs | 111 +++++++++++++++------ 1 file changed, 80 insertions(+), 31 deletions(-) diff --git a/crates/wasmedge-sys/src/instance/module.rs b/crates/wasmedge-sys/src/instance/module.rs index 482d949e5..d2b5404e6 100644 --- a/crates/wasmedge-sys/src/instance/module.rs +++ b/crates/wasmedge-sys/src/instance/module.rs @@ -394,48 +394,27 @@ pub struct ImportModule { } impl Drop for ImportModule { fn drop(&mut self) { - dbg!("import module registered: {}", self.registered); - dbg!( - "import module strong count: {}", - Arc::strong_count(&self.inner) - ); - if !self.registered && Arc::strong_count(&self.inner) == 1 && !self.inner.0.is_null() { - // free the module instance - unsafe { - ffi::WasmEdge_ModuleInstanceDelete(self.inner.0); - } - - dbg!("len of funcs: {}", self.funcs.len()); - // cleanup the stuff belonging to each host function before really dropping the host function for func in self.funcs.iter() { assert_eq!(Arc::strong_count(&func.inner), 1, "[wasmedge-sys] The host function is still in use while dropping the import module"); assert!(func.registered, "[wasmedge-sys] Trying to drop a non-registered host function while dropping the import module"); + if func.data_owner { + unsafe { + // drop host data + let p = ffi::WasmEdge_FunctionInstanceGetData(func.inner.lock().0); + let _ = Box::from_raw(p as *mut std::ffi::c_void); + } + } + // remove the real_func from HOST_FUNCS let footprint = func.inner.lock().0 as usize; - dbg!("footprint: {}", footprint); - if let Some(key) = HOST_FUNC_FOOTPRINTS.lock().remove(&footprint) { - dbg!("remove the host function from HOST_FUNCS container"); - - if func.data_owner { - dbg!(func.inner.lock().0); - unsafe { - // drop host data - dbg!("drop host data while dropping the import module"); - let p = ffi::WasmEdge_FunctionInstanceGetData(func.inner.lock().0); - dbg!("start box::from_raw"); - let _ = Box::from_raw(p as *mut std::ffi::c_void); - dbg!("Done! drop host data while dropping the import module"); - } - } - let mut map_host_func = HOST_FUNCS.write(); if map_host_func.contains_key(&key) { - map_host_func.remove(&key).expect( + let _ = map_host_func.remove(&key).expect( "[wasmedge-sys] Failed to remove the host function from HOST_FUNCS_NEW container", ); } @@ -451,6 +430,11 @@ impl Drop for ImportModule { } } } + + // free the module instance + unsafe { + ffi::WasmEdge_ModuleInstanceDelete(self.inner.0); + } } } } @@ -512,7 +496,6 @@ impl AsImport for ImportModule { self.funcs.push(func); let f = self.funcs.last_mut().unwrap(); f.registered = true; - dbg!(f.data_owner); let func_name: WasmEdgeString = name.into(); unsafe { @@ -1474,6 +1457,7 @@ mod tests { drop(import); assert_eq!(Arc::strong_count(&import_clone.inner), 1); + drop(import_clone); } @@ -1562,4 +1546,69 @@ mod tests { let host_data = result.unwrap(); assert_eq!(host_data.radius, 10); } + + #[test] + fn test_instance_valgrind_memory_check() { + // define host data + #[derive(Clone, Debug)] + struct Circle { + radius: i32, + } + impl Drop for Circle { + fn drop(&mut self) { + println!("dropping circle: {}", self.radius); + } + } + + #[sys_host_function] + fn real_add_new( + _frame: CallingFrame, + inputs: Vec, + data: &mut Circle, + ) -> Result, HostFuncError> { + println!("radius of circle: {}", data.radius); + + if inputs.len() != 2 { + return Err(HostFuncError::User(1)); + } + + let a = if inputs[0].ty() == ValType::I32 { + inputs[0].to_i32() + } else { + return Err(HostFuncError::User(2)); + }; + + let b = if inputs[1].ty() == ValType::I32 { + inputs[1].to_i32() + } else { + return Err(HostFuncError::User(3)); + }; + + let c = a + b; + + Ok(vec![WasmValue::from_i32(c)]) + } + + // create an import module + let host_name = "extern"; + let result = ImportModule::::create(host_name, None); + assert!(result.is_ok()); + let mut import = result.unwrap(); + + // create a host function + let result = FuncType::create([ValType::ExternRef, ValType::I32], [ValType::I32]); + assert!(result.is_ok()); + let func_ty = result.unwrap(); + let result = Function::create_sync_func::( + &func_ty, + Box::new(real_add_new), + Some(Box::new(Circle { radius: 10 })), + 0, + ); + let host_func = result.unwrap(); + + // add the host function + import.add_func("func-add", host_func); + drop(import); + } } From 56f3e286bd9b5cd709aa49415cfd93da50592895 Mon Sep 17 00:00:00 2001 From: Xin Liu Date: Tue, 7 Nov 2023 03:57:04 +0000 Subject: [PATCH 06/15] chore(rust-sys): remove debug code Signed-off-by: Xin Liu --- crates/wasmedge-sys/src/instance/function.rs | 1 - crates/wasmedge-sys/src/lib.rs | 5 +++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/wasmedge-sys/src/instance/function.rs b/crates/wasmedge-sys/src/instance/function.rs index 968eaf08b..6a95d87a5 100644 --- a/crates/wasmedge-sys/src/instance/function.rs +++ b/crates/wasmedge-sys/src/instance/function.rs @@ -496,7 +496,6 @@ impl Function { } impl Drop for Function { fn drop(&mut self) { - dbg!("func registered: {}", self.registered); if !self.registered && Arc::strong_count(&self.inner) == 1 { // remove the real_func from HOST_FUNCS let footprint = self.inner.lock().0 as usize; diff --git a/crates/wasmedge-sys/src/lib.rs b/crates/wasmedge-sys/src/lib.rs index 6298e9583..b4b696a62 100644 --- a/crates/wasmedge-sys/src/lib.rs +++ b/crates/wasmedge-sys/src/lib.rs @@ -142,7 +142,7 @@ pub(crate) type BoxedFn = Box< >; lazy_static! { - pub static ref HOST_FUNCS: RwLock>>> = + pub(crate) static ref HOST_FUNCS: RwLock>>> = RwLock::new(HashMap::new()); } @@ -165,7 +165,8 @@ lazy_static! { // Stores the mapping from the address of each host function pointer to the key of the `HOST_FUNCS`. lazy_static! { - pub static ref HOST_FUNC_FOOTPRINTS: Mutex> = Mutex::new(HashMap::new()); + pub(crate) static ref HOST_FUNC_FOOTPRINTS: Mutex> = + Mutex::new(HashMap::new()); } /// The object that is used to perform a [host function](crate::Function) is required to implement this trait. From c41e2abb33c0076434c44b39aefb843bb12d81cf Mon Sep 17 00:00:00 2001 From: Xin Liu Date: Tue, 7 Nov 2023 03:57:25 +0000 Subject: [PATCH 07/15] chore(rust-sdk): remove debug code Signed-off-by: Xin Liu --- src/vm.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/vm.rs b/src/vm.rs index 23a3eb669..1b59d70db 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -505,9 +505,7 @@ impl Vm { Some(named_instance) => { let named_func = named_instance.func(func_name.as_ref())?; let res = named_func.run(self.executor(), args); - dbg!("drop named_func"); drop(named_func); - dbg!("DONE! drop anmed_func"); res } None => Err(Box::new(WasmEdgeError::Vm(VmError::NotFoundModule( From f503c4d306598a18f4cd7b619bbbe8bf110c2b9b Mon Sep 17 00:00:00 2001 From: Xin Liu Date: Tue, 7 Nov 2023 04:07:48 +0000 Subject: [PATCH 08/15] version(rust-sdk): bump to `0.13.0` Signed-off-by: Xin Liu --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 4e309c1f5..8d848bea9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,7 +8,7 @@ license = "Apache-2.0" name = "wasmedge-sdk" readme = "README.md" repository = "https://github.com/WasmEdge/wasmedge-rust-sdk" -version = "0.12.3-dev" +version = "0.13.0" [dependencies] anyhow = "1.0" From 59395caae5f37579f651dc87be111bc433fd8972 Mon Sep 17 00:00:00 2001 From: Xin Liu Date: Tue, 7 Nov 2023 09:30:07 +0000 Subject: [PATCH 09/15] chore(rust-sys): supress clippy warning Signed-off-by: Xin Liu --- crates/wasmedge-sys/src/instance/function.rs | 1 + crates/wasmedge-sys/src/instance/module.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/crates/wasmedge-sys/src/instance/function.rs b/crates/wasmedge-sys/src/instance/function.rs index 6a95d87a5..8bcc82f4a 100644 --- a/crates/wasmedge-sys/src/instance/function.rs +++ b/crates/wasmedge-sys/src/instance/function.rs @@ -495,6 +495,7 @@ impl Function { } } impl Drop for Function { + #[allow(clippy::from_raw_with_void_ptr)] fn drop(&mut self) { if !self.registered && Arc::strong_count(&self.inner) == 1 { // remove the real_func from HOST_FUNCS diff --git a/crates/wasmedge-sys/src/instance/module.rs b/crates/wasmedge-sys/src/instance/module.rs index d2b5404e6..f182615d5 100644 --- a/crates/wasmedge-sys/src/instance/module.rs +++ b/crates/wasmedge-sys/src/instance/module.rs @@ -393,6 +393,7 @@ pub struct ImportModule { data: Option>, } impl Drop for ImportModule { + #[allow(clippy::from_raw_with_void_ptr)] fn drop(&mut self) { if !self.registered && Arc::strong_count(&self.inner) == 1 && !self.inner.0.is_null() { // cleanup the stuff belonging to each host function before really dropping the host function From f240c8f0de31bf6ecc1917258ec86898cc5d984f Mon Sep 17 00:00:00 2001 From: Xin Liu Date: Tue, 7 Nov 2023 09:38:41 +0000 Subject: [PATCH 10/15] ci(ci-build): update rust version Signed-off-by: Xin Liu --- .github/workflows/ci-build.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci-build.yml b/.github/workflows/ci-build.yml index 19367fbcf..289c38f98 100644 --- a/.github/workflows/ci-build.yml +++ b/.github/workflows/ci-build.yml @@ -21,7 +21,7 @@ jobs: strategy: matrix: os: [ubuntu-22.04, ubuntu-20.04] - rust: [1.72, 1.71, 1.70.0] + rust: [1.73, 1.72, 1.71] container: image: wasmedge/wasmedge:ubuntu-build-clang @@ -90,7 +90,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - rust: [1.72, 1.71, 1.70.0] + rust: [1.73, 1.72, 1.71] container: image: fedora:latest @@ -167,7 +167,7 @@ jobs: strategy: matrix: os: [macos-11, macos-12] - rust: [1.72, 1.71, 1.70.0] + rust: [1.73, 1.72, 1.71] steps: - name: Checkout sources @@ -222,7 +222,7 @@ jobs: runs-on: windows-2022 strategy: matrix: - rust: [1.72, 1.71, 1.70.0] + rust: [1.73, 1.72, 1.71] env: WASMEDGE_DIR: ${{ github.workspace }}\WasmEdge WASMEDGE_BUILD_DIR: ${{ github.workspace }}\WasmEdge\build From 8b711ede19415ec73318a085f6695dfd3d5f034f Mon Sep 17 00:00:00 2001 From: Xin Liu Date: Tue, 7 Nov 2023 09:38:58 +0000 Subject: [PATCH 11/15] ci(standalone): update rust version Signed-off-by: Xin Liu --- .github/workflows/standalone.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/standalone.yml b/.github/workflows/standalone.yml index 8d172e6a3..b4ee4efe0 100644 --- a/.github/workflows/standalone.yml +++ b/.github/workflows/standalone.yml @@ -21,7 +21,7 @@ jobs: runs-on: ubuntu-22.04 strategy: matrix: - rust: [1.72, 1.71, 1.70.0] + rust: [1.73, 1.72, 1.71] steps: - name: Checkout WasmEdge Rust SDK @@ -55,7 +55,7 @@ jobs: runs-on: ubuntu-20.04 strategy: matrix: - rust: [1.72, 1.71, 1.70.0] + rust: [1.73, 1.72, 1.71] steps: - name: Checkout WasmEdge Rust SDK @@ -90,7 +90,7 @@ jobs: strategy: matrix: os: [macos-11, macos-12] - rust: [1.72, 1.71, 1.70.0] + rust: [1.73, 1.72, 1.71] steps: - name: Checkout sources @@ -119,7 +119,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - rust: [1.72, 1.71, 1.70.0] + rust: [1.73, 1.72, 1.71] container: image: fedora:latest From 0a2153c544894ea11070660b560118759d301b92 Mon Sep 17 00:00:00 2001 From: Xin Liu Date: Tue, 7 Nov 2023 09:40:58 +0000 Subject: [PATCH 12/15] chore(rust-sys): update rustdoc Signed-off-by: Xin Liu --- crates/wasmedge-sys/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/wasmedge-sys/src/lib.rs b/crates/wasmedge-sys/src/lib.rs index b4b696a62..256340ecf 100644 --- a/crates/wasmedge-sys/src/lib.rs +++ b/crates/wasmedge-sys/src/lib.rs @@ -8,7 +8,7 @@ //! //! For developers, it is strongly recommended that the APIs in `wasmedge-sys` are used to construct high-level libraries, while `wasmedge-sdk` is for building up business applications. //! -//! * Notice that [wasmedge-sys](https://crates.io/crates/wasmedge-sys) requires **Rust v1.70 or above** in the **stable** channel. +//! * Notice that [wasmedge-sys](https://crates.io/crates/wasmedge-sys) requires **Rust v1.71 or above** in the **stable** channel. //! //! ## Build @@ -19,6 +19,7 @@ //! //! | wasmedge-sdk | WasmEdge lib | wasmedge-sys | wasmedge-types| wasmedge-macro| async-wasi| //! | :-----------: | :-----------: | :-----------: | :-----------: | :-----------: | :-------: | +//! | 0.13.0 | 0.13.5 | 0.17.3 | 0.4.4 | 0.6.1 | 0.1.0 | //! | 0.12.2 | 0.13.4 | 0.17.2 | 0.4.4 | 0.6.1 | 0.1.0 | //! | 0.12.1 | 0.13.4 | 0.17.1 | 0.4.4 | 0.6.1 | 0.1.0 | //! | 0.12.0 | 0.13.4 | 0.17.0 | 0.4.4 | 0.6.1 | 0.1.0 | From 73c88dbfcc4e3bbcc49be69de4c3daf71b21ec0e Mon Sep 17 00:00:00 2001 From: Xin Liu Date: Tue, 7 Nov 2023 09:41:35 +0000 Subject: [PATCH 13/15] chore(rust-sdk): update rustdoc and `README` Signed-off-by: Xin Liu --- README.md | 3 ++- src/lib.rs | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index e06749f53..b5506cf49 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,7 @@ This crate depends on the WasmEdge C API. In linux/macOS the crate can download | wasmedge-sdk | WasmEdge lib | wasmedge-sys | wasmedge-types| wasmedge-macro| async-wasi| | :-----------: | :-----------: | :-----------: | :-----------: | :-----------: | :-------: | + | 0.13.0 | 0.13.5 | 0.17.3 | 0.4.4 | 0.6.1 | 0.1.0 | | 0.12.2 | 0.13.4 | 0.17.2 | 0.4.4 | 0.6.1 | 0.1.0 | | 0.12.1 | 0.13.4 | 0.17.1 | 0.4.4 | 0.6.1 | 0.1.0 | | 0.12.0 | 0.13.4 | 0.17.0 | 0.4.4 | 0.6.1 | 0.1.0 | @@ -58,7 +59,7 @@ The following architectures are supported for automatic downloads: This crate uses `rust-bindgen` during the build process. If you would like to use an external `rust-bindgen` you can set the `WASMEDGE_RUST_BINDGEN_PATH` environment variable to the `bindgen` executable path. This is particularly useful in systems like Alpine Linux (see [rust-lang/rust-bindgen#2360](https://github.com/rust-lang/rust-bindgen/issues/2360#issuecomment-1595869379), [rust-lang/rust-bindgen#2333](https://github.com/rust-lang/rust-bindgen/issues/2333)). -**Notice:** The minimum supported Rust version is 1.70. +**Notice:** The minimum supported Rust version is 1.71. ## API Reference diff --git a/src/lib.rs b/src/lib.rs index 8b19ac227..9e556b9f0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -25,6 +25,7 @@ //! //! | wasmedge-sdk | WasmEdge lib | wasmedge-sys | wasmedge-types| wasmedge-macro| async-wasi| //! | :-----------: | :-----------: | :-----------: | :-----------: | :-----------: | :-------: | +//! | 0.13.0 | 0.13.5 | 0.17.3 | 0.4.4 | 0.6.1 | 0.1.0 | //! | 0.12.2 | 0.13.4 | 0.17.2 | 0.4.4 | 0.6.1 | 0.1.0 | //! | 0.12.1 | 0.13.4 | 0.17.1 | 0.4.4 | 0.6.1 | 0.1.0 | //! | 0.12.0 | 0.13.4 | 0.17.0 | 0.4.4 | 0.6.1 | 0.1.0 | @@ -67,7 +68,7 @@ //! //! This crate uses `rust-bindgen` during the build process. If you would like to use an external `rust-bindgen` you can set the `WASMEDGE_RUST_BINDGEN_PATH` environment variable to the `bindgen` executable path. This is particularly useful in systems like Alpine Linux (see [rust-lang/rust-bindgen#2360](https://github.com/rust-lang/rust-bindgen/issues/2360#issuecomment-1595869379), [rust-lang/rust-bindgen#2333](https://github.com/rust-lang/rust-bindgen/issues/2333)). //! -//! **Notice:** The minimum supported Rust version is 1.70. +//! **Notice:** The minimum supported Rust version is 1.71. //! //! ## API Reference //! From 60d5003883eef42ce3fee9a4aadb4ba78db0832d Mon Sep 17 00:00:00 2001 From: Xin Liu Date: Tue, 7 Nov 2023 14:01:26 +0000 Subject: [PATCH 14/15] chore(rust-sys): update dependencies Signed-off-by: Xin Liu --- crates/wasmedge-sys/Cargo.toml | 4 ++-- crates/wasmedge-sys/build.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/wasmedge-sys/Cargo.toml b/crates/wasmedge-sys/Cargo.toml index f60439006..f1c4a61de 100644 --- a/crates/wasmedge-sys/Cargo.toml +++ b/crates/wasmedge-sys/Cargo.toml @@ -13,7 +13,7 @@ repository = "https://github.com/WasmEdge/wasmedge-rust-sdk" version = "0.17.3" [dependencies] -fiber-for-wasmedge = { version = "8.0.1", optional = true } +fiber-for-wasmedge = { version = "14.0.4", optional = true } libc = "0.2.94" paste = "1.0.5" scoped-tls = "1" @@ -32,7 +32,7 @@ async-wasi = { workspace = true, optional = true } setjmp = "0.1" [build-dependencies] -bindgen = { version = "0.65", default-features = false, features = ["runtime"] } +bindgen = { version = "0.69", default-features = false, features = ["runtime"] } cmake = "0.1" reqwest = { version = "0.11", default-features = false, features = [ "blocking", diff --git a/crates/wasmedge-sys/build.rs b/crates/wasmedge-sys/build.rs index a66cf8cda..d39b6c566 100644 --- a/crates/wasmedge-sys/build.rs +++ b/crates/wasmedge-sys/build.rs @@ -136,7 +136,7 @@ fn main() { .clang_arg(format!("-I{inc_dir}")) .prepend_enum_name(false) // The API already prepends the name. .dynamic_link_require_all(true) - .parse_callbacks(Box::new(bindgen::CargoCallbacks)) + .parse_callbacks(Box::new(bindgen::CargoCallbacks::new())) .generate() .expect("failed to generate bindings") .write_to_file(out_file) From dcb751f923618fde6c342a37219255f721788b61 Mon Sep 17 00:00:00 2001 From: Xin Liu Date: Tue, 7 Nov 2023 15:08:27 +0000 Subject: [PATCH 15/15] ci: update to `macos-13` Signed-off-by: Xin Liu --- .github/workflows/ci-build.yml | 2 +- .github/workflows/standalone.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci-build.yml b/.github/workflows/ci-build.yml index 289c38f98..285880db9 100644 --- a/.github/workflows/ci-build.yml +++ b/.github/workflows/ci-build.yml @@ -166,7 +166,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [macos-11, macos-12] + os: [macos-12, macos-13] rust: [1.73, 1.72, 1.71] steps: diff --git a/.github/workflows/standalone.yml b/.github/workflows/standalone.yml index b4ee4efe0..e8d6f74c9 100644 --- a/.github/workflows/standalone.yml +++ b/.github/workflows/standalone.yml @@ -89,7 +89,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [macos-11, macos-12] + os: [macos-12, macos-13] rust: [1.73, 1.72, 1.71] steps: