sqlite: change approach to fix segfault SQLTagStore

Author: louwers

src/node_sqlite.cc

@@ -831,8 +831,8 @@ void DatabaseSync::CreateTagStore(const FunctionCallbackInfo<Value>& args) {
     capacity = args[0].As<Number>()->Value();
   }
 
-  BaseObjectPtr<SQLTagStore> session =
-      SQLTagStore::Create(env, BaseObjectWeakPtr<DatabaseSync>(db), capacity);
+  BaseObjectPtr<SQLTagStore> session = SQLTagStore::Create(
+      env, BaseObjectWeakPtr<DatabaseSync>(db), args.This(), capacity);
   if (!session) {
     // Handle error if creation failed
     THROW_ERR_SQLITE_ERROR(env->isolate(), "Failed to create SQLTagStore");
@@ -2710,14 +2710,18 @@ Local<FunctionTemplate> SQLTagStore::GetConstructorTemplate(Environment* env) {
 }
 
 BaseObjectPtr<SQLTagStore> SQLTagStore::Create(
-    Environment* env, BaseObjectWeakPtr<DatabaseSync> database, int capacity) {
+    Environment* env,
+    BaseObjectWeakPtr<DatabaseSync> database,
+    Local<Object> database_object,
+    int capacity) {
   Local<Object> obj;
   if (!GetConstructorTemplate(env)
            ->InstanceTemplate()
            ->NewInstance(env->context())
            .ToLocal(&obj)) {
     return nullptr;
   }
+  obj->SetInternalField(kDatabaseObject, database_object);
   return MakeBaseObject<SQLTagStore>(env, obj, std::move(database), capacity);
 }
 
@@ -2728,9 +2732,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) {

src/node_sqlite.h

@@ -304,13 +304,21 @@ 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,
               int capacity);
   ~SQLTagStore() override;
   static BaseObjectPtr<SQLTagStore> Create(
-      Environment* env, BaseObjectWeakPtr<DatabaseSync> database, int capacity);
+      Environment* env,
+      BaseObjectWeakPtr<DatabaseSync> database,
+      v8::Local<v8::Object> database_object,
+      int capacity);
   static v8::Local<v8::FunctionTemplate> GetConstructorTemplate(
       Environment* env);
   static void All(const v8::FunctionCallbackInfo<v8::Value>& args);

test/parallel/test-sqlite-template-tag.js

@@ -1,4 +1,6 @@
 'use strict';
+// Flags: --expose-gc
+
 const { skipIfSQLiteMissing } = require('../common');
 skipIfSQLiteMissing();
 
@@ -124,3 +126,34 @@ 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)');
+    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);
+});