diff --git a/backend/windmill-duckdb-ffi-internal/src/lib.rs b/backend/windmill-duckdb-ffi-internal/src/lib.rs index e2c918c3fe..cb5c43a018 100644 --- a/backend/windmill-duckdb-ffi-internal/src/lib.rs +++ b/backend/windmill-duckdb-ffi-internal/src/lib.rs @@ -16,6 +16,17 @@ pub struct Arg { pub json_value: serde_json::Value, } +// Freeing from the caller side crashes the runtime with jemalloc enabled (EXIT CODE 11 SEGFAULT) +#[unsafe(no_mangle)] +pub extern "C" fn free_cstr(string: *mut c_char) -> () { + if string.is_null() { + return; + } + unsafe { + let _ = CString::from_raw(string); + } +} + #[unsafe(no_mangle)] pub extern "C" fn run_duckdb_ffi( query_block_list: *const *const c_char, @@ -289,11 +300,11 @@ fn row_to_value(row: &Row<'_>, column_names: &[String]) -> Result, duckdb::types::Value::UBigInt(u) => serde_json::Value::Number(u.into()), duckdb::types::Value::Float(f) => serde_json::Value::Number( serde_json::Number::from_f64(f as f64) - .ok_or_else(|| ("Could not convert to f64".to_string()))?, + .ok_or_else(|| "Could not convert to f64".to_string())?, ), duckdb::types::Value::Double(f) => serde_json::Value::Number( serde_json::Number::from_f64(f) - .ok_or_else(|| ("Could not convert to f64".to_string()))?, + .ok_or_else(|| "Could not convert to f64".to_string())?, ), duckdb::types::Value::Decimal(d) => serde_json::Value::String(d.to_string()), duckdb::types::Value::Timestamp(_, ts) => serde_json::Value::String(ts.to_string()), @@ -401,7 +412,7 @@ fn json_value_to_duckdb_value( "double" | "float8" => duckdb::types::Value::Double(v), "decimal" | "numeric" => duckdb::types::Value::Decimal( Decimal::from_f64(v) - .ok_or_else(|| ("Could not convert f64 to Decimal".to_string()))?, + .ok_or_else(|| "Could not convert f64 to Decimal".to_string())?, ), _ => duckdb::types::Value::Double(v), // default fallback } @@ -436,7 +447,7 @@ fn string_to_duckdb_timestamp(s: &str) -> Result { fn string_to_duckdb_date(s: &str) -> Result { use chrono::Datelike; let date = chrono::NaiveDate::parse_from_str(s, "%Y-%m-%d") - .map_err(|e| (format!("Invalid date format: {}", e)))?; + .map_err(|e| format!("Invalid date format: {}", e))?; Ok(duckdb::types::Value::Date32(date.num_days_from_ce())) } diff --git a/backend/windmill-worker/src/duckdb_executor.rs b/backend/windmill-worker/src/duckdb_executor.rs index aea5f7be15..ceb0ccb96e 100644 --- a/backend/windmill-worker/src/duckdb_executor.rs +++ b/backend/windmill-worker/src/duckdb_executor.rs @@ -1,6 +1,6 @@ use std::cell::RefCell; use std::env; -use std::ffi::{c_char, CString}; +use std::ffi::{c_char, CStr, CString}; use std::ptr::NonNull; use std::sync::{Arc, Mutex}; @@ -193,6 +193,7 @@ struct DuckDbFfiLib { column_order_ptr: *mut *mut c_char, ) -> *mut c_char, >, + free_cstr: Symbol<'static, unsafe extern "C" fn(string: *mut c_char) -> ()>, } impl DuckDbFfiLib { @@ -232,6 +233,7 @@ impl DuckDbFfiLib { let lib = Box::leak(Box::new(lib)); Ok(DuckDbFfiLib { run_duckdb_ffi: unsafe { lib.get(b"run_duckdb_ffi").map_err(to_anyhow)? }, + free_cstr: unsafe { lib.get(b"free_cstr").map_err(to_anyhow)? }, }) } } @@ -264,8 +266,9 @@ fn run_duckdb_ffi_safe<'a>( let w_id = CString::new(w_id).map_err(to_anyhow)?; let run_duckdb_ffi = &DuckDbFfiLib::get_singleton()?.run_duckdb_ffi; + let free_cstr = &DuckDbFfiLib::get_singleton()?.free_cstr; let mut column_order: *mut c_char = std::ptr::null_mut(); - let result_cstr = unsafe { + let result_str = unsafe { let ptr = run_duckdb_ffi( query_block_list.as_ptr(), query_block_list_count, @@ -275,27 +278,19 @@ fn run_duckdb_ffi_safe<'a>( w_id.as_ptr(), &mut column_order, ); - CString::from_raw(ptr) // Using from_raw to take ownership and ensure it gets freed + let str = CStr::from_ptr(ptr).to_string_lossy().to_string(); + free_cstr(ptr); + str }; let column_order = if column_order.is_null() { None } else { - Some(unsafe { - serde_json::from_str::>(&CString::from_raw(column_order).to_string_lossy())? - }) + let str = unsafe { CStr::from_ptr(column_order).to_string_lossy().to_string() }; + unsafe { free_cstr(column_order) }; + Some(serde_json::from_str::>(&str)?) }; - let result_str = result_cstr - .to_str() - .map_err(|e| { - Error::ExecutionErr(format!( - "Failed to convert result C string to Rust string: {}", - e.to_string() - )) - })? - .to_string(); - if result_str.starts_with("ERROR") { Err(Error::ExecutionErr(result_str[6..].to_string())) } else {