Skip to content

Commit

Permalink
cleanup: host takes ownership of memory blocks it gets as arguments (#…
Browse files Browse the repository at this point in the history
…743)

This PR changes the host to take ownership of memory blocks passed into
Extism host functions, see:
extism/js-sdk#71 (comment)
  • Loading branch information
zshipko authored Sep 23, 2024
1 parent 8222164 commit 3ac3d9a
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 8 deletions.
6 changes: 6 additions & 0 deletions kernel/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
Empty file modified runtime/src/extism-runtime.wasm
100755 → 100644
Empty file.
53 changes: 45 additions & 8 deletions runtime/src/pdk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<CurrentPlugin>,
input: &[Val],
Expand All @@ -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) };
Expand All @@ -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<CurrentPlugin>,
input: &[Val],
Expand All @@ -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) };
Expand All @@ -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<CurrentPlugin>,
input: &[Val],
Expand All @@ -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)) }
Expand All @@ -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(());
}

Expand All @@ -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);

Expand All @@ -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<CurrentPlugin>,
input: &[Val],
Expand All @@ -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!(
Expand All @@ -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;

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);

Expand All @@ -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<CurrentPlugin>,
input: &[Val],
Expand All @@ -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<CurrentPlugin>,
input: &[Val],
Expand All @@ -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<CurrentPlugin>,
input: &[Val],
Expand All @@ -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<CurrentPlugin>,
input: &[Val],
Expand All @@ -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<CurrentPlugin>,
input: &[Val],
Expand Down

0 comments on commit 3ac3d9a

Please sign in to comment.