From 800d5a1f2e8caa9d67b8fd3094c18ecf54677cd7 Mon Sep 17 00:00:00 2001 From: Bart Louwers Date: Tue, 28 Oct 2025 16:00:33 +0100 Subject: [PATCH 1/5] sqlite: fix segfault SQLTagStore when db handle is garbage collected --- src/node_sqlite.cc | 6 +++--- src/node_sqlite.h | 9 +++++---- test/parallel/test-sqlite-template-tag.js | 14 ++++++++++++++ 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 0b111ce1fb1d59..e075a1e2a92908 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -832,7 +832,7 @@ void DatabaseSync::CreateTagStore(const FunctionCallbackInfo& args) { } BaseObjectPtr session = - SQLTagStore::Create(env, BaseObjectWeakPtr(db), capacity); + SQLTagStore::Create(env, BaseObjectPtr(db), capacity); if (!session) { // Handle error if creation failed THROW_ERR_SQLITE_ERROR(env->isolate(), "Failed to create SQLTagStore"); @@ -2660,7 +2660,7 @@ void IllegalConstructor(const FunctionCallbackInfo& args) { SQLTagStore::SQLTagStore(Environment* env, Local object, - BaseObjectWeakPtr database, + BaseObjectPtr database, int capacity) : BaseObject(env, object), database_(std::move(database)), @@ -2709,7 +2709,7 @@ Local SQLTagStore::GetConstructorTemplate(Environment* env) { } BaseObjectPtr SQLTagStore::Create( - Environment* env, BaseObjectWeakPtr database, int capacity) { + Environment* env, BaseObjectPtr database, int capacity) { Local obj; if (!GetConstructorTemplate(env) ->InstanceTemplate() diff --git a/src/node_sqlite.h b/src/node_sqlite.h index 8f0f9f15d621d5..667e45b03562c7 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -306,11 +306,12 @@ class SQLTagStore : public BaseObject { public: SQLTagStore(Environment* env, v8::Local object, - BaseObjectWeakPtr database, + BaseObjectPtr database, int capacity); ~SQLTagStore() override; - static BaseObjectPtr Create( - Environment* env, BaseObjectWeakPtr database, int capacity); + static BaseObjectPtr Create(Environment* env, + BaseObjectPtr database, + int capacity); static v8::Local GetConstructorTemplate( Environment* env); static void All(const v8::FunctionCallbackInfo& info); @@ -329,7 +330,7 @@ class SQLTagStore : public BaseObject { private: static BaseObjectPtr PrepareStatement( const v8::FunctionCallbackInfo& args); - BaseObjectWeakPtr database_; + BaseObjectPtr database_; LRUCache> sql_tags_; int capacity_; friend class StatementExecutionHelper; diff --git a/test/parallel/test-sqlite-template-tag.js b/test/parallel/test-sqlite-template-tag.js index 8628200cf8930f..810b034089dbd1 100644 --- a/test/parallel/test-sqlite-template-tag.js +++ b/test/parallel/test-sqlite-template-tag.js @@ -100,3 +100,17 @@ test('TagStore capacity, size, and clear', () => { test('sql.db returns the associated DatabaseSync instance', () => { assert.strictEqual(sql.db, db); }); + +test('regression test https://github.com/nodejs/node/issues/60448', () => { + const sql = new DatabaseSync(':memory:').createTagStore(); + + sql.db.exec('CREATE TABLE test (data NUMBER)'); + + // Simulating meaningful work/likely triggering garbage collection... + for (const x of new Array(100_000).fill(0)) { + // eslint-disable-next-line no-unused-expressions + x + x; + } + // eslint-disable-next-line no-unused-expressions + sql.run`INSERT INTO test (data) VALUES (1)`; +}); From 37f0477adced553c1ddbfef8f402af4fd936e7c0 Mon Sep 17 00:00:00 2001 From: Bart Louwers Date: Tue, 28 Oct 2025 16:02:52 +0100 Subject: [PATCH 2/5] use INTEGER instead of NUMBER --- test/parallel/test-sqlite-template-tag.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-sqlite-template-tag.js b/test/parallel/test-sqlite-template-tag.js index 810b034089dbd1..4735a45acbb24e 100644 --- a/test/parallel/test-sqlite-template-tag.js +++ b/test/parallel/test-sqlite-template-tag.js @@ -104,7 +104,7 @@ test('sql.db returns the associated DatabaseSync instance', () => { test('regression test https://github.com/nodejs/node/issues/60448', () => { const sql = new DatabaseSync(':memory:').createTagStore(); - sql.db.exec('CREATE TABLE test (data NUMBER)'); + sql.db.exec('CREATE TABLE test (data INTEGER)'); // Simulating meaningful work/likely triggering garbage collection... for (const x of new Array(100_000).fill(0)) { From f2039eb2f06a8dd9b8287ad0039ab0ae85f716ba Mon Sep 17 00:00:00 2001 From: Bart Louwers Date: Tue, 28 Oct 2025 19:46:56 +0100 Subject: [PATCH 3/5] test: use --expose-gc --- test/parallel/test-sqlite-template-tag.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/test/parallel/test-sqlite-template-tag.js b/test/parallel/test-sqlite-template-tag.js index 4735a45acbb24e..7979befacdc6a1 100644 --- 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 + require('../common'); const assert = require('assert'); const { DatabaseSync } = require('node:sqlite'); @@ -106,11 +108,8 @@ test('regression test https://github.com/nodejs/node/issues/60448', () => { sql.db.exec('CREATE TABLE test (data INTEGER)'); - // Simulating meaningful work/likely triggering garbage collection... - for (const x of new Array(100_000).fill(0)) { - // eslint-disable-next-line no-unused-expressions - x + x; - } + global.gc(); + // eslint-disable-next-line no-unused-expressions sql.run`INSERT INTO test (data) VALUES (1)`; }); From b633db4957788c17bf790c38f279ee74a4b09b92 Mon Sep 17 00:00:00 2001 From: Bart Louwers Date: Tue, 28 Oct 2025 19:59:56 +0100 Subject: [PATCH 4/5] test: rename test and remove GitHub link --- test/parallel/test-sqlite-template-tag.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-sqlite-template-tag.js b/test/parallel/test-sqlite-template-tag.js index 7979befacdc6a1..4889f83a724703 100644 --- a/test/parallel/test-sqlite-template-tag.js +++ b/test/parallel/test-sqlite-template-tag.js @@ -103,7 +103,7 @@ test('sql.db returns the associated DatabaseSync instance', () => { assert.strictEqual(sql.db, db); }); -test('regression test https://github.com/nodejs/node/issues/60448', () => { +test('a tag store keeps the database alive by itself', () => { const sql = new DatabaseSync(':memory:').createTagStore(); sql.db.exec('CREATE TABLE test (data INTEGER)'); From b8df359775a9d24bf9d4aa0d09070f38402326cb Mon Sep 17 00:00:00 2001 From: Bart Louwers Date: Tue, 28 Oct 2025 20:35:38 +0100 Subject: [PATCH 5/5] test: run linter --- test/parallel/test-sqlite-template-tag.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-sqlite-template-tag.js b/test/parallel/test-sqlite-template-tag.js index 4889f83a724703..0ba7ffe4a136a4 100644 --- a/test/parallel/test-sqlite-template-tag.js +++ b/test/parallel/test-sqlite-template-tag.js @@ -1,5 +1,5 @@ 'use strict'; -// Flags: --expose-gc +// Flags: --expose-gc require('../common'); const assert = require('assert');