From 57ed6daef3f042a2bf1d949a570c5c73a3e84145 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guilherme=20Ara=C3=BAjo?= Date: Mon, 23 Mar 2026 21:00:51 -0300 Subject: [PATCH 1/2] sqlite: fix sqlite3_stmt leak in prepare create failure 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 | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index c3b7d279f4dca0..2fccefd90413cb 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1554,6 +1554,10 @@ void DatabaseSync::Prepare(const FunctionCallbackInfo& args) { CHECK_ERROR_OR_THROW(env->isolate(), db, r, SQLITE_OK, void()); BaseObjectPtr stmt = StatementSync::Create(env, BaseObjectPtr(db), s); + if (!stmt) { + sqlite3_finalize(s); + return; + } db->statements_.insert(stmt.get()); if (return_arrays.has_value()) { From 78e32bc92de890e1a784b64b94cd3315c6d15fe8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guilherme=20Ara=C3=BAjo?= Date: Thu, 16 Jul 2026 10:47:35 -0300 Subject: [PATCH 2/2] refactor: implement sqlite3_stmt raii --- src/node_sqlite.cc | 119 +++++++++++++++++++++++---------------------- src/node_sqlite.h | 12 +++-- 2 files changed, 69 insertions(+), 62 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 2fccefd90413cb..3f4a5cdd9ff31d 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1550,12 +1550,12 @@ 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()); - BaseObjectPtr stmt = - StatementSync::Create(env, BaseObjectPtr(db), s); + BaseObjectPtr stmt = StatementSync::Create( + env, BaseObjectPtr(db), std::move(stmt_ptr)); if (!stmt) { - sqlite3_finalize(s); return; } db->statements_.insert(stmt.get()); @@ -2569,10 +2569,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(); @@ -2589,8 +2588,7 @@ StatementSync::~StatementSync() { } void StatementSync::Finalize() { - sqlite3_finalize(statement_); - statement_ = nullptr; + statement_.reset(); InvalidateColumnNameCache(); } @@ -2604,11 +2602,11 @@ inline bool StatementSync::IsFinalized() { 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; @@ -2624,10 +2622,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; } @@ -2659,12 +2657,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()); } } @@ -2694,7 +2692,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++; } @@ -2719,7 +2718,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()) { @@ -2729,9 +2728,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()), @@ -2739,10 +2738,10 @@ 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()) { ArrayBufferViewContents buf(value); - r = sqlite3_bind_blob64(statement_, + r = sqlite3_bind_blob64(statement_.get(), index, buf.data(), static_cast(buf.length()), @@ -2754,7 +2753,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, @@ -2769,11 +2768,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(); @@ -2790,10 +2789,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; @@ -3046,12 +3045,12 @@ void StatementSync::All(const FunctionCallbackInfo& args) { return; } - auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt->statement_); }); + auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt->statement_.get()); }); Local result; if (StatementExecutionHelper::All(env, stmt->db_.get(), - stmt->statement_, + stmt->statement_.get(), stmt->return_arrays_, stmt->use_big_ints_) .ToLocal(&result)) { @@ -3098,7 +3097,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)) { @@ -3121,7 +3120,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); } @@ -3133,7 +3132,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(); @@ -3148,14 +3147,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; @@ -3177,7 +3176,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; } @@ -3192,7 +3191,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"); @@ -3373,9 +3372,9 @@ void SQLTagStore::Run(const FunctionCallbackInfo& args) { } uint32_t n_params = args.Length() - 1; - int r = sqlite3_reset(stmt->statement_); + int r = sqlite3_reset(stmt->statement_.get()); CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void()); - int param_count = sqlite3_bind_parameter_count(stmt->statement_); + int param_count = sqlite3_bind_parameter_count(stmt->statement_.get()); for (int i = 0; i < static_cast(n_params) && i < param_count; ++i) { Local value = args[i + 1]; if (!stmt->BindValue(value, i + 1)) { @@ -3385,7 +3384,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); } @@ -3406,9 +3405,9 @@ void SQLTagStore::Iterate(const FunctionCallbackInfo& args) { } uint32_t n_params = args.Length() - 1; - int r = sqlite3_reset(stmt->statement_); + int r = sqlite3_reset(stmt->statement_.get()); CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void()); - int param_count = sqlite3_bind_parameter_count(stmt->statement_); + int param_count = sqlite3_bind_parameter_count(stmt->statement_.get()); for (int i = 0; i < static_cast(n_params) && i < param_count; ++i) { Local value = args[i + 1]; if (!stmt->BindValue(value, i + 1)) { @@ -3443,10 +3442,10 @@ void SQLTagStore::Get(const FunctionCallbackInfo& args) { uint32_t n_params = args.Length() - 1; Isolate* isolate = env->isolate(); - int r = sqlite3_reset(stmt->statement_); + int r = sqlite3_reset(stmt->statement_.get()); CHECK_ERROR_OR_THROW(isolate, stmt->db_.get(), r, SQLITE_OK, void()); - int param_count = sqlite3_bind_parameter_count(stmt->statement_); + int param_count = sqlite3_bind_parameter_count(stmt->statement_.get()); for (int i = 0; i < static_cast(n_params) && i < param_count; ++i) { Local value = args[i + 1]; if (!stmt->BindValue(value, i + 1)) { @@ -3457,7 +3456,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)) { @@ -3482,10 +3481,10 @@ void SQLTagStore::All(const FunctionCallbackInfo& args) { uint32_t n_params = args.Length() - 1; Isolate* isolate = env->isolate(); - int r = sqlite3_reset(stmt->statement_); + int r = sqlite3_reset(stmt->statement_.get()); CHECK_ERROR_OR_THROW(isolate, stmt->db_.get(), r, SQLITE_OK, void()); - int param_count = sqlite3_bind_parameter_count(stmt->statement_); + int param_count = sqlite3_bind_parameter_count(stmt->statement_.get()); for (int i = 0; i < static_cast(n_params) && i < param_count; ++i) { Local value = args[i + 1]; if (!stmt->BindValue(value, i + 1)) { @@ -3493,11 +3492,11 @@ void SQLTagStore::All(const FunctionCallbackInfo& args) { } } - auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt->statement_); }); + auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt->statement_.get()); }); Local result; if (StatementExecutionHelper::All(env, stmt->db_.get(), - stmt->statement_, + stmt->statement_.get(), stmt->return_arrays_, stmt->use_big_ints_) .ToLocal(&result)) { @@ -3563,19 +3562,20 @@ 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(); } - 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(); } @@ -3639,7 +3639,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() @@ -3648,7 +3648,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, @@ -3721,11 +3722,11 @@ 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()); - sqlite3_reset(iter->stmt_->statement_); + sqlite3_reset(iter->stmt_->statement_.get()); MaybeLocal values[] = {Boolean::New(isolate, true), Null(isolate)}; Local result; if (NewDictionaryInstanceNullProto(env->context(), iter_template, values) @@ -3735,13 +3736,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) @@ -3777,7 +3778,7 @@ void StatementSyncIterator::Return(const FunctionCallbackInfo& args) { env, iter->stmt_->IsFinalized(), "statement has been finalized"); Isolate* isolate = env->isolate(); - 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 e7281ed266af5d..e82d8e2f7dcc1b 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -134,6 +134,12 @@ class StatementSyncIterator; class StatementSync; class BackupJob; +inline void FinalizeStatement(sqlite3_stmt* stmt) { + sqlite3_finalize(stmt); +} + +using StatementPtr = DeleteFnPtr; + class StatementExecutionHelper { public: static v8::MaybeLocal All(Environment* env, @@ -257,13 +263,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); @@ -290,7 +296,7 @@ class StatementSync : public BaseObject { private: ~StatementSync() override; BaseObjectPtr db_; - sqlite3_stmt* statement_; + StatementPtr statement_; bool return_arrays_ = false; bool use_big_ints_; bool allow_bare_named_params_;