Skip to content

Commit d4e7c6c

Browse files
committed
Unoptimize row iteration
We're using the unsafe variant of `napi_set_named_property()` with raw CStrings. Let's unoptimize for now to see if that explains the SIGSEGVs people are reporting.
1 parent 99e5229 commit d4e7c6c

File tree

1 file changed

+20
-28
lines changed

1 file changed

+20
-28
lines changed

src/lib.rs

Lines changed: 20 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
mod auth;
2424

2525
use napi::{
26-
bindgen_prelude::{Array, FromNapiValue, ToNapiValue},
26+
bindgen_prelude::{Array, FromNapiValue},
2727
Env, JsUnknown, Result, ValueType,
2828
};
2929
use napi_derive::napi;
@@ -210,7 +210,7 @@ pub struct Options {
210210
}
211211

212212
/// Access mode.
213-
///
213+
///
214214
/// The `better-sqlite3` API allows the caller to configure the format of
215215
/// query results. This struct encapsulates the different access mode configs.
216216
struct AccessMode {
@@ -607,7 +607,7 @@ pub struct Statement {
607607
// The libSQL statement instance.
608608
stmt: Arc<tokio::sync::Mutex<libsql::Statement>>,
609609
// The column names.
610-
column_names: Vec<std::ffi::CString>,
610+
column_names: Vec<String>,
611611
// The access mode.
612612
mode: AccessMode,
613613
}
@@ -626,10 +626,10 @@ impl Statement {
626626
stmt: libsql::Statement,
627627
mode: AccessMode,
628628
) -> Self {
629-
let column_names: Vec<std::ffi::CString> = stmt
629+
let column_names: Vec<String> = stmt
630630
.columns()
631631
.iter()
632-
.map(|c| std::ffi::CString::new(c.name().to_string()).unwrap())
632+
.map(|c| c.name().to_string())
633633
.collect();
634634
let stmt = Arc::new(tokio::sync::Mutex::new(stmt));
635635
Self {
@@ -715,7 +715,7 @@ impl Statement {
715715
fn get_internal(
716716
env: &Env,
717717
row: &Option<libsql::Row>,
718-
column_names: &[std::ffi::CString],
718+
column_names: &[String],
719719
safe_ints: bool,
720720
raw: bool,
721721
pluck: bool,
@@ -876,9 +876,12 @@ impl Statement {
876876
}
877877
}
878878

879-
880879
#[napi]
881-
pub fn statement_iterate_sync(stmt: &Statement, _env: Env, params: Option<napi::JsUnknown>) -> Result<RowsIterator> {
880+
pub fn statement_iterate_sync(
881+
stmt: &Statement,
882+
_env: Env,
883+
params: Option<napi::JsUnknown>,
884+
) -> Result<RowsIterator> {
882885
let rt = runtime()?;
883886
let safe_ints = stmt.mode.safe_ints.load(Ordering::SeqCst);
884887
let raw = stmt.mode.raw.load(Ordering::SeqCst);
@@ -891,9 +894,7 @@ pub fn statement_iterate_sync(stmt: &Statement, _env: Env, params: Option<napi::
891894
let rows = stmt.query(params).await.map_err(Error::from)?;
892895
let mut column_names = Vec::new();
893896
for i in 0..rows.column_count() {
894-
column_names.push(
895-
std::ffi::CString::new(rows.column_name(i).unwrap().to_string()).unwrap(),
896-
);
897+
column_names.push(rows.column_name(i).unwrap().to_string());
897898
}
898899
Ok::<_, napi::Error>((rows, column_names))
899900
})?;
@@ -1045,7 +1046,7 @@ fn map_value(value: JsUnknown) -> Result<libsql::Value> {
10451046
#[napi]
10461047
pub struct RowsIterator {
10471048
rows: Arc<tokio::sync::Mutex<libsql::Rows>>,
1048-
column_names: Vec<std::ffi::CString>,
1049+
column_names: Vec<String>,
10491050
safe_ints: bool,
10501051
raw: bool,
10511052
pluck: bool,
@@ -1055,7 +1056,7 @@ pub struct RowsIterator {
10551056
impl RowsIterator {
10561057
pub fn new(
10571058
rows: Arc<tokio::sync::Mutex<libsql::Rows>>,
1058-
column_names: Vec<std::ffi::CString>,
1059+
column_names: Vec<String>,
10591060
safe_ints: bool,
10601061
raw: bool,
10611062
pluck: bool,
@@ -1093,7 +1094,7 @@ pub fn iterator_next_sync(iter: &RowsIterator) -> Result<Record> {
10931094
#[napi]
10941095
pub struct Record {
10951096
row: Option<libsql::Row>,
1096-
column_names: Vec<std::ffi::CString>,
1097+
column_names: Vec<String>,
10971098
safe_ints: bool,
10981099
raw: bool,
10991100
pluck: bool,
@@ -1132,7 +1133,7 @@ fn runtime() -> Result<&'static Runtime> {
11321133

11331134
fn map_row(
11341135
env: &Env,
1135-
column_names: &[std::ffi::CString],
1136+
column_names: &[String],
11361137
row: &libsql::Row,
11371138
safe_ints: bool,
11381139
raw: bool,
@@ -1168,7 +1169,7 @@ fn convert_value_to_js(
11681169

11691170
fn map_row_object(
11701171
env: &Env,
1171-
column_names: &[std::ffi::CString],
1172+
column_names: &[String],
11721173
row: &libsql::Row,
11731174
safe_ints: bool,
11741175
pluck: bool,
@@ -1186,8 +1187,7 @@ fn map_row_object(
11861187
env.get_null()?.into_unknown()
11871188
}
11881189
} else {
1189-
let result = env.create_object()?;
1190-
let result = unsafe { napi::JsObject::to_napi_value(env.raw(), result)? };
1190+
let mut result = env.create_object()?;
11911191
// If not plucking, get all columns
11921192
for idx in 0..column_count {
11931193
let value = match row.get_value(idx as i32) {
@@ -1197,24 +1197,16 @@ fn map_row_object(
11971197

11981198
let column_name = &column_names[idx];
11991199
let js_value = convert_value_to_js(env, &value, safe_ints)?;
1200-
unsafe {
1201-
napi::sys::napi_set_named_property(
1202-
env.raw(),
1203-
result,
1204-
column_name.as_ptr(),
1205-
napi::JsUnknown::to_napi_value(env.raw(), js_value)?,
1206-
);
1207-
}
1200+
result.set_named_property(column_name, js_value)?;
12081201
}
1209-
let result: napi::JsObject = unsafe { napi::JsObject::from_napi_value(env.raw(), result)? };
12101202
result.into_unknown()
12111203
};
12121204
Ok(result)
12131205
}
12141206

12151207
fn map_row_raw(
12161208
env: &Env,
1217-
column_names: &[std::ffi::CString],
1209+
column_names: &[String],
12181210
row: &libsql::Row,
12191211
safe_ints: bool,
12201212
pluck: bool,

0 commit comments

Comments
 (0)