From 5c5fe0b81c5d0c2e7f0716e26e0dfa068ca4f388 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guilherme=20Ara=C3=BAjo?= Date: Sun, 9 Aug 2026 14:23:13 -0300 Subject: [PATCH 1/2] sqlite: manage sqlite3_stmt lifetime with RAII MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Guilherme Araújo --- src/node_sqlite.cc | 109 ++++++++++++---------- src/node_sqlite.h | 12 ++- test/parallel/test-sqlite-template-tag.js | 17 ++++ 3 files changed, 84 insertions(+), 54 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 68554ed33b31..4695188787ef 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1590,6 +1590,7 @@ void DatabaseSync::Prepare(const FunctionCallbackInfo& args) { Utf8Value sql(env->isolate(), args[0].As()); sqlite3_stmt* s = nullptr; int r = sqlite3_prepare_v2(db->connection_, *sql, -1, &s, nullptr); + StatementPtr stmt_ptr(s); CHECK_ERROR_OR_THROW(env->isolate(), db, r, SQLITE_OK, void()); @@ -1602,8 +1603,11 @@ void DatabaseSync::Prepare(const FunctionCallbackInfo& args) { return; } - BaseObjectPtr stmt = - StatementSync::Create(env, BaseObjectPtr(db), s); + BaseObjectPtr stmt = StatementSync::Create( + env, BaseObjectPtr(db), std::move(stmt_ptr)); + if (!stmt) { + return; + } db->statements_.insert(stmt.get()); if (return_arrays.has_value()) { @@ -2660,10 +2664,9 @@ int DatabaseSync::AuthorizerCallback(void* user_data, StatementSync::StatementSync(Environment* env, Local object, BaseObjectPtr db, - sqlite3_stmt* stmt) - : BaseObject(env, object), db_(std::move(db)) { + StatementPtr stmt) + : BaseObject(env, object), db_(std::move(db)), statement_(std::move(stmt)) { MakeWeak(); - statement_ = stmt; use_big_ints_ = db_->use_big_ints(); return_arrays_ = db_->return_arrays(); allow_bare_named_params_ = db_->allow_bare_named_params(); @@ -2677,15 +2680,15 @@ StatementSync::~StatementSync() { } void StatementSync::Close() { + db_->UntrackStatement(this); + if (!IsFinalized()) { - db_->UntrackStatement(this); Finalize(); } } void StatementSync::Finalize() { - sqlite3_finalize(statement_); - statement_ = nullptr; + statement_.reset(); InvalidateColumnNameCache(); } @@ -2714,11 +2717,11 @@ void StatementSync::Dispose(const FunctionCallbackInfo& args) { inline int StatementSync::ResetStatement() { reset_generation_++; - return sqlite3_reset(statement_); + return sqlite3_reset(statement_.get()); } bool StatementSync::BindParams(const FunctionCallbackInfo& args) { - int r = sqlite3_clear_bindings(statement_); + int r = sqlite3_clear_bindings(statement_.get()); CHECK_ERROR_OR_THROW(env()->isolate(), db_.get(), r, SQLITE_OK, false); int anon_idx = 1; @@ -2735,10 +2738,10 @@ bool StatementSync::BindParams(const FunctionCallbackInfo& args) { if (allow_bare_named_params_ && !bare_named_params_.has_value()) { bare_named_params_.emplace(); - int param_count = sqlite3_bind_parameter_count(statement_); + int param_count = sqlite3_bind_parameter_count(statement_.get()); // Parameter indexing starts at one. for (int i = 1; i <= param_count; ++i) { - const char* name = sqlite3_bind_parameter_name(statement_, i); + const char* name = sqlite3_bind_parameter_name(statement_.get(), i); if (name == nullptr) { continue; } @@ -2770,12 +2773,12 @@ bool StatementSync::BindParams(const FunctionCallbackInfo& args) { } Utf8Value utf8_key(env()->isolate(), key); - int r = sqlite3_bind_parameter_index(statement_, *utf8_key); + int r = sqlite3_bind_parameter_index(statement_.get(), *utf8_key); if (r == 0) { if (allow_bare_named_params_) { auto lookup = bare_named_params_->find(std::string(*utf8_key)); if (lookup != bare_named_params_->end()) { - r = sqlite3_bind_parameter_index(statement_, + r = sqlite3_bind_parameter_index(statement_.get(), lookup->second.c_str()); } } @@ -2805,7 +2808,8 @@ bool StatementSync::BindParams(const FunctionCallbackInfo& args) { for (int i = anon_start; i < args.Length(); ++i) { while (1) { - const char* param = sqlite3_bind_parameter_name(statement_, anon_idx); + const char* param = + sqlite3_bind_parameter_name(statement_.get(), anon_idx); if (param == nullptr || param[0] == '?') break; anon_idx++; } @@ -2831,7 +2835,7 @@ bool StatementSync::BindValue(const Local& value, const int index) { int r; if (value->IsNumber()) { const double val = value.As()->Value(); - r = sqlite3_bind_double(statement_, index, val); + r = sqlite3_bind_double(statement_.get(), index, val); } else if (value->IsString()) { Utf8Value val(isolate, value.As()); if (val.IsAllocated()) { @@ -2841,9 +2845,9 @@ bool StatementSync::BindValue(const Local& value, const int index) { const sqlite3_uint64 length = static_cast(val.length()); val.Release(); r = sqlite3_bind_text64( - statement_, index, data, length, std::free, SQLITE_UTF8); + statement_.get(), index, data, length, std::free, SQLITE_UTF8); } else { - r = sqlite3_bind_text64(statement_, + r = sqlite3_bind_text64(statement_.get(), index, *val, static_cast(val.length()), @@ -2851,17 +2855,17 @@ bool StatementSync::BindValue(const Local& value, const int index) { SQLITE_UTF8); } } else if (value->IsNull()) { - r = sqlite3_bind_null(statement_, index); + r = sqlite3_bind_null(statement_.get(), index); } else if (value->IsArrayBufferView() || value->IsArrayBuffer() || value->IsSharedArrayBuffer()) { ArrayBufferViewContents buf(value); - r = sqlite3_bind_blob64(statement_, + r = sqlite3_bind_blob64(statement_.get(), index, buf.data(), static_cast(buf.length()), SQLITE_TRANSIENT); } else if (value->IsBoolean()) { - r = sqlite3_bind_int(statement_, index, value->IsTrue() ? 1 : 0); + r = sqlite3_bind_int(statement_.get(), index, value->IsTrue() ? 1 : 0); } else if (value->IsBigInt()) { bool lossless; int64_t as_int = value.As()->Int64Value(&lossless); @@ -2869,7 +2873,7 @@ bool StatementSync::BindValue(const Local& value, const int index) { THROW_ERR_INVALID_ARG_VALUE(env(), "BigInt value is too large to bind."); return false; } - r = sqlite3_bind_int64(statement_, index, as_int); + r = sqlite3_bind_int64(statement_.get(), index, as_int); } else { THROW_ERR_INVALID_ARG_TYPE( isolate, @@ -2884,11 +2888,11 @@ bool StatementSync::BindValue(const Local& value, const int index) { MaybeLocal StatementSync::ColumnToValue(const int column) { return StatementExecutionHelper::ColumnToValue( - env(), statement_, column, use_big_ints_); + env(), statement_.get(), column, use_big_ints_); } MaybeLocal StatementSync::ColumnNameToName(const int column) { - const char* col_name = sqlite3_column_name(statement_, column); + const char* col_name = sqlite3_column_name(statement_.get(), column); if (col_name == nullptr) { THROW_ERR_INVALID_STATE(env(), "Cannot get name of column %d", column); return MaybeLocal(); @@ -2905,10 +2909,10 @@ bool StatementSync::GetCachedColumnNames(LocalVector* keys) { Isolate* isolate = env()->isolate(); const int reprepare_count = - sqlite3_stmt_status(statement_, SQLITE_STMTSTATUS_REPREPARE, false); + sqlite3_stmt_status(statement_.get(), SQLITE_STMTSTATUS_REPREPARE, false); if (reprepare_count != cached_column_names_reprepare_count_) { cached_column_names_.clear(); - const int num_cols = sqlite3_column_count(statement_); + const int num_cols = sqlite3_column_count(statement_.get()); if (num_cols == 0) { cached_column_names_reprepare_count_ = reprepare_count; return true; @@ -3189,7 +3193,7 @@ void StatementSync::All(const FunctionCallbackInfo& args) { Local result; if (StatementExecutionHelper::All(env, stmt->db_.get(), - stmt->statement_, + stmt->statement_.get(), stmt->return_arrays_, stmt->use_big_ints_) .ToLocal(&result)) { @@ -3238,7 +3242,7 @@ void StatementSync::Get(const FunctionCallbackInfo& args) { Local result; if (StatementExecutionHelper::Get(env, stmt->db_.get(), - stmt->statement_, + stmt->statement_.get(), stmt->return_arrays_, stmt->use_big_ints_) .ToLocal(&result)) { @@ -3261,7 +3265,7 @@ void StatementSync::Run(const FunctionCallbackInfo& args) { Local result; if (StatementExecutionHelper::Run( - env, stmt->db_.get(), stmt->statement_, stmt->use_big_ints_) + env, stmt->db_.get(), stmt->statement_.get(), stmt->use_big_ints_) .ToLocal(&result)) { args.GetReturnValue().Set(result); } @@ -3273,7 +3277,7 @@ void StatementSync::Columns(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE( env, stmt->IsFinalized(), "statement has been finalized"); - int num_cols = sqlite3_column_count(stmt->statement_); + int num_cols = sqlite3_column_count(stmt->statement_.get()); Isolate* isolate = env->isolate(); LocalVector cols(isolate); auto sqlite_column_template = env->sqlite_column_template(); @@ -3288,14 +3292,14 @@ void StatementSync::Columns(const FunctionCallbackInfo& args) { for (int i = 0; i < num_cols; ++i) { MaybeLocal values[] = { NullableSQLiteStringToValue( - isolate, sqlite3_column_origin_name(stmt->statement_, i)), + isolate, sqlite3_column_origin_name(stmt->statement_.get(), i)), NullableSQLiteStringToValue( - isolate, sqlite3_column_database_name(stmt->statement_, i)), + isolate, sqlite3_column_database_name(stmt->statement_.get(), i)), stmt->ColumnNameToName(i), NullableSQLiteStringToValue( - isolate, sqlite3_column_table_name(stmt->statement_, i)), + isolate, sqlite3_column_table_name(stmt->statement_.get(), i)), NullableSQLiteStringToValue( - isolate, sqlite3_column_decltype(stmt->statement_, i)), + isolate, sqlite3_column_decltype(stmt->statement_.get(), i)), }; Local col; @@ -3317,7 +3321,7 @@ void StatementSync::SourceSQLGetter(const FunctionCallbackInfo& args) { THROW_AND_RETURN_ON_BAD_STATE( env, stmt->IsFinalized(), "statement has been finalized"); Local sql; - if (!String::NewFromUtf8(env->isolate(), sqlite3_sql(stmt->statement_)) + if (!String::NewFromUtf8(env->isolate(), sqlite3_sql(stmt->statement_.get())) .ToLocal(&sql)) { return; } @@ -3332,7 +3336,7 @@ void StatementSync::ExpandedSQLGetter(const FunctionCallbackInfo& args) { env, stmt->IsFinalized(), "statement has been finalized"); // sqlite3_expanded_sql may return nullptr without producing an error code. - char* expanded = sqlite3_expanded_sql(stmt->statement_); + char* expanded = sqlite3_expanded_sql(stmt->statement_.get()); if (expanded == nullptr) { return THROW_ERR_SQLITE_ERROR( env->isolate(), "Expanded SQL text would exceed configured limits"); @@ -3506,11 +3510,11 @@ bool SQLTagStore::ResetAndBindStatement( int r = stmt->ResetStatement(); CHECK_ERROR_OR_THROW(isolate, stmt->db_.get(), r, SQLITE_OK, false); - r = sqlite3_clear_bindings(stmt->statement_); + r = sqlite3_clear_bindings(stmt->statement_.get()); CHECK_ERROR_OR_THROW(isolate, stmt->db_.get(), r, SQLITE_OK, false); uint32_t n_params = args.Length() - 1; - int param_count = sqlite3_bind_parameter_count(stmt->statement_); + int param_count = sqlite3_bind_parameter_count(stmt->statement_.get()); if (param_count != static_cast(n_params)) { THROW_ERR_INVALID_ARG_VALUE( env, @@ -3547,7 +3551,7 @@ void SQLTagStore::Run(const FunctionCallbackInfo& args) { Local result; if (StatementExecutionHelper::Run( - env, stmt->db_.get(), stmt->statement_, stmt->use_big_ints_) + env, stmt->db_.get(), stmt->statement_.get(), stmt->use_big_ints_) .ToLocal(&result)) { args.GetReturnValue().Set(result); } @@ -3602,7 +3606,7 @@ void SQLTagStore::Get(const FunctionCallbackInfo& args) { Local result; if (StatementExecutionHelper::Get(env, stmt->db_.get(), - stmt->statement_, + stmt->statement_.get(), stmt->return_arrays_, stmt->use_big_ints_) .ToLocal(&result)) { @@ -3636,7 +3640,7 @@ void SQLTagStore::All(const FunctionCallbackInfo& args) { Local result; if (StatementExecutionHelper::All(env, stmt->db_.get(), - stmt->statement_, + stmt->statement_.get(), stmt->return_arrays_, stmt->use_big_ints_) .ToLocal(&result)) { @@ -3704,10 +3708,10 @@ BaseObjectPtr SQLTagStore::PrepareStatement( sqlite3_stmt* s = nullptr; int r = sqlite3_prepare_v2( session->database_->connection_, sql.data(), sql.size(), &s, nullptr); + StatementPtr stmt_ptr(s); if (r != SQLITE_OK) { THROW_ERR_SQLITE_ERROR(isolate, session->database_.get()); - sqlite3_finalize(s); return BaseObjectPtr(); } @@ -3718,15 +3722,17 @@ BaseObjectPtr SQLTagStore::PrepareStatement( return BaseObjectPtr(); } - BaseObjectPtr stmt_obj = StatementSync::Create( - env, BaseObjectPtr(session->database_), s); + BaseObjectPtr stmt_obj = + StatementSync::Create(env, + BaseObjectPtr(session->database_), + std::move(stmt_ptr)); if (!stmt_obj) { THROW_ERR_SQLITE_ERROR(isolate, "Failed to create StatementSync"); - sqlite3_finalize(s); return BaseObjectPtr(); } + session->database_->statements_.insert(stmt_obj.get()); session->sql_tags_.Put(sql, stmt_obj); stmt = stmt_obj; } @@ -3789,7 +3795,7 @@ Local StatementSync::GetConstructorTemplate( } BaseObjectPtr StatementSync::Create( - Environment* env, BaseObjectPtr db, sqlite3_stmt* stmt) { + Environment* env, BaseObjectPtr db, StatementPtr stmt) { Local obj; if (!GetConstructorTemplate(env) ->InstanceTemplate() @@ -3798,7 +3804,8 @@ BaseObjectPtr StatementSync::Create( return nullptr; } - return MakeBaseObject(env, obj, std::move(db), stmt); + return MakeBaseObject( + env, obj, std::move(db), std::move(stmt)); } StatementSyncIterator::StatementSyncIterator(Environment* env, @@ -3871,7 +3878,7 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo& args) { iter->statement_reset_generation_ != iter->stmt_->reset_generation_, "iterator was invalidated"); - int r = sqlite3_step(iter->stmt_->statement_); + int r = sqlite3_step(iter->stmt_->statement_.get()); if (r != SQLITE_ROW) { CHECK_ERROR_OR_THROW( env->isolate(), iter->stmt_->db_.get(), r, SQLITE_DONE, void()); @@ -3889,13 +3896,13 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo& args) { return; } - int num_cols = sqlite3_column_count(iter->stmt_->statement_); + int num_cols = sqlite3_column_count(iter->stmt_->statement_.get()); Local row_value; LocalVector row_keys(isolate); LocalVector row_values(isolate); if (ExtractRowValues(env, - iter->stmt_->statement_, + iter->stmt_->statement_.get(), num_cols, iter->stmt_->use_big_ints_, &row_values) @@ -3935,7 +3942,7 @@ void StatementSyncIterator::Return(const FunctionCallbackInfo& args) { // is invoked by the language during abrupt completion (e.g. a `throw` // inside a `for...of` body), and throwing on a deferred SQLite error // would discard the caller's already-pending exception. - sqlite3_reset(iter->stmt_->statement_); + sqlite3_reset(iter->stmt_->statement_.get()); iter->done_ = true; auto iter_template = getLazyIterTemplate(env); diff --git a/src/node_sqlite.h b/src/node_sqlite.h index b4446e5db859..17025a528622 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -135,6 +135,12 @@ class StatementSync; class BackupJob; class Session; +inline void FinalizeStatement(sqlite3_stmt* stmt) { + sqlite3_finalize(stmt); +} + +using StatementPtr = DeleteFnPtr; + class StatementExecutionHelper { public: static v8::MaybeLocal All(Environment* env, @@ -263,13 +269,13 @@ class StatementSync : public BaseObject { StatementSync(Environment* env, v8::Local object, BaseObjectPtr db, - sqlite3_stmt* stmt); + StatementPtr stmt); void MemoryInfo(MemoryTracker* tracker) const override; static v8::Local GetConstructorTemplate( Environment* env); static BaseObjectPtr Create(Environment* env, BaseObjectPtr db, - sqlite3_stmt* stmt); + StatementPtr stmt); static void All(const v8::FunctionCallbackInfo& args); static void Iterate(const v8::FunctionCallbackInfo& args); static void Get(const v8::FunctionCallbackInfo& args); @@ -299,7 +305,7 @@ class StatementSync : public BaseObject { ~StatementSync() override; void Close(); BaseObjectPtr db_; - sqlite3_stmt* statement_; + StatementPtr statement_; bool return_arrays_ = false; bool use_big_ints_; bool allow_bare_named_params_; diff --git a/test/parallel/test-sqlite-template-tag.js b/test/parallel/test-sqlite-template-tag.js index 20376e199d1b..eaa6d19fc7cd 100644 --- a/test/parallel/test-sqlite-template-tag.js +++ b/test/parallel/test-sqlite-template-tag.js @@ -371,3 +371,20 @@ test('tag store prevents circular reference leaks', async () => { return after < before * 1.5; }, 20); }); + +test('cached statements are finalized when the database is closed', () => { + const db = new DatabaseSync(':memory:'); + const sql = db.createTagStore(); + + db.exec('CREATE TABLE foo (id INTEGER PRIMARY KEY)'); + db.exec('INSERT INTO foo (id) VALUES (1)'); + assert.deepStrictEqual(sql.all`SELECT id FROM foo`, [{ __proto__: null, id: 1 }]); + + db.close(); + db.open(); + + assert.throws(() => sql.all`SELECT id FROM foo`, { + code: 'ERR_SQLITE_ERROR', + message: /no such table/i, + }); +}); From 31e31a118a8ca673d468ab96204b9d8e24aeda82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guilherme=20Ara=C3=BAjo?= Date: Tue, 11 Aug 2026 14:15:38 -0300 Subject: [PATCH 2/2] sqlite: fix stmt pointers --- src/node_sqlite.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 4695188787ef..9c80d18cdaab 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -3188,7 +3188,7 @@ void StatementSync::All(const FunctionCallbackInfo& args) { bool needs_reset = true; auto reset = OnScopeLeave([&]() { - if (needs_reset) sqlite3_reset(stmt->statement_); + if (needs_reset) sqlite3_reset(stmt->statement_.get()); }); Local result; if (StatementExecutionHelper::All(env, @@ -3198,7 +3198,7 @@ void StatementSync::All(const FunctionCallbackInfo& args) { stmt->use_big_ints_) .ToLocal(&result)) { RESET_AND_CHECK( - isolate, stmt->db_.get(), stmt->statement_, needs_reset, void()); + isolate, stmt->db_.get(), stmt->statement_.get(), needs_reset, void()); args.GetReturnValue().Set(result); } } @@ -3635,7 +3635,7 @@ void SQLTagStore::All(const FunctionCallbackInfo& args) { Isolate* isolate = env->isolate(); bool needs_reset = true; auto reset = OnScopeLeave([&]() { - if (needs_reset) sqlite3_reset(stmt->statement_); + if (needs_reset) sqlite3_reset(stmt->statement_.get()); }); Local result; if (StatementExecutionHelper::All(env, @@ -3645,7 +3645,7 @@ void SQLTagStore::All(const FunctionCallbackInfo& args) { stmt->use_big_ints_) .ToLocal(&result)) { RESET_AND_CHECK( - isolate, stmt->db_.get(), stmt->statement_, needs_reset, void()); + isolate, stmt->db_.get(), stmt->statement_.get(), needs_reset, void()); args.GetReturnValue().Set(result); } } @@ -3885,7 +3885,7 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo& args) { iter->done_ = true; RESET_OR_THROW(env->isolate(), iter->stmt_->db_.get(), - iter->stmt_->statement_, + iter->stmt_->statement_.get(), void()); MaybeLocal values[] = {Boolean::New(isolate, true), Null(isolate)}; Local result;