Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 45d25c4

Browse filesBrowse files
louwersaduh95
authored andcommitted
sqlite: change approach to fix segfault SQLTagStore
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>
1 parent e62608b commit 45d25c4
Copy full SHA for 45d25c4

3 files changed

+42-3Lines changed: 42 additions & 3 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎src/node_sqlite.cc‎

Copy file name to clipboardExpand all lines: src/node_sqlite.cc
+3-3Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2819,6 +2819,7 @@ BaseObjectPtr<SQLTagStore> SQLTagStore::Create(
28192819
.ToLocal(&obj)) {
28202820
return nullptr;
28212821
}
2822+
obj->SetInternalField(kDatabaseObject, database->object());
28222823
return MakeBaseObject<SQLTagStore>(env, obj, std::move(database), capacity);
28232824
}
28242825

@@ -2829,9 +2830,8 @@ void SQLTagStore::CapacityGetter(const FunctionCallbackInfo<Value>& args) {
28292830
}
28302831

28312832
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>());
28352835
}
28362836

28372837
void SQLTagStore::SizeGetter(const FunctionCallbackInfo<Value>& args) {
Collapse file

‎src/node_sqlite.h‎

Copy file name to clipboardExpand all lines: src/node_sqlite.h
+5Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,11 @@ class Session : public BaseObject {
305305

306306
class SQLTagStore : public BaseObject {
307307
public:
308+
enum InternalFields {
309+
kDatabaseObject = BaseObject::kInternalFieldCount,
310+
kInternalFieldCount
311+
};
312+
308313
SQLTagStore(Environment* env,
309314
v8::Local<v8::Object> object,
310315
BaseObjectWeakPtr<DatabaseSync> database,
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-sqlite-template-tag.js
+34Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
'use strict';
2+
// Flags: --expose-gc
3+
24
const { skipIfSQLiteMissing } = require('../common');
35
skipIfSQLiteMissing();
46

@@ -135,3 +137,35 @@ test('sql error messages are descriptive', () => {
135137
message: /no such table/i,
136138
});
137139
});
140+
141+
test('a tag store keeps the database alive by itself', () => {
142+
const sql = new DatabaseSync(':memory:').createTagStore();
143+
144+
sql.db.exec('CREATE TABLE test (data INTEGER)');
145+
146+
global.gc();
147+
148+
// eslint-disable-next-line no-unused-expressions
149+
sql.run`INSERT INTO test (data) VALUES (1)`;
150+
});
151+
152+
test('tag store prevents circular reference leaks', async () => {
153+
const { gcUntil } = require('../common/gc');
154+
155+
const before = process.memoryUsage().heapUsed;
156+
157+
// Create many SQLTagStore + DatabaseSync pairs with circular references
158+
for (let i = 0; i < 1000; i++) {
159+
const sql = new DatabaseSync(':memory:').createTagStore();
160+
sql.db.exec('CREATE TABLE test (data INTEGER)');
161+
// eslint-disable-next-line no-void
162+
sql.db.setAuthorizer(() => void sql.db);
163+
}
164+
165+
// GC until memory stabilizes or give up after 20 attempts
166+
await gcUntil('tag store leak check', () => {
167+
const after = process.memoryUsage().heapUsed;
168+
// Memory should not grow significantly (allow 50% margin for noise)
169+
return after < before * 1.5;
170+
}, 20);
171+
});

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.