From c51eb62188b4cea1eb1b1e203220b65aaaea6ff3 Mon Sep 17 00:00:00 2001 From: "Kamat, Trivikram" <16024985+trivikr@users.noreply.github.com> Date: Mon, 27 Jul 2026 07:37:42 -0700 Subject: [PATCH] sqlite: invalidate sessions when closing database DatabaseSync::DeleteSessions() deleted native sqlite3_session handles without clearing the pointers held by their Session wrappers. Reopening the database allowed stale session handles to be used, crashing the process. Track Session wrappers and delete sessions through Session::Delete() so the wrappers are marked closed. Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: codex:gpt-5.6-sol --- src/node_sqlite.cc | 23 +++++++++++++++++------ src/node_sqlite.h | 5 ++++- test/parallel/test-sqlite-session.js | 16 ++++++++++++++++ 3 files changed, 37 insertions(+), 7 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 8e2f13cd633439..20b9de31724cd0 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -912,10 +912,9 @@ void DatabaseSync::RemoveBackup(BackupJob* job) { void DatabaseSync::DeleteSessions() { // all attached sessions need to be deleted before the database is closed // https://www.sqlite.org/session/sqlite3session_create.html - for (auto* session : sessions_) { - sqlite3session_delete(session); + while (!sessions_.empty()) { + (*sessions_.begin())->Delete(); } - sessions_.clear(); } DatabaseSync::~DatabaseSync() { @@ -2118,13 +2117,22 @@ void DatabaseSync::CreateSession(const FunctionCallbackInfo& args) { sqlite3_session* pSession; int r = sqlite3session_create(db->connection_, db_name.c_str(), &pSession); CHECK_ERROR_OR_THROW(env->isolate(), db, r, SQLITE_OK, void()); - db->sessions_.insert(pSession); + bool wrapper_owns_session = false; + auto delete_session_on_failure = OnScopeLeave([&]() { + if (!wrapper_owns_session) { + sqlite3session_delete(pSession); + } + }); r = sqlite3session_attach(pSession, table == "" ? nullptr : table.c_str()); CHECK_ERROR_OR_THROW(env->isolate(), db, r, SQLITE_OK, void()); BaseObjectPtr session = Session::Create(env, BaseObjectPtr(db), pSession); + if (!session) { + return; + } + wrapper_owns_session = true; args.GetReturnValue().Set(session->object()); } @@ -3807,6 +3815,7 @@ Session::Session(Environment* env, : BaseObject(env, object), session_(session), database_(std::move(database)) { + database_->sessions_.insert(this); MakeWeak(); } @@ -3899,10 +3908,12 @@ void Session::Dispose(const v8::FunctionCallbackInfo& args) { } void Session::Delete() { - if (!database_ || !database_->connection_ || session_ == nullptr) return; + if (session_ == nullptr) return; sqlite3session_delete(session_); - database_->sessions_.erase(session_); session_ = nullptr; + if (database_) { + database_->sessions_.erase(this); + } } void DefineConstants(Local target) { diff --git a/src/node_sqlite.h b/src/node_sqlite.h index 84c0e26e61ad8f..48463b215cb319 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -133,6 +133,7 @@ class DatabaseSyncLimits; class StatementSyncIterator; class StatementSync; class BackupJob; +class Session; class StatementExecutionHelper { public: @@ -243,7 +244,7 @@ class DatabaseSync : public BaseObject { bool ignore_next_sqlite_error_; std::set backups_; - std::set sessions_; + std::unordered_set sessions_; std::unordered_set statements_; friend class DatabaseSyncLimits; @@ -360,6 +361,8 @@ class Session : public BaseObject { void Delete(); sqlite3_session* session_; BaseObjectPtr database_; // The Parent Database + + friend class DatabaseSync; }; class SQLTagStore : public BaseObject { diff --git a/test/parallel/test-sqlite-session.js b/test/parallel/test-sqlite-session.js index ad481ba4de77cb..c36b4352a3416f 100644 --- a/test/parallel/test-sqlite-session.js +++ b/test/parallel/test-sqlite-session.js @@ -81,6 +81,22 @@ test('session.changeset() - closed database results in exception', (t) => { }); }); +test('session methods - reopened database results in exception', (t) => { + for (const method of ['changeset', 'close']) { + const database = new DatabaseSync(':memory:'); + const session = database.createSession(); + database.close(); + database.open(); + + t.assert.throws(() => { + session[method](); + }, { + name: 'Error', + message: 'session is not open', + }); + } +}); + test('database.applyChangeset() - closed database results in exception', (t) => { const database = new DatabaseSync(':memory:'); const session = database.createSession();