From 3ac3d9abcf1ced68d7a9cac098c61c920f74512e Mon Sep 17 00:00:00 2001 From: zach Date: Mon, 23 Sep 2024 10:42:45 -0700 Subject: [PATCH] cleanup: host takes ownership of memory blocks it gets as arguments (#743) This PR changes the host to take ownership of memory blocks passed into Extism host functions, see: https://github.com/extism/js-sdk/pull/71#issuecomment-2233687933 --- kernel/src/lib.rs | 6 ++++ runtime/src/extism-runtime.wasm | Bin runtime/src/pdk.rs | 53 +++++++++++++++++++++++++++----- 3 files changed, 51 insertions(+), 8 deletions(-) mode change 100755 => 100644 runtime/src/extism-runtime.wasm diff --git a/kernel/src/lib.rs b/kernel/src/lib.rs index 5b9c81b..96542f7 100644 --- a/kernel/src/lib.rs +++ b/kernel/src/lib.rs @@ -495,6 +495,8 @@ pub unsafe fn store_u64(p: Pointer, x: u64) { /// Set the range of the input data in memory /// h must always be a handle so that length works on it /// len must match length(handle) +/// **Note**: this function takes ownership of the handle passed in +/// the caller should not `free` this value #[no_mangle] pub unsafe fn input_set(h: Handle, len: u64) { let root = MemoryRoot::new(); @@ -509,6 +511,8 @@ pub unsafe fn input_set(h: Handle, len: u64) { } /// Set the range of the output data in memory +/// **Note**: this function takes ownership of the handle passed in +/// the caller should not `free` this value #[no_mangle] pub unsafe fn output_set(p: Pointer, len: u64) { let root = MemoryRoot::new(); @@ -554,6 +558,8 @@ pub unsafe fn reset() { /// Set the error message offset, the handle passed to this /// function should not be freed after this call +/// **Note**: this function takes ownership of the handle passed in +/// the caller should not `free` this value #[no_mangle] pub unsafe fn error_set(h: Handle) { let root = MemoryRoot::new(); diff --git a/runtime/src/extism-runtime.wasm b/runtime/src/extism-runtime.wasm old mode 100755 new mode 100644 diff --git a/runtime/src/pdk.rs b/runtime/src/pdk.rs index ecd99e4..28881f3 100644 --- a/runtime/src/pdk.rs +++ b/runtime/src/pdk.rs @@ -20,6 +20,8 @@ macro_rules! args { /// Get a configuration value /// Params: i64 (offset) /// Returns: i64 (offset) +/// **Note**: this function takes ownership of the handle passed in +/// the caller should not `free` this value pub(crate) fn config_get( mut caller: Caller, input: &[Val], @@ -38,6 +40,7 @@ pub(crate) fn config_get( }; let val = data.manifest.config.get(key); let ptr = val.map(|x| (x.len(), x.as_ptr())); + data.memory_free(handle)?; let mem = match ptr { Some((len, ptr)) => { let bytes = unsafe { std::slice::from_raw_parts(ptr, len) }; @@ -55,6 +58,9 @@ pub(crate) fn config_get( /// Get a variable /// Params: i64 (offset) /// Returns: i64 (offset) +/// **Note**: this function takes ownership of the handle passed in +/// the caller should not `free` this value, but the return value +/// will need to be freed pub(crate) fn var_get( mut caller: Caller, input: &[Val], @@ -73,6 +79,8 @@ pub(crate) fn var_get( }; let val = data.vars.get(key); let ptr = val.map(|x| (x.len(), x.as_ptr())); + data.memory_free(handle)?; + let mem = match ptr { Some((len, ptr)) => { let bytes = unsafe { std::slice::from_raw_parts(ptr, len) }; @@ -90,6 +98,8 @@ pub(crate) fn var_get( /// Set a variable, if the value offset is 0 then the provided key will be removed /// Params: i64 (key offset), i64 (value offset) /// Returns: none +/// **Note**: this function takes ownership of the handles passed in +/// the caller should not `free` these values pub(crate) fn var_set( mut caller: Caller, input: &[Val], @@ -104,12 +114,12 @@ pub(crate) fn var_set( let voffset = args!(input, 1, i64) as u64; let key_offs = args!(input, 0, i64) as u64; + let key_handle = match data.memory_handle(key_offs) { + Some(h) => h, + None => anyhow::bail!("invalid handle offset for var key: {key_offs}"), + }; let key = { - let handle = match data.memory_handle(key_offs) { - Some(h) => h, - None => anyhow::bail!("invalid handle offset for var key: {key_offs}"), - }; - let key = data.memory_str(handle)?; + let key = data.memory_str(key_handle)?; let key_len = key.len(); let key_ptr = key.as_ptr(); unsafe { std::str::from_utf8_unchecked(std::slice::from_raw_parts(key_ptr, key_len)) } @@ -118,6 +128,7 @@ pub(crate) fn var_set( // Remove if the value offset is 0 if voffset == 0 { data.vars.remove(key); + data.memory_free(key_handle)?; return Ok(()); } @@ -144,6 +155,9 @@ pub(crate) fn var_set( let value = data.memory_bytes(handle)?.to_vec(); + data.memory_free(handle)?; + data.memory_free(key_handle)?; + // Insert the value from memory into the `vars` map data.vars.insert(key.to_string(), value); @@ -153,6 +167,9 @@ pub(crate) fn var_set( /// Make an HTTP request /// Params: i64 (offset to JSON encoded HttpRequest), i64 (offset to body or 0) /// Returns: i64 (offset) +/// **Note**: this function takes ownership of the handles passed in +/// the caller should not `free` these values, the result will need to +/// be freed. pub(crate) fn http_request( #[allow(unused_mut)] mut caller: Caller, input: &[Val], @@ -166,6 +183,7 @@ pub(crate) fn http_request( Some(h) => h, None => anyhow::bail!("http_request input is invalid: {http_req_offset}"), }; + data.free(handle)?; let req: extism_manifest::HttpRequest = serde_json::from_slice(data.memory_bytes(handle)?)?; output[0] = Val::I64(0); anyhow::bail!( @@ -182,6 +200,7 @@ pub(crate) fn http_request( None => anyhow::bail!("invalid handle offset for http request: {http_req_offset}"), }; let req: extism_manifest::HttpRequest = serde_json::from_slice(data.memory_bytes(handle)?)?; + data.memory_free(handle)?; let body_offset = args!(input, 1, i64) as u64; @@ -235,6 +254,10 @@ pub(crate) fn http_request( r.call() }; + if let Some(handle) = data.memory_handle(body_offset) { + data.memory_free(handle)?; + } + let reader = match res { Ok(res) => { data.http_status = res.status(); @@ -302,19 +325,21 @@ pub fn log( ) -> Result<(), Error> { let data: &mut CurrentPlugin = caller.data_mut(); + let offset = args!(input, 0, i64) as u64; + // Check if the current log level should be logged let global_log_level = tracing::level_filters::LevelFilter::current(); if global_log_level == tracing::level_filters::LevelFilter::OFF || level > global_log_level { + if let Some(handle) = data.memory_handle(offset) { + data.memory_free(handle)?; + } return Ok(()); } - let offset = args!(input, 0, i64) as u64; - let handle = match data.memory_handle(offset) { Some(h) => h, None => anyhow::bail!("invalid handle offset for log message: {offset}"), }; - let id = data.id.to_string(); let buf = data.memory_str(handle); @@ -338,12 +363,16 @@ pub fn log( }, Err(_) => tracing::error!(plugin = id, "unable to log message: {:?}", buf), } + + data.memory_free(handle)?; Ok(()) } /// Write to logs (warning) /// Params: i64 (offset) /// Returns: none +/// **Note**: this function takes ownership of the handle passed in +/// the caller should not `free` this value pub(crate) fn log_warn( caller: Caller, input: &[Val], @@ -355,6 +384,8 @@ pub(crate) fn log_warn( /// Write to logs (info) /// Params: i64 (offset) /// Returns: none +/// **Note**: this function takes ownership of the handle passed in +/// the caller should not `free` this value pub(crate) fn log_info( caller: Caller, input: &[Val], @@ -366,6 +397,8 @@ pub(crate) fn log_info( /// Write to logs (debug) /// Params: i64 (offset) /// Returns: none +/// **Note**: this function takes ownership of the handle passed in +/// the caller should not `free` this value pub(crate) fn log_debug( caller: Caller, input: &[Val], @@ -377,6 +410,8 @@ pub(crate) fn log_debug( /// Write to logs (error) /// Params: i64 (offset) /// Returns: none +/// **Note**: this function takes ownership of the handle passed in +/// the caller should not `free` this value pub(crate) fn log_error( caller: Caller, input: &[Val], @@ -388,6 +423,8 @@ pub(crate) fn log_error( /// Write to logs (trace) /// Params: i64 (offset) /// Returns: none +/// **Note**: this function takes ownership of the handle passed in +/// the caller should not `free` this value pub(crate) fn log_trace( caller: Caller, input: &[Val],