Skip to content

Commit 4828926

Browse files
authored
Fix SEGFAULT with duckdb and jemalloc (#6680)
1 parent 8f4fef9 commit 4828926

File tree

2 files changed

+26
-20
lines changed

2 files changed

+26
-20
lines changed

backend/windmill-duckdb-ffi-internal/src/lib.rs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,17 @@ pub struct Arg {
1616
pub json_value: serde_json::Value,
1717
}
1818

19+
// Freeing from the caller side crashes the runtime with jemalloc enabled (EXIT CODE 11 SEGFAULT)
20+
#[unsafe(no_mangle)]
21+
pub extern "C" fn free_cstr(string: *mut c_char) -> () {
22+
if string.is_null() {
23+
return;
24+
}
25+
unsafe {
26+
let _ = CString::from_raw(string);
27+
}
28+
}
29+
1930
#[unsafe(no_mangle)]
2031
pub extern "C" fn run_duckdb_ffi(
2132
query_block_list: *const *const c_char,
@@ -289,11 +300,11 @@ fn row_to_value(row: &Row<'_>, column_names: &[String]) -> Result<Box<RawValue>,
289300
duckdb::types::Value::UBigInt(u) => serde_json::Value::Number(u.into()),
290301
duckdb::types::Value::Float(f) => serde_json::Value::Number(
291302
serde_json::Number::from_f64(f as f64)
292-
.ok_or_else(|| ("Could not convert to f64".to_string()))?,
303+
.ok_or_else(|| "Could not convert to f64".to_string())?,
293304
),
294305
duckdb::types::Value::Double(f) => serde_json::Value::Number(
295306
serde_json::Number::from_f64(f)
296-
.ok_or_else(|| ("Could not convert to f64".to_string()))?,
307+
.ok_or_else(|| "Could not convert to f64".to_string())?,
297308
),
298309
duckdb::types::Value::Decimal(d) => serde_json::Value::String(d.to_string()),
299310
duckdb::types::Value::Timestamp(_, ts) => serde_json::Value::String(ts.to_string()),
@@ -401,7 +412,7 @@ fn json_value_to_duckdb_value(
401412
"double" | "float8" => duckdb::types::Value::Double(v),
402413
"decimal" | "numeric" => duckdb::types::Value::Decimal(
403414
Decimal::from_f64(v)
404-
.ok_or_else(|| ("Could not convert f64 to Decimal".to_string()))?,
415+
.ok_or_else(|| "Could not convert f64 to Decimal".to_string())?,
405416
),
406417
_ => duckdb::types::Value::Double(v), // default fallback
407418
}
@@ -436,7 +447,7 @@ fn string_to_duckdb_timestamp(s: &str) -> Result<duckdb::types::Value, String> {
436447
fn string_to_duckdb_date(s: &str) -> Result<duckdb::types::Value, String> {
437448
use chrono::Datelike;
438449
let date = chrono::NaiveDate::parse_from_str(s, "%Y-%m-%d")
439-
.map_err(|e| (format!("Invalid date format: {}", e)))?;
450+
.map_err(|e| format!("Invalid date format: {}", e))?;
440451
Ok(duckdb::types::Value::Date32(date.num_days_from_ce()))
441452
}
442453

backend/windmill-worker/src/duckdb_executor.rs

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::cell::RefCell;
22
use std::env;
3-
use std::ffi::{c_char, CString};
3+
use std::ffi::{c_char, CStr, CString};
44
use std::ptr::NonNull;
55
use std::sync::{Arc, Mutex};
66

@@ -193,6 +193,7 @@ struct DuckDbFfiLib {
193193
column_order_ptr: *mut *mut c_char,
194194
) -> *mut c_char,
195195
>,
196+
free_cstr: Symbol<'static, unsafe extern "C" fn(string: *mut c_char) -> ()>,
196197
}
197198

198199
impl DuckDbFfiLib {
@@ -232,6 +233,7 @@ impl DuckDbFfiLib {
232233
let lib = Box::leak(Box::new(lib));
233234
Ok(DuckDbFfiLib {
234235
run_duckdb_ffi: unsafe { lib.get(b"run_duckdb_ffi").map_err(to_anyhow)? },
236+
free_cstr: unsafe { lib.get(b"free_cstr").map_err(to_anyhow)? },
235237
})
236238
}
237239
}
@@ -264,8 +266,9 @@ fn run_duckdb_ffi_safe<'a>(
264266
let w_id = CString::new(w_id).map_err(to_anyhow)?;
265267

266268
let run_duckdb_ffi = &DuckDbFfiLib::get_singleton()?.run_duckdb_ffi;
269+
let free_cstr = &DuckDbFfiLib::get_singleton()?.free_cstr;
267270
let mut column_order: *mut c_char = std::ptr::null_mut();
268-
let result_cstr = unsafe {
271+
let result_str = unsafe {
269272
let ptr = run_duckdb_ffi(
270273
query_block_list.as_ptr(),
271274
query_block_list_count,
@@ -275,27 +278,19 @@ fn run_duckdb_ffi_safe<'a>(
275278
w_id.as_ptr(),
276279
&mut column_order,
277280
);
278-
CString::from_raw(ptr) // Using from_raw to take ownership and ensure it gets freed
281+
let str = CStr::from_ptr(ptr).to_string_lossy().to_string();
282+
free_cstr(ptr);
283+
str
279284
};
280285

281286
let column_order = if column_order.is_null() {
282287
None
283288
} else {
284-
Some(unsafe {
285-
serde_json::from_str::<Vec<String>>(&CString::from_raw(column_order).to_string_lossy())?
286-
})
289+
let str = unsafe { CStr::from_ptr(column_order).to_string_lossy().to_string() };
290+
unsafe { free_cstr(column_order) };
291+
Some(serde_json::from_str::<Vec<String>>(&str)?)
287292
};
288293

289-
let result_str = result_cstr
290-
.to_str()
291-
.map_err(|e| {
292-
Error::ExecutionErr(format!(
293-
"Failed to convert result C string to Rust string: {}",
294-
e.to_string()
295-
))
296-
})?
297-
.to_string();
298-
299294
if result_str.starts_with("ERROR") {
300295
Err(Error::ExecutionErr(result_str[6..].to_string()))
301296
} else {

0 commit comments

Comments
 (0)