sqlite: keep sessions alive across SQLite callbacks · nodejs/node@5f95032 · GitHub
Skip to content

Commit 5f95032

Browse files
TrevorBurnhamaduh95
authored andcommitted
sqlite: keep sessions alive across SQLite callbacks
Session objects are weak and nothing else holds a strong reference to them, so a garbage collection can free one while SQLite is still using it. SQLite runs "PRAGMA table_xinfo" from inside its pre-update hook, which reaches JavaScript, so a GC during a callback invoked from there can collect a session the hook is still walking. Hold a strong reference to every attached session for the duration of each callback SQLite invokes. The trace callback now enters that guard before building its payload, since the allocation can itself trigger a garbage collection. Fixes: #65460 Signed-off-by: Trevor Burnham <trevorburnham@gmail.com> PR-URL: #65465 Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
1 parent f9175a2 commit 5f95032

3 files changed

Lines changed: 70 additions & 2 deletions

File tree

‎src/node_sqlite.cc‎

Lines changed: 13 additions & 1 deletion

‎src/node_sqlite.h‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,13 @@ class DatabaseSync : public BaseObject {
289289
void DecrementCallbackDepth() { --callback_depth_; }
290290
bool IsInCallback() const { return callback_depth_ > 0; }
291291

292+
// SQLite reaches back into JavaScript from inside its pre-update hook, while
293+
// it is still walking this connection's session list. Session objects are
294+
// weak, so a garbage collection during such a callback could collect one and
295+
// free memory SQLite is still using. Returns a strong reference to every
296+
// attached session so that a callback can hold them for its duration.
297+
std::vector<BaseObjectPtr<Session>> PinSessions() const;
298+
292299
// SQLite forbids an authorizer callback from doing anything that modifies
293300
// the database connection that invoked it, which includes preparing and
294301
// stepping statements. See https://www.sqlite.org/c3ref/set_authorizer.html.
@@ -505,9 +512,13 @@ class SQLTagStore : public BaseObject {
505512
friend class StatementExecutionHelper;
506513
};
507514

515+
// Guards a window in which SQLite hands control back to JavaScript. Construct
516+
// it before allocating anything on the V8 heap, since the pinned sessions
517+
// below are what keep a garbage collection during that window safe.
508518
class CallbackDepthGuard {
509519
public:
510-
explicit CallbackDepthGuard(DatabaseSync* db) : db_(db) {
520+
explicit CallbackDepthGuard(DatabaseSync* db)
521+
: db_(db), pinned_sessions_(db->PinSessions()) {
511522
db_->IncrementCallbackDepth();
512523
}
513524
~CallbackDepthGuard() { db_->DecrementCallbackDepth(); }
@@ -516,6 +527,7 @@ class CallbackDepthGuard {
516527

517528
private:
518529
DatabaseSync* db_;
530+
std::vector<BaseObjectPtr<Session>> pinned_sessions_;
519531
};
520532

521533
class TraceEventSuppressionGuard {

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

Lines changed: 44 additions & 0 deletions

0 commit comments

Comments
 (0)