sqlite: keep sessions alive across SQLite callbacks

@@ -1024,6 +1024,15 @@ void DatabaseSync::RemoveBackup(BackupJob* job) {   backups_.erase(job); } +std::vector<BaseObjectPtr<Session>> DatabaseSync::PinSessions() const {+  std::vector<BaseObjectPtr<Session>> pinned;+  pinned.reserve(sessions_.size());+  for (Session* session : sessions_) {+    pinned.emplace_back(session);+  }+  return pinned;+}+ void DatabaseSync::DeleteSessions() {   // all attached sessions need to be deleted before the database is closed   // https://www.sqlite.org/session/sqlite3session_create.html@@ -2832,6 +2841,10 @@ int DatabaseSync::TraceCallback(unsigned int type,     return 0;   } +  // Entered before building the payload below, because allocating it can+  // trigger a garbage collection that SQLite is not prepared for.+  CallbackDepthGuard guard(db);+   Isolate* isolate = env->isolate();   HandleScope handle_scope(isolate); @@ -2870,7 +2883,6 @@ int DatabaseSync::TraceCallback(unsigned int type,    Local<Object> payload = Object::New(isolate, Null(isolate), keys, values, 3); -  CallbackDepthGuard guard(db);   ch->Publish(env, payload);    return 0;

@@ -292,6 +292,13 @@ class DatabaseSync : public BaseObject {   void DecrementCallbackDepth() { --callback_depth_; }   bool IsInCallback() const { return callback_depth_ > 0; } +  // SQLite reaches back into JavaScript from inside its pre-update hook, while+  // it is still walking this connection's session list. Session objects are+  // weak, so a garbage collection during such a callback could collect one and+  // free memory SQLite is still using. Returns a strong reference to every+  // attached session so that a callback can hold them for its duration.+  std::vector<BaseObjectPtr<Session>> PinSessions() const;+   // SQLite forbids an authorizer callback from doing anything that modifies   // the database connection that invoked it, which includes preparing and   // stepping statements. See https://www.sqlite.org/c3ref/set_authorizer.html.@@ -508,9 +515,13 @@ class SQLTagStore : public BaseObject {   friend class StatementExecutionHelper; }; +// Guards a window in which SQLite hands control back to JavaScript. Construct+// it before allocating anything on the V8 heap, since the pinned sessions+// below are what keep a garbage collection during that window safe. class CallbackDepthGuard {  public:-  explicit CallbackDepthGuard(DatabaseSync* db) : db_(db) {+  explicit CallbackDepthGuard(DatabaseSync* db)+      : db_(db), pinned_sessions_(db->PinSessions()) {     db_->IncrementCallbackDepth();   }   ~CallbackDepthGuard() { db_->DecrementCallbackDepth(); }@@ -519,6 +530,7 @@ class CallbackDepthGuard {   private:   DatabaseSync* db_;+  std::vector<BaseObjectPtr<Session>> pinned_sessions_; };  class TraceEventSuppressionGuard {

@@ -664,6 +664,50 @@ test('session - keeps its database alive after the db handle is dropped', async   session.close(); }); +// SQLite runs "PRAGMA table_xinfo" from inside its pre-update hook, while it is+// still walking the connection's session list. Session objects are weak, so a+// GC during a callback that the PRAGMA triggers could collect a session that+// JavaScript no longer references and free memory the walk is still using.+test('session - survives GC during an authorizer callback', (t) => {+  const database = new DatabaseSync(':memory:');+  database.exec('CREATE TABLE data(key INTEGER PRIMARY KEY)');+  database.createSession(); // Never referenced again, so it is collectable.++  let ran = false;+  database.setAuthorizer((actionCode, param1) => {+    if (actionCode === constants.SQLITE_PRAGMA && param1 === 'table_xinfo') {+      ran = true;+      globalThis.gc();+      globalThis.gc();+    }+    return constants.SQLITE_OK;+  });++  database.exec('INSERT INTO data VALUES (1)');+  t.assert.ok(ran, 'the authorizer callback never ran');+});++test("session - survives GC during a 'sqlite.db.query' subscriber", (t) => {+  const dc = require('node:diagnostics_channel');+  const database = new DatabaseSync(':memory:');+  database.exec('CREATE TABLE data(key INTEGER PRIMARY KEY)');+  database.createSession(); // Never referenced again, so it is collectable.++  let ran = false;+  const handler = ({ sql }) => {+    if (sql.includes('table_xinfo')) {+      ran = true;+      globalThis.gc();+      globalThis.gc();+    }+  };+  dc.subscribe('sqlite.db.query', handler);+  t.after(() => dc.unsubscribe('sqlite.db.query', handler));++  database.exec('INSERT INTO data VALUES (1)');+  t.assert.ok(ran, 'the subscriber never ran');+});+ test('session supports ERM', (t) => {   const database = new DatabaseSync(':memory:');   let afterDisposeSession;