Skip to content

Commit 26fa594

Browse files
cjihrigRafaelGSS
cjihrig
authored and
RafaelGSS
committed
sqlite: refactor prepared statement iterator
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: #57493 PR-URL: #57569 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Edy Silva <[email protected]> Reviewed-By: Zeyu "Alex" Yang <[email protected]>
1 parent d066d1f commit 26fa594

File tree

4 files changed

+198
-185
lines changed

4 files changed

+198
-185
lines changed

‎src/env_properties.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,7 @@
448448
V(shutdown_wrap_template, v8::ObjectTemplate) \
449449
V(socketaddress_constructor_template, v8::FunctionTemplate) \
450450
V(sqlite_statement_sync_constructor_template, v8::FunctionTemplate) \
451+
V(sqlite_statement_sync_iterator_constructor_template, v8::FunctionTemplate) \
451452
V(sqlite_session_constructor_template, v8::FunctionTemplate) \
452453
V(streambaseentry_ctor_template, v8::FunctionTemplate) \
453454
V(streambaseoutputstream_constructor_template, v8::ObjectTemplate) \

‎src/node_sqlite.cc‎

Lines changed: 122 additions & 180 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ using v8::ConstructorBehavior;
2525
using v8::Context;
2626
using v8::DontDelete;
2727
using v8::Exception;
28-
using v8::External;
2928
using v8::Function;
3029
using v8::FunctionCallback;
3130
using v8::FunctionCallbackInfo;
@@ -1620,142 +1619,12 @@ void StatementSync::All(const FunctionCallbackInfo<Value>& args) {
16201619
args.GetReturnValue().Set(Array::New(isolate, rows.data(), rows.size()));
16211620
}
16221621

1623-
void StatementSync::IterateReturnCallback(
1624-
const FunctionCallbackInfo<Value>& args) {
1625-
Environment* env = Environment::GetCurrent(args);
1626-
auto isolate = env->isolate();
1627-
auto context = isolate->GetCurrentContext();
1628-
1629-
auto self = args.This();
1630-
// iterator has fetch all result or break, prevent next func to return result
1631-
if (self->Set(context, env->isfinished_string(), Boolean::New(isolate, true))
1632-
.IsNothing()) {
1633-
// An error will have been scheduled.
1634-
return;
1635-
}
1636-
1637-
Local<Value> val;
1638-
if (!self->Get(context, env->statement_string()).ToLocal(&val)) {
1639-
// An error will have been scheduled.
1640-
return;
1641-
}
1642-
auto external_stmt = Local<External>::Cast(val);
1643-
auto stmt = static_cast<StatementSync*>(external_stmt->Value());
1644-
if (!stmt->IsFinalized()) {
1645-
sqlite3_reset(stmt->statement_);
1646-
}
1647-
1648-
LocalVector<Name> keys(isolate, {env->done_string(), env->value_string()});
1649-
LocalVector<Value> values(isolate,
1650-
{Boolean::New(isolate, true), Null(isolate)});
1651-
1652-
DCHECK_EQ(keys.size(), values.size());
1653-
Local<Object> result = Object::New(
1654-
isolate, Null(isolate), keys.data(), values.data(), keys.size());
1655-
args.GetReturnValue().Set(result);
1656-
}
1657-
1658-
void StatementSync::IterateNextCallback(
1659-
const FunctionCallbackInfo<Value>& args) {
1660-
Environment* env = Environment::GetCurrent(args);
1661-
auto isolate = env->isolate();
1662-
auto context = isolate->GetCurrentContext();
1663-
1664-
auto self = args.This();
1665-
1666-
Local<Value> val;
1667-
if (!self->Get(context, env->isfinished_string()).ToLocal(&val)) {
1668-
// An error will have been scheduled.
1669-
return;
1670-
}
1671-
1672-
// skip iteration if is_finished
1673-
auto is_finished = Local<Boolean>::Cast(val);
1674-
if (is_finished->Value()) {
1675-
Local<Name> keys[] = {env->done_string(), env->value_string()};
1676-
Local<Value> values[] = {Boolean::New(isolate, true), Null(isolate)};
1677-
static_assert(arraysize(keys) == arraysize(values));
1678-
Local<Object> result = Object::New(
1679-
isolate, Null(isolate), &keys[0], &values[0], arraysize(keys));
1680-
args.GetReturnValue().Set(result);
1681-
return;
1682-
}
1683-
1684-
if (!self->Get(context, env->statement_string()).ToLocal(&val)) {
1685-
// An error will have been scheduled.
1686-
return;
1687-
}
1688-
1689-
auto external_stmt = Local<External>::Cast(val);
1690-
auto stmt = static_cast<StatementSync*>(external_stmt->Value());
1691-
1692-
if (!self->Get(context, env->num_cols_string()).ToLocal(&val)) {
1693-
// An error will have been scheduled.
1694-
return;
1695-
}
1696-
1697-
auto num_cols = Local<Integer>::Cast(val)->Value();
1698-
1699-
THROW_AND_RETURN_ON_BAD_STATE(
1700-
env, stmt->IsFinalized(), "statement has been finalized");
1701-
1702-
int r = sqlite3_step(stmt->statement_);
1703-
if (r != SQLITE_ROW) {
1704-
CHECK_ERROR_OR_THROW(
1705-
env->isolate(), stmt->db_.get(), r, SQLITE_DONE, void());
1706-
1707-
// cleanup when no more rows to fetch
1708-
sqlite3_reset(stmt->statement_);
1709-
if (self->Set(
1710-
context, env->isfinished_string(), Boolean::New(isolate, true))
1711-
.IsNothing()) {
1712-
// An error would have been scheduled
1713-
return;
1714-
}
1715-
1716-
LocalVector<Name> keys(isolate, {env->done_string(), env->value_string()});
1717-
LocalVector<Value> values(isolate,
1718-
{Boolean::New(isolate, true), Null(isolate)});
1719-
1720-
DCHECK_EQ(keys.size(), values.size());
1721-
Local<Object> result = Object::New(
1722-
isolate, Null(isolate), keys.data(), values.data(), keys.size());
1723-
args.GetReturnValue().Set(result);
1724-
return;
1725-
}
1726-
1727-
LocalVector<Name> row_keys(isolate);
1728-
row_keys.reserve(num_cols);
1729-
LocalVector<Value> row_values(isolate);
1730-
row_values.reserve(num_cols);
1731-
for (int i = 0; i < num_cols; ++i) {
1732-
Local<Name> key;
1733-
if (!stmt->ColumnNameToName(i).ToLocal(&key)) return;
1734-
Local<Value> val;
1735-
if (!stmt->ColumnToValue(i).ToLocal(&val)) return;
1736-
row_keys.emplace_back(key);
1737-
row_values.emplace_back(val);
1738-
}
1739-
1740-
Local<Object> row = Object::New(
1741-
isolate, Null(isolate), row_keys.data(), row_values.data(), num_cols);
1742-
1743-
LocalVector<Name> keys(isolate, {env->done_string(), env->value_string()});
1744-
LocalVector<Value> values(isolate, {Boolean::New(isolate, false), row});
1745-
1746-
DCHECK_EQ(keys.size(), values.size());
1747-
Local<Object> result = Object::New(
1748-
isolate, Null(isolate), keys.data(), values.data(), keys.size());
1749-
args.GetReturnValue().Set(result);
1750-
}
1751-
17521622
void StatementSync::Iterate(const FunctionCallbackInfo<Value>& args) {
17531623
StatementSync* stmt;
17541624
ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This());
17551625
Environment* env = Environment::GetCurrent(args);
17561626
THROW_AND_RETURN_ON_BAD_STATE(
17571627
env, stmt->IsFinalized(), "statement has been finalized");
1758-
auto isolate = env->isolate();
17591628
auto context = env->context();
17601629
int r = sqlite3_reset(stmt->statement_);
17611630
CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void());
@@ -1764,67 +1633,28 @@ void StatementSync::Iterate(const FunctionCallbackInfo<Value>& args) {
17641633
return;
17651634
}
17661635

1767-
Local<Function> next_func;
1768-
Local<Function> return_func;
1769-
if (!Function::New(context, StatementSync::IterateNextCallback)
1770-
.ToLocal(&next_func) ||
1771-
!Function::New(context, StatementSync::IterateReturnCallback)
1772-
.ToLocal(&return_func)) {
1773-
// An error will have been scheduled.
1774-
return;
1775-
}
1776-
1777-
Local<Name> keys[] = {env->next_string(), env->return_string()};
1778-
Local<Value> values[] = {next_func, return_func};
1779-
static_assert(arraysize(keys) == arraysize(values));
1780-
17811636
Local<Object> global = context->Global();
17821637
Local<Value> js_iterator;
17831638
Local<Value> js_iterator_prototype;
1784-
if (!global->Get(context, env->iterator_string()).ToLocal(&js_iterator))
1639+
if (!global->Get(context, env->iterator_string()).ToLocal(&js_iterator)) {
17851640
return;
1641+
}
17861642
if (!js_iterator.As<Object>()
17871643
->Get(context, env->prototype_string())
1788-
.ToLocal(&js_iterator_prototype))
1789-
return;
1790-
1791-
Local<Object> iterable_iterator = Object::New(
1792-
isolate, js_iterator_prototype, &keys[0], &values[0], arraysize(keys));
1793-
1794-
auto num_cols_pd = v8::PropertyDescriptor(
1795-
v8::Integer::New(isolate, sqlite3_column_count(stmt->statement_)), false);
1796-
num_cols_pd.set_enumerable(false);
1797-
num_cols_pd.set_configurable(false);
1798-
if (iterable_iterator
1799-
->DefineProperty(context, env->num_cols_string(), num_cols_pd)
1800-
.IsNothing()) {
1801-
// An error will have been scheduled.
1644+
.ToLocal(&js_iterator_prototype)) {
18021645
return;
18031646
}
18041647

1805-
auto stmt_pd =
1806-
v8::PropertyDescriptor(v8::External::New(isolate, stmt), false);
1807-
stmt_pd.set_enumerable(false);
1808-
stmt_pd.set_configurable(false);
1809-
if (iterable_iterator
1810-
->DefineProperty(context, env->statement_string(), stmt_pd)
1648+
BaseObjectPtr<StatementSyncIterator> iter =
1649+
StatementSyncIterator::Create(env, BaseObjectPtr<StatementSync>(stmt));
1650+
if (iter->object()
1651+
->GetPrototype()
1652+
.As<Object>()
1653+
->SetPrototype(context, js_iterator_prototype)
18111654
.IsNothing()) {
1812-
// An error will have been scheduled.
18131655
return;
18141656
}
1815-
1816-
auto is_finished_pd =
1817-
v8::PropertyDescriptor(v8::Boolean::New(isolate, false), true);
1818-
stmt_pd.set_enumerable(false);
1819-
stmt_pd.set_configurable(false);
1820-
if (iterable_iterator
1821-
->DefineProperty(context, env->isfinished_string(), is_finished_pd)
1822-
.IsNothing()) {
1823-
// An error will have been scheduled.
1824-
return;
1825-
}
1826-
1827-
args.GetReturnValue().Set(iterable_iterator);
1657+
args.GetReturnValue().Set(iter->object());
18281658
}
18291659

18301660
void StatementSync::Get(const FunctionCallbackInfo<Value>& args) {
@@ -2144,6 +1974,118 @@ BaseObjectPtr<StatementSync> StatementSync::Create(
21441974
return MakeBaseObject<StatementSync>(env, obj, std::move(db), stmt);
21451975
}
21461976

1977+
StatementSyncIterator::StatementSyncIterator(Environment* env,
1978+
Local<Object> object,
1979+
BaseObjectPtr<StatementSync> stmt)
1980+
: BaseObject(env, object), stmt_(std::move(stmt)) {
1981+
MakeWeak();
1982+
done_ = false;
1983+
}
1984+
1985+
StatementSyncIterator::~StatementSyncIterator() {}
1986+
void StatementSyncIterator::MemoryInfo(MemoryTracker* tracker) const {}
1987+
1988+
Local<FunctionTemplate> StatementSyncIterator::GetConstructorTemplate(
1989+
Environment* env) {
1990+
Local<FunctionTemplate> tmpl =
1991+
env->sqlite_statement_sync_iterator_constructor_template();
1992+
if (tmpl.IsEmpty()) {
1993+
Isolate* isolate = env->isolate();
1994+
tmpl = NewFunctionTemplate(isolate, IllegalConstructor);
1995+
tmpl->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "StatementSyncIterator"));
1996+
tmpl->InstanceTemplate()->SetInternalFieldCount(
1997+
StatementSync::kInternalFieldCount);
1998+
SetProtoMethod(isolate, tmpl, "next", StatementSyncIterator::Next);
1999+
SetProtoMethod(isolate, tmpl, "return", StatementSyncIterator::Return);
2000+
env->set_sqlite_statement_sync_iterator_constructor_template(tmpl);
2001+
}
2002+
return tmpl;
2003+
}
2004+
2005+
BaseObjectPtr<StatementSyncIterator> StatementSyncIterator::Create(
2006+
Environment* env, BaseObjectPtr<StatementSync> stmt) {
2007+
Local<Object> obj;
2008+
if (!GetConstructorTemplate(env)
2009+
->InstanceTemplate()
2010+
->NewInstance(env->context())
2011+
.ToLocal(&obj)) {
2012+
return BaseObjectPtr<StatementSyncIterator>();
2013+
}
2014+
2015+
return MakeBaseObject<StatementSyncIterator>(env, obj, std::move(stmt));
2016+
}
2017+
2018+
void StatementSyncIterator::Next(const FunctionCallbackInfo<Value>& args) {
2019+
StatementSyncIterator* iter;
2020+
ASSIGN_OR_RETURN_UNWRAP(&iter, args.This());
2021+
Environment* env = Environment::GetCurrent(args);
2022+
THROW_AND_RETURN_ON_BAD_STATE(
2023+
env, iter->stmt_->IsFinalized(), "statement has been finalized");
2024+
Isolate* isolate = env->isolate();
2025+
LocalVector<Name> keys(isolate, {env->done_string(), env->value_string()});
2026+
2027+
if (iter->done_) {
2028+
LocalVector<Value> values(isolate,
2029+
{Boolean::New(isolate, true), Null(isolate)});
2030+
Local<Object> result = Object::New(
2031+
isolate, Null(isolate), keys.data(), values.data(), keys.size());
2032+
args.GetReturnValue().Set(result);
2033+
return;
2034+
}
2035+
2036+
int r = sqlite3_step(iter->stmt_->statement_);
2037+
if (r != SQLITE_ROW) {
2038+
CHECK_ERROR_OR_THROW(
2039+
env->isolate(), iter->stmt_->db_.get(), r, SQLITE_DONE, void());
2040+
sqlite3_reset(iter->stmt_->statement_);
2041+
LocalVector<Value> values(isolate,
2042+
{Boolean::New(isolate, true), Null(isolate)});
2043+
Local<Object> result = Object::New(
2044+
isolate, Null(isolate), keys.data(), values.data(), keys.size());
2045+
args.GetReturnValue().Set(result);
2046+
return;
2047+
}
2048+
2049+
int num_cols = sqlite3_column_count(iter->stmt_->statement_);
2050+
LocalVector<Name> row_keys(isolate);
2051+
LocalVector<Value> row_values(isolate);
2052+
row_keys.reserve(num_cols);
2053+
row_values.reserve(num_cols);
2054+
for (int i = 0; i < num_cols; ++i) {
2055+
Local<Name> key;
2056+
if (!iter->stmt_->ColumnNameToName(i).ToLocal(&key)) return;
2057+
Local<Value> val;
2058+
if (!iter->stmt_->ColumnToValue(i).ToLocal(&val)) return;
2059+
row_keys.emplace_back(key);
2060+
row_values.emplace_back(val);
2061+
}
2062+
2063+
Local<Object> row = Object::New(
2064+
isolate, Null(isolate), row_keys.data(), row_values.data(), num_cols);
2065+
LocalVector<Value> values(isolate, {Boolean::New(isolate, false), row});
2066+
Local<Object> result = Object::New(
2067+
isolate, Null(isolate), keys.data(), values.data(), keys.size());
2068+
args.GetReturnValue().Set(result);
2069+
}
2070+
2071+
void StatementSyncIterator::Return(const FunctionCallbackInfo<Value>& args) {
2072+
StatementSyncIterator* iter;
2073+
ASSIGN_OR_RETURN_UNWRAP(&iter, args.This());
2074+
Environment* env = Environment::GetCurrent(args);
2075+
THROW_AND_RETURN_ON_BAD_STATE(
2076+
env, iter->stmt_->IsFinalized(), "statement has been finalized");
2077+
Isolate* isolate = env->isolate();
2078+
2079+
sqlite3_reset(iter->stmt_->statement_);
2080+
iter->done_ = true;
2081+
LocalVector<Name> keys(isolate, {env->done_string(), env->value_string()});
2082+
LocalVector<Value> values(isolate,
2083+
{Boolean::New(isolate, true), Null(isolate)});
2084+
Local<Object> result = Object::New(
2085+
isolate, Null(isolate), keys.data(), values.data(), keys.size());
2086+
args.GetReturnValue().Set(result);
2087+
}
2088+
21472089
Session::Session(Environment* env,
21482090
Local<Object> object,
21492091
BaseObjectWeakPtr<DatabaseSync> database,

‎src/node_sqlite.h‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,10 +145,29 @@ class StatementSync : public BaseObject {
145145
v8::MaybeLocal<v8::Value> ColumnToValue(const int column);
146146
v8::MaybeLocal<v8::Name> ColumnNameToName(const int column);
147147

148-
static void IterateNextCallback(
149-
const v8::FunctionCallbackInfo<v8::Value>& args);
150-
static void IterateReturnCallback(
151-
const v8::FunctionCallbackInfo<v8::Value>& args);
148+
friend class StatementSyncIterator;
149+
};
150+
151+
class StatementSyncIterator : public BaseObject {
152+
public:
153+
StatementSyncIterator(Environment* env,
154+
v8::Local<v8::Object> object,
155+
BaseObjectPtr<StatementSync> stmt);
156+
void MemoryInfo(MemoryTracker* tracker) const override;
157+
static v8::Local<v8::FunctionTemplate> GetConstructorTemplate(
158+
Environment* env);
159+
static BaseObjectPtr<StatementSyncIterator> Create(
160+
Environment* env, BaseObjectPtr<StatementSync> stmt);
161+
static void Next(const v8::FunctionCallbackInfo<v8::Value>& args);
162+
static void Return(const v8::FunctionCallbackInfo<v8::Value>& args);
163+
164+
SET_MEMORY_INFO_NAME(StatementSyncIterator)
165+
SET_SELF_SIZE(StatementSyncIterator)
166+
167+
private:
168+
~StatementSyncIterator() override;
169+
BaseObjectPtr<StatementSync> stmt_;
170+
bool done_;
152171
};
153172

154173
using Sqlite3ChangesetGenFunc = int (*)(sqlite3_session*, int*, void**);

0 commit comments

Comments
 (0)