1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

sqlite: refactor prepared statement iterator

Browse Source 
This commit refactors the StatementSync iterator implementation
in two primary ways:

- The iterator internal state is no longer exposed to JavaScript.
- The iterator prevents the prepared statement from being GC'ed.

Fixes: https://github.com/nodejs/node/issues/57493
PR-URL: https://github.com/nodejs/node/pull/57569
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
This commit is contained in:
Colin Ihrig 2025-04-03 10:16:24 -04:00 committed by GitHub
parent 3984bc1a36
commit 771b6829e6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 198 additions and 185 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
src/env_properties.h
View File

@ -448,6 +448,7 @@
V(shutdown_wrap_template, v8::ObjectTemplate) \ V(shutdown_wrap_template, v8::ObjectTemplate) \
V(socketaddress_constructor_template, v8::FunctionTemplate) \ V(socketaddress_constructor_template, v8::FunctionTemplate) \
V(sqlite_statement_sync_constructor_template, v8::FunctionTemplate) \ V(sqlite_statement_sync_constructor_template, v8::FunctionTemplate) \
V(sqlite_statement_sync_iterator_constructor_template, v8::FunctionTemplate) \
V(sqlite_session_constructor_template, v8::FunctionTemplate) \ V(sqlite_session_constructor_template, v8::FunctionTemplate) \
V(streambaseentry_ctor_template, v8::FunctionTemplate) \ V(streambaseentry_ctor_template, v8::FunctionTemplate) \
V(streambaseoutputstream_constructor_template, v8::ObjectTemplate) \ V(streambaseoutputstream_constructor_template, v8::ObjectTemplate) \

302
src/node_sqlite.cc
View File

@ -25,7 +25,6 @@ using v8::ConstructorBehavior;
using v8::Context; using v8::Context;
using v8::DontDelete; using v8::DontDelete;
using v8::Exception; using v8::Exception;
using v8::External;
using v8::Function; using v8::Function;
using v8::FunctionCallback; using v8::FunctionCallback;
using v8::FunctionCallbackInfo; using v8::FunctionCallbackInfo;
@ -1620,142 +1619,12 @@ void StatementSync::All(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(Array::New(isolate, rows.data(), rows.size())); args.GetReturnValue().Set(Array::New(isolate, rows.data(), rows.size()));
} }
void StatementSync::IterateReturnCallback(
const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
auto isolate = env->isolate();
auto context = isolate->GetCurrentContext();
auto self = args.This();
// iterator has fetch all result or break, prevent next func to return result
if (self->Set(context, env->isfinished_string(), Boolean::New(isolate, true))
.IsNothing()) {
// An error will have been scheduled.
return;
}
Local<Value> val;
if (!self->Get(context, env->statement_string()).ToLocal(&val)) {
// An error will have been scheduled.
return;
}
auto external_stmt = Local<External>::Cast(val);
auto stmt = static_cast<StatementSync*>(external_stmt->Value());
if (!stmt->IsFinalized()) {
sqlite3_reset(stmt->statement_);
}
LocalVector<Name> keys(isolate, {env->done_string(), env->value_string()});
LocalVector<Value> values(isolate,
{Boolean::New(isolate, true), Null(isolate)});
DCHECK_EQ(keys.size(), values.size());
Local<Object> result = Object::New(
isolate, Null(isolate), keys.data(), values.data(), keys.size());
args.GetReturnValue().Set(result);
}
void StatementSync::IterateNextCallback(
const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
auto isolate = env->isolate();
auto context = isolate->GetCurrentContext();
auto self = args.This();
Local<Value> val;
if (!self->Get(context, env->isfinished_string()).ToLocal(&val)) {
// An error will have been scheduled.
return;
}
// skip iteration if is_finished
auto is_finished = Local<Boolean>::Cast(val);
if (is_finished->Value()) {
Local<Name> keys[] = {env->done_string(), env->value_string()};
Local<Value> values[] = {Boolean::New(isolate, true), Null(isolate)};
static_assert(arraysize(keys) == arraysize(values));
Local<Object> result = Object::New(
isolate, Null(isolate), &keys[0], &values[0], arraysize(keys));
args.GetReturnValue().Set(result);
return;
}
if (!self->Get(context, env->statement_string()).ToLocal(&val)) {
// An error will have been scheduled.
return;
}
auto external_stmt = Local<External>::Cast(val);
auto stmt = static_cast<StatementSync*>(external_stmt->Value());
if (!self->Get(context, env->num_cols_string()).ToLocal(&val)) {
// An error will have been scheduled.
return;
}
auto num_cols = Local<Integer>::Cast(val)->Value();
THROW_AND_RETURN_ON_BAD_STATE(
env, stmt->IsFinalized(), "statement has been finalized");
int r = sqlite3_step(stmt->statement_);
if (r != SQLITE_ROW) {
CHECK_ERROR_OR_THROW(
env->isolate(), stmt->db_.get(), r, SQLITE_DONE, void());
// cleanup when no more rows to fetch
sqlite3_reset(stmt->statement_);
if (self->Set(
context, env->isfinished_string(), Boolean::New(isolate, true))
.IsNothing()) {
// An error would have been scheduled
return;
}
LocalVector<Name> keys(isolate, {env->done_string(), env->value_string()});
LocalVector<Value> values(isolate,
{Boolean::New(isolate, true), Null(isolate)});
DCHECK_EQ(keys.size(), values.size());
Local<Object> result = Object::New(
isolate, Null(isolate), keys.data(), values.data(), keys.size());
args.GetReturnValue().Set(result);
return;
}
LocalVector<Name> row_keys(isolate);
row_keys.reserve(num_cols);
LocalVector<Value> row_values(isolate);
row_values.reserve(num_cols);
for (int i = 0; i < num_cols; ++i) {
Local<Name> key;
if (!stmt->ColumnNameToName(i).ToLocal(&key)) return;
Local<Value> val;
if (!stmt->ColumnToValue(i).ToLocal(&val)) return;
row_keys.emplace_back(key);
row_values.emplace_back(val);
}
Local<Object> row = Object::New(
isolate, Null(isolate), row_keys.data(), row_values.data(), num_cols);
LocalVector<Name> keys(isolate, {env->done_string(), env->value_string()});
LocalVector<Value> values(isolate, {Boolean::New(isolate, false), row});
DCHECK_EQ(keys.size(), values.size());
Local<Object> result = Object::New(
isolate, Null(isolate), keys.data(), values.data(), keys.size());
args.GetReturnValue().Set(result);
}
void StatementSync::Iterate(const FunctionCallbackInfo<Value>& args) { void StatementSync::Iterate(const FunctionCallbackInfo<Value>& args) {
StatementSync* stmt; StatementSync* stmt;
ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This()); ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This());
Environment* env = Environment::GetCurrent(args); Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE( THROW_AND_RETURN_ON_BAD_STATE(
env, stmt->IsFinalized(), "statement has been finalized"); env, stmt->IsFinalized(), "statement has been finalized");
auto isolate = env->isolate();
auto context = env->context(); auto context = env->context();
int r = sqlite3_reset(stmt->statement_); int r = sqlite3_reset(stmt->statement_);
CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void()); CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void());
@ -1764,67 +1633,28 @@ void StatementSync::Iterate(const FunctionCallbackInfo<Value>& args) {
return; return;
} }
Local<Function> next_func;
Local<Function> return_func;
if (!Function::New(context, StatementSync::IterateNextCallback)
.ToLocal(&next_func) ||
!Function::New(context, StatementSync::IterateReturnCallback)
.ToLocal(&return_func)) {
// An error will have been scheduled.
return;
}
Local<Name> keys[] = {env->next_string(), env->return_string()};
Local<Value> values[] = {next_func, return_func};
static_assert(arraysize(keys) == arraysize(values));
Local<Object> global = context->Global(); Local<Object> global = context->Global();
Local<Value> js_iterator; Local<Value> js_iterator;
Local<Value> js_iterator_prototype; Local<Value> js_iterator_prototype;
if (!global->Get(context, env->iterator_string()).ToLocal(&js_iterator)) if (!global->Get(context, env->iterator_string()).ToLocal(&js_iterator)) {
return; return;
}
if (!js_iterator.As<Object>() if (!js_iterator.As<Object>()
->Get(context, env->prototype_string()) ->Get(context, env->prototype_string())
.ToLocal(&js_iterator_prototype)) .ToLocal(&js_iterator_prototype)) {
return;
Local<Object> iterable_iterator = Object::New(
isolate, js_iterator_prototype, &keys[0], &values[0], arraysize(keys));
auto num_cols_pd = v8::PropertyDescriptor(
v8::Integer::New(isolate, sqlite3_column_count(stmt->statement_)), false);
num_cols_pd.set_enumerable(false);
num_cols_pd.set_configurable(false);
if (iterable_iterator
->DefineProperty(context, env->num_cols_string(), num_cols_pd)
.IsNothing()) {
// An error will have been scheduled.
return; return;
} }
auto stmt_pd = BaseObjectPtr<StatementSyncIterator> iter =
v8::PropertyDescriptor(v8::External::New(isolate, stmt), false); StatementSyncIterator::Create(env, BaseObjectPtr<StatementSync>(stmt));
stmt_pd.set_enumerable(false); if (iter->object()
stmt_pd.set_configurable(false); ->GetPrototype()
if (iterable_iterator .As<Object>()
->DefineProperty(context, env->statement_string(), stmt_pd) ->SetPrototype(context, js_iterator_prototype)
.IsNothing()) { .IsNothing()) {
// An error will have been scheduled.
return; return;
} }
args.GetReturnValue().Set(iter->object());
auto is_finished_pd =
v8::PropertyDescriptor(v8::Boolean::New(isolate, false), true);
stmt_pd.set_enumerable(false);
stmt_pd.set_configurable(false);
if (iterable_iterator
->DefineProperty(context, env->isfinished_string(), is_finished_pd)
.IsNothing()) {
// An error will have been scheduled.
return;
}
args.GetReturnValue().Set(iterable_iterator);
} }
void StatementSync::Get(const FunctionCallbackInfo<Value>& args) { void StatementSync::Get(const FunctionCallbackInfo<Value>& args) {
@ -2144,6 +1974,118 @@ BaseObjectPtr<StatementSync> StatementSync::Create(
return MakeBaseObject<StatementSync>(env, obj, std::move(db), stmt); return MakeBaseObject<StatementSync>(env, obj, std::move(db), stmt);
} }
StatementSyncIterator::StatementSyncIterator(Environment* env,
Local<Object> object,
BaseObjectPtr<StatementSync> stmt)
: BaseObject(env, object), stmt_(std::move(stmt)) {
MakeWeak();
done_ = false;
}
StatementSyncIterator::~StatementSyncIterator() {}
void StatementSyncIterator::MemoryInfo(MemoryTracker* tracker) const {}
Local<FunctionTemplate> StatementSyncIterator::GetConstructorTemplate(
Environment* env) {
Local<FunctionTemplate> tmpl =
env->sqlite_statement_sync_iterator_constructor_template();
if (tmpl.IsEmpty()) {
Isolate* isolate = env->isolate();
tmpl = NewFunctionTemplate(isolate, IllegalConstructor);
tmpl->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "StatementSyncIterator"));
tmpl->InstanceTemplate()->SetInternalFieldCount(
StatementSync::kInternalFieldCount);
SetProtoMethod(isolate, tmpl, "next", StatementSyncIterator::Next);
SetProtoMethod(isolate, tmpl, "return", StatementSyncIterator::Return);
env->set_sqlite_statement_sync_iterator_constructor_template(tmpl);
}
return tmpl;
}
BaseObjectPtr<StatementSyncIterator> StatementSyncIterator::Create(
Environment* env, BaseObjectPtr<StatementSync> stmt) {
Local<Object> obj;
if (!GetConstructorTemplate(env)
->InstanceTemplate()
->NewInstance(env->context())
.ToLocal(&obj)) {
return BaseObjectPtr<StatementSyncIterator>();
}
return MakeBaseObject<StatementSyncIterator>(env, obj, std::move(stmt));
}
void StatementSyncIterator::Next(const FunctionCallbackInfo<Value>& args) {
StatementSyncIterator* iter;
ASSIGN_OR_RETURN_UNWRAP(&iter, args.This());
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(
env, iter->stmt_->IsFinalized(), "statement has been finalized");
Isolate* isolate = env->isolate();
LocalVector<Name> keys(isolate, {env->done_string(), env->value_string()});
if (iter->done_) {
LocalVector<Value> values(isolate,
{Boolean::New(isolate, true), Null(isolate)});
Local<Object> result = Object::New(
isolate, Null(isolate), keys.data(), values.data(), keys.size());
args.GetReturnValue().Set(result);
return;
}
int r = sqlite3_step(iter->stmt_->statement_);
if (r != SQLITE_ROW) {
CHECK_ERROR_OR_THROW(
env->isolate(), iter->stmt_->db_.get(), r, SQLITE_DONE, void());
sqlite3_reset(iter->stmt_->statement_);
LocalVector<Value> values(isolate,
{Boolean::New(isolate, true), Null(isolate)});
Local<Object> result = Object::New(
isolate, Null(isolate), keys.data(), values.data(), keys.size());
args.GetReturnValue().Set(result);
return;
}
int num_cols = sqlite3_column_count(iter->stmt_->statement_);
LocalVector<Name> row_keys(isolate);
LocalVector<Value> row_values(isolate);
row_keys.reserve(num_cols);
row_values.reserve(num_cols);
for (int i = 0; i < num_cols; ++i) {
Local<Name> key;
if (!iter->stmt_->ColumnNameToName(i).ToLocal(&key)) return;
Local<Value> val;
if (!iter->stmt_->ColumnToValue(i).ToLocal(&val)) return;
row_keys.emplace_back(key);
row_values.emplace_back(val);
}
Local<Object> row = Object::New(
isolate, Null(isolate), row_keys.data(), row_values.data(), num_cols);
LocalVector<Value> values(isolate, {Boolean::New(isolate, false), row});
Local<Object> result = Object::New(
isolate, Null(isolate), keys.data(), values.data(), keys.size());
args.GetReturnValue().Set(result);
}
void StatementSyncIterator::Return(const FunctionCallbackInfo<Value>& args) {
StatementSyncIterator* iter;
ASSIGN_OR_RETURN_UNWRAP(&iter, args.This());
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(
env, iter->stmt_->IsFinalized(), "statement has been finalized");
Isolate* isolate = env->isolate();
sqlite3_reset(iter->stmt_->statement_);
iter->done_ = true;
LocalVector<Name> keys(isolate, {env->done_string(), env->value_string()});
LocalVector<Value> values(isolate,
{Boolean::New(isolate, true), Null(isolate)});
Local<Object> result = Object::New(
isolate, Null(isolate), keys.data(), values.data(), keys.size());
args.GetReturnValue().Set(result);
}
Session::Session(Environment* env, Session::Session(Environment* env,
Local<Object> object, Local<Object> object,
BaseObjectWeakPtr<DatabaseSync> database, BaseObjectWeakPtr<DatabaseSync> database,

27
src/node_sqlite.h
View File

@ -145,10 +145,29 @@ class StatementSync : public BaseObject {
v8::MaybeLocal<v8::Value> ColumnToValue(const int column); v8::MaybeLocal<v8::Value> ColumnToValue(const int column);
v8::MaybeLocal<v8::Name> ColumnNameToName(const int column); v8::MaybeLocal<v8::Name> ColumnNameToName(const int column);
static void IterateNextCallback( friend class StatementSyncIterator;
const v8::FunctionCallbackInfo<v8::Value>& args); };
static void IterateReturnCallback(
const v8::FunctionCallbackInfo<v8::Value>& args); class StatementSyncIterator : public BaseObject {
public:
StatementSyncIterator(Environment* env,
v8::Local<v8::Object> object,
BaseObjectPtr<StatementSync> stmt);
void MemoryInfo(MemoryTracker* tracker) const override;
static v8::Local<v8::FunctionTemplate> GetConstructorTemplate(
Environment* env);
static BaseObjectPtr<StatementSyncIterator> Create(
Environment* env, BaseObjectPtr<StatementSync> stmt);
static void Next(const v8::FunctionCallbackInfo<v8::Value>& args);
static void Return(const v8::FunctionCallbackInfo<v8::Value>& args);
SET_MEMORY_INFO_NAME(StatementSyncIterator)
SET_SELF_SIZE(StatementSyncIterator)
private:
~StatementSyncIterator() override;
BaseObjectPtr<StatementSync> stmt_;
bool done_;
}; };
using Sqlite3ChangesetGenFunc = int (*)(sqlite3_session*, int*, void**); using Sqlite3ChangesetGenFunc = int (*)(sqlite3_session*, int*, void**);

53
test/parallel/test-sqlite-statement-sync.js
View File

@ -1,3 +1,4 @@
// Flags: --expose-gc
'use strict'; 'use strict';
require('../common'); require('../common');
const tmpdir = require('../common/tmpdir'); const tmpdir = require('../common/tmpdir');
@ -88,7 +89,10 @@ suite('StatementSync.prototype.iterate()', () => {
const db = new DatabaseSync(nextDb()); const db = new DatabaseSync(nextDb());
t.after(() => { db.close(); }); t.after(() => { db.close(); });
const stmt = db.prepare('CREATE TABLE storage(key TEXT, val TEXT)'); const stmt = db.prepare('CREATE TABLE storage(key TEXT, val TEXT)');
t.assert.deepStrictEqual(stmt.iterate().toArray(), []); const iter = stmt.iterate();
t.assert.strictEqual(iter instanceof globalThis.Iterator, true);
t.assert.ok(iter[Symbol.iterator]);
t.assert.deepStrictEqual(iter.toArray(), []);
}); });
test('executes a query and returns all results', (t) => { test('executes a query and returns all results', (t) => {
@ -119,6 +123,53 @@ suite('StatementSync.prototype.iterate()', () => {
t.assert.deepStrictEqual(item, itemsLoop.shift()); t.assert.deepStrictEqual(item, itemsLoop.shift());
} }
}); });
test('iterator keeps the prepared statement from being collected', (t) => {
const db = new DatabaseSync(':memory:');
db.exec(`
CREATE TABLE test(key TEXT, val TEXT);
INSERT INTO test (key, val) VALUES ('key1', 'val1');
INSERT INTO test (key, val) VALUES ('key2', 'val2');
`);
// Do not keep an explicit reference to the prepared statement.
const iterator = db.prepare('SELECT * FROM test').iterate();
const results = [];
global.gc();
for (const item of iterator) {
results.push(item);
}
t.assert.deepStrictEqual(results, [
{ __proto__: null, key: 'key1', val: 'val1' },
{ __proto__: null, key: 'key2', val: 'val2' },
]);
});
test('iterator can be exited early', (t) => {
const db = new DatabaseSync(':memory:');
db.exec(`
CREATE TABLE test(key TEXT, val TEXT);
INSERT INTO test (key, val) VALUES ('key1', 'val1');
INSERT INTO test (key, val) VALUES ('key2', 'val2');
`);
const iterator = db.prepare('SELECT * FROM test').iterate();
const results = [];
for (const item of iterator) {
results.push(item);
break;
}
t.assert.deepStrictEqual(results, [
{ __proto__: null, key: 'key1', val: 'val1' },
]);
t.assert.deepStrictEqual(
iterator.next(),
{ __proto__: null, done: true, value: null },
);
});
}); });
suite('StatementSync.prototype.run()', () => { suite('StatementSync.prototype.run()', () => {