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();