Skip to content

Commit 3c0f289

Browse files
authored
fix(ext/node): segfault on calling StatementSync methods after connection has closed (#31331)
Previously, running this code causes segfault: ```ts import { DatabaseSync } from "node:sqlite"; const db = new DatabaseSync(':memory:'); const stmt = db.prepare('SELECT 1 AS value'); db.close(); stmt.get(); // segmentation fault (core dumped) ``` Changes in this PR also allows the https://github.com/nodejs/node/blob/v24.2.0/test/parallel/test-sqlite-statement-sync-columns.js test to pass.
1 parent 8c61416 commit 3c0f289

File tree

5 files changed

+147
-72
lines changed

5 files changed

+147
-72
lines changed

ext/node/ops/sqlite/database.rs

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ use super::Session;
3030
use super::SqliteError;
3131
use super::StatementSync;
3232
use super::session::SessionOptions;
33+
use super::statement::InnerStatementPtr;
3334
use super::statement::check_error_code2;
3435
use super::validators;
3536

@@ -230,7 +231,7 @@ impl<'a> ApplyChangesetOptions<'a> {
230231

231232
pub struct DatabaseSync {
232233
pub conn: Rc<RefCell<Option<rusqlite::Connection>>>,
233-
statements: Rc<RefCell<Vec<*mut libsqlite3_sys::sqlite3_stmt>>>,
234+
statements: Rc<RefCell<Vec<InnerStatementPtr>>>,
234235
options: DatabaseSyncOptions,
235236
location: String,
236237
}
@@ -491,12 +492,16 @@ impl DatabaseSync {
491492

492493
// Finalize all prepared statements
493494
for stmt in self.statements.borrow_mut().drain(..) {
494-
if !stmt.is_null() {
495-
// SAFETY: `stmt` is a valid statement handle.
496-
unsafe {
497-
libsqlite3_sys::sqlite3_finalize(stmt);
495+
match stmt.get() {
496+
None => continue,
497+
Some(ptr) => {
498+
// SAFETY: `ptr` is a valid statement handle.
499+
unsafe {
500+
libsqlite3_sys::sqlite3_finalize(ptr);
501+
};
502+
stmt.set(None);
498503
}
499-
}
504+
};
500505
}
501506

502507
let _ = self.conn.borrow_mut().take();
@@ -561,10 +566,11 @@ impl DatabaseSync {
561566
return Err(SqliteError::PrepareFailed);
562567
}
563568

564-
self.statements.borrow_mut().push(raw_stmt);
569+
let stmt_cell = Rc::new(Cell::new(Some(raw_stmt)));
570+
self.statements.borrow_mut().push(stmt_cell.clone());
565571

566572
Ok(StatementSync {
567-
inner: raw_stmt,
573+
inner: stmt_cell,
568574
db: Rc::downgrade(&self.conn),
569575
statements: Rc::clone(&self.statements),
570576
use_big_ints: Cell::new(false),

ext/node/ops/sqlite/mod.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,10 @@ pub enum SqliteError {
123123
#[error(transparent)]
124124
#[property("code" = self.code())]
125125
Validation(#[from] validators::Error),
126+
#[class(generic)]
127+
#[error("statement has been finalized")]
128+
#[property("code" = self.code())]
129+
StatementFinalized,
126130
}
127131

128132
#[derive(Debug, PartialEq, Eq)]
@@ -181,7 +185,8 @@ impl SqliteError {
181185
| Self::DuplicateNamedParameter(..)
182186
| Self::AlreadyClosed
183187
| Self::InUse
184-
| Self::AlreadyOpen => ErrorCode::ERR_INVALID_STATE,
188+
| Self::AlreadyOpen
189+
| Self::StatementFinalized => ErrorCode::ERR_INVALID_STATE,
185190
Self::NumberTooLarge(_, _) => ErrorCode::ERR_OUT_OF_RANGE,
186191
Self::LoadExensionFailed(_) => ErrorCode::ERR_LOAD_SQLITE_EXTENSION,
187192
_ => ErrorCode::ERR_SQLITE_ERROR,

0 commit comments

Comments
 (0)