FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

sqlite: invalidate sessions when closing database · nodejs/node@90a8b23 · GitHub

/ node Public

Commit 90a8b23

Browse files
trivikr
authored and
aduh95
committed
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 PR-URL: #64783 Fixes: #64782 Reviewed-By: Aviv Keller <me@aviv.sh>
1 parent cf425ad commit 90a8b23

3 files changed

Lines changed: 37 additions & 7 deletions

File tree

‎src/node_sqlite.cc‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -912,10 +912,9 @@ void DatabaseSync::RemoveBackup(BackupJob* job) {
912912
void DatabaseSync::DeleteSessions() {
913913
// all attached sessions need to be deleted before the database is closed
914914
// https://www.sqlite.org/session/sqlite3session_create.html
915-
for (auto* session : sessions_) {
916-
sqlite3session_delete(session);
915+
while (!sessions_.empty()) {
916+
(*sessions_.begin())->Delete();
917917
}
918-
sessions_.clear();
919918
}
920919

921920
DatabaseSync::~DatabaseSync() {
@@ -2118,13 +2117,22 @@ void DatabaseSync::CreateSession(const FunctionCallbackInfo<Value>& args) {
21182117
sqlite3_session* pSession;
21192118
int r = sqlite3session_create(db->connection_, db_name.c_str(), &pSession);
21202119
CHECK_ERROR_OR_THROW(env->isolate(), db, r, SQLITE_OK, void());
2121-
db->sessions_.insert(pSession);
2120+
bool wrapper_owns_session = false;
2121+
auto delete_session_on_failure = OnScopeLeave([&]() {
2122+
if (!wrapper_owns_session) {
2123+
sqlite3session_delete(pSession);
2124+
}
2125+
});
21222126

21232127
r = sqlite3session_attach(pSession, table == "" ? nullptr : table.c_str());
21242128
CHECK_ERROR_OR_THROW(env->isolate(), db, r, SQLITE_OK, void());
21252129

21262130
BaseObjectPtr<Session> session =
21272131
Session::Create(env, BaseObjectPtr<DatabaseSync>(db), pSession);
2132+
if (!session) {
2133+
return;
2134+
}
2135+
wrapper_owns_session = true;
21282136
args.GetReturnValue().Set(session->object());
21292137
}
21302138

@@ -3813,6 +3821,7 @@ Session::Session(Environment* env,
38133821
: BaseObject(env, object),
38143822
session_(session),
38153823
database_(std::move(database)) {
3824+
database_->sessions_.insert(this);
38163825
MakeWeak();
38173826
}
38183827

@@ -3905,10 +3914,12 @@ void Session::Dispose(const v8::FunctionCallbackInfo<v8::Value>& args) {
39053914
}
39063915

39073916
void Session::Delete() {
3908-
if (!database_ || !database_->connection_ || session_ == nullptr) return;
3917+
if (session_ == nullptr) return;
39093918
sqlite3session_delete(session_);
3910-
database_->sessions_.erase(session_);
39113919
session_ = nullptr;
3920+
if (database_) {
3921+
database_->sessions_.erase(this);
3922+
}
39123923
}
39133924

39143925
void DefineConstants(Local<Object> target) {

‎src/node_sqlite.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ class DatabaseSyncLimits;
133133
class StatementSyncIterator;
134134
class StatementSync;
135135
class BackupJob;
136+
class Session;
136137

137138
class StatementExecutionHelper {
138139
public:
@@ -243,7 +244,7 @@ class DatabaseSync : public BaseObject {
243244
bool ignore_next_sqlite_error_;
244245

245246
std::set<BackupJob*> backups_;
246-
std::set<sqlite3_session*> sessions_;
247+
std::unordered_set<Session*> sessions_;
247248
std::unordered_set<StatementSync*> statements_;
248249

249250
friend class DatabaseSyncLimits;
@@ -360,6 +361,8 @@ class Session : public BaseObject {
360361
void Delete();
361362
sqlite3_session* session_;
362363
BaseObjectPtr<DatabaseSync> database_; // The Parent Database
364+
365+
friend class DatabaseSync;
363366
};
364367

365368
class SQLTagStore : public BaseObject {

‎test/parallel/test-sqlite-session.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,22 @@ test('session.changeset() - closed database results in exception', (t) => {
8181
});
8282
});
8383

84+
test('session methods - reopened database results in exception', (t) => {
85+
for (const method of ['changeset', 'close']) {
86+
const database = new DatabaseSync(':memory:');
87+
const session = database.createSession();
88+
database.close();
89+
database.open();
90+
91+
t.assert.throws(() => {
92+
session[method]();
93+
}, {
94+
name: 'Error',
95+
message: 'session is not open',
96+
});
97+
}
98+
});
99+
84100
test('database.applyChangeset() - closed database results in exception', (t) => {
85101
const database = new DatabaseSync(':memory:');
86102
const session = database.createSession();

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL