sqlite: change approach to fix segfault SQLTagStore

louwers · aduh95 · commit 0e7571ae3e97 · 2026-02-22T23:16:13.000+01:00
PR-URL: #60462 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc
@@ -2819,6 +2819,7 @@ BaseObjectPtr<SQLTagStore> SQLTagStore::Create(
            .ToLocal(&obj)) {
     return nullptr;
   }
+  obj->SetInternalField(kDatabaseObject, database->object());
   return MakeBaseObject<SQLTagStore>(env, obj, std::move(database), capacity);
 }
 
@@ -2829,9 +2830,8 @@ void SQLTagStore::CapacityGetter(const FunctionCallbackInfo<Value>& args) {
 }
 
 void SQLTagStore::DatabaseGetter(const FunctionCallbackInfo<Value>& args) {
-  SQLTagStore* store;
-  ASSIGN_OR_RETURN_UNWRAP(&store, args.This());
-  args.GetReturnValue().Set(store->database_->object());
+  args.GetReturnValue().Set(
+      args.This()->GetInternalField(kDatabaseObject).As<Value>());
 }
 
 void SQLTagStore::SizeGetter(const FunctionCallbackInfo<Value>& args) {
diff --git a/src/node_sqlite.h b/src/node_sqlite.h
@@ -305,6 +305,11 @@ class Session : public BaseObject {
 
 class SQLTagStore : public BaseObject {
  public:
+  enum InternalFields {
+    kDatabaseObject = BaseObject::kInternalFieldCount,
+    kInternalFieldCount
+  };
+
   SQLTagStore(Environment* env,
               v8::Local<v8::Object> object,
               BaseObjectWeakPtr<DatabaseSync> database,
diff --git a/test/parallel/test-sqlite-template-tag.js b/test/parallel/test-sqlite-template-tag.js
@@ -1,4 +1,6 @@
 'use strict';
+// Flags: --expose-gc
+
 const { skipIfSQLiteMissing } = require('../common');
 skipIfSQLiteMissing();
 
@@ -135,3 +137,35 @@ test('sql error messages are descriptive', () => {
     message: /no such table/i,
   });
 });
+
+test('a tag store keeps the database alive by itself', () => {
+  const sql = new DatabaseSync(':memory:').createTagStore();
+
+  sql.db.exec('CREATE TABLE test (data INTEGER)');
+
+  global.gc();
+
+  // eslint-disable-next-line no-unused-expressions
+  sql.run`INSERT INTO test (data) VALUES (1)`;
+});
+
+test('tag store prevents circular reference leaks', async () => {
+  const { gcUntil } = require('../common/gc');
+
+  const before = process.memoryUsage().heapUsed;
+
+  // Create many SQLTagStore + DatabaseSync pairs with circular references
+  for (let i = 0; i < 1000; i++) {
+    const sql = new DatabaseSync(':memory:').createTagStore();
+    sql.db.exec('CREATE TABLE test (data INTEGER)');
+    // eslint-disable-next-line no-void
+    sql.db.setAuthorizer(() => void sql.db);
+  }
+
+  // GC until memory stabilizes or give up after 20 attempts
+  await gcUntil('tag store leak check', () => {
+    const after = process.memoryUsage().heapUsed;
+    // Memory should not grow significantly (allow 50% margin for noise)
+    return after < before * 1.5;
+  }, 20);
+});

Original file line number	Diff line number	Diff line change
`@@ -2819,6 +2819,7 @@ BaseObjectPtr<SQLTagStore> SQLTagStore::Create(`
`2819`	`2819`	`.ToLocal(&obj)) {`
`2820`	`2820`	`return nullptr;`
`2821`	`2821`	`}`
	`2822`	`+ obj->SetInternalField(kDatabaseObject, database->object());`
`2822`	`2823`	`return MakeBaseObject<SQLTagStore>(env, obj, std::move(database), capacity);`
`2823`	`2824`	`}`
`2824`	`2825`
`@@ -2829,9 +2830,8 @@ void SQLTagStore::CapacityGetter(const FunctionCallbackInfo<Value>& args) {`
`2829`	`2830`	`}`
`2830`	`2831`
`2831`	`2832`	`void SQLTagStore::DatabaseGetter(const FunctionCallbackInfo<Value>& args) {`
`2832`		`- SQLTagStore* store;`
`2833`		`- ASSIGN_OR_RETURN_UNWRAP(&store, args.This());`
`2834`		`- args.GetReturnValue().Set(store->database_->object());`
	`2833`	`+ args.GetReturnValue().Set(`
	`2834`	`+ args.This()->GetInternalField(kDatabaseObject).As<Value>());`
`2835`	`2835`	`}`
`2836`	`2836`
`2837`	`2837`	`void SQLTagStore::SizeGetter(const FunctionCallbackInfo<Value>& args) {`