Skip to content

Commit ee17943

Browse files
cjihrigaduh95
cjihrig
authored and
aduh95
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 813cc34 commit ee17943

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
@@ -440,6 +440,7 @@
440440
V(shutdown_wrap_template, v8::ObjectTemplate) \
441441
V(socketaddress_constructor_template, v8::FunctionTemplate) \
442442
V(sqlite_statement_sync_constructor_template, v8::FunctionTemplate) \
443+
V(sqlite_statement_sync_iterator_constructor_template, v8::FunctionTemplate) \
443444
V(sqlite_session_constructor_template, v8::FunctionTemplate) \
444445
V(streambaseentry_ctor_template, v8::FunctionTemplate) \
445446
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;
@@ -1630,142 +1629,12 @@ void StatementSync::All(const FunctionCallbackInfo<Value>& args) {
16301629
args.GetReturnValue().Set(Array::New(isolate, rows.data(), rows.size()));
16311630
}
16321631

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

1777-
Local<Function> next_func;
1778-
Local<Function> return_func;
1779-
if (!Function::New(context, StatementSync::IterateNextCallback)
1780-
.ToLocal(&next_func) ||
1781-
!Function::New(context, StatementSync::IterateReturnCallback)
1782-
.ToLocal(&return_func)) {
1783-
// An error will have been scheduled.
1784-
return;
1785-
}
1786-
1787-
Local<Name> keys[] = {env->next_string(), env->return_string()};
1788-
Local<Value> values[] = {next_func, return_func};
1789-
static_assert(arraysize(keys) == arraysize(values));
1790-
17911646
Local<Object> global = context->Global();
17921647
Local<Value> js_iterator;
17931648
Local<Value> js_iterator_prototype;
1794-
if (!global->Get(context, env->iterator_string()).ToLocal(&js_iterator))
1649+
if (!global->Get(context, env->iterator_string()).ToLocal(&js_iterator)) {
17951650
return;
1651+
}
17961652
if (!js_iterator.As<Object>()
17971653
->Get(context, env->prototype_string())
1798-
.ToLocal(&js_iterator_prototype))
1799-
return;
1800-
1801-
Local<Object> iterable_iterator = Object::New(
1802-
isolate, js_iterator_prototype, &keys[0], &values[0], arraysize(keys));
1803-
1804-
auto num_cols_pd = v8::PropertyDescriptor(
1805-
v8::Integer::New(isolate, sqlite3_column_count(stmt->statement_)), false);
1806-
num_cols_pd.set_enumerable(false);
1807-
num_cols_pd.set_configurable(false);
1808-
if (iterable_iterator
1809-
->DefineProperty(context, env->num_cols_string(), num_cols_pd)
1810-
.IsNothing()) {
1811-
// An error will have been scheduled.
1654+
.ToLocal(&js_iterator_prototype)) {
18121655
return;
18131656
}
18141657

1815-
auto stmt_pd =
1816-
v8::PropertyDescriptor(v8::External::New(isolate, stmt), false);
1817-
stmt_pd.set_enumerable(false);
1818-
stmt_pd.set_configurable(false);
1819-
if (iterable_iterator
1820-
->DefineProperty(context, env->statement_string(), stmt_pd)
1658+
BaseObjectPtr<StatementSyncIterator> iter =
1659+
StatementSyncIterator::Create(env, BaseObjectPtr<StatementSync>(stmt));
1660+
if (iter->object()
1661+
->GetPrototype()
1662+
.As<Object>()
1663+
->SetPrototype(context, js_iterator_prototype)
18211664
.IsNothing()) {
1822-
// An error will have been scheduled.
18231665
return;
18241666
}
1825-
1826-
auto is_finished_pd =
1827-
v8::PropertyDescriptor(v8::Boolean::New(isolate, false), true);
1828-
stmt_pd.set_enumerable(false);
1829-
stmt_pd.set_configurable(false);
1830-
if (iterable_iterator
1831-
->DefineProperty(context, env->isfinished_string(), is_finished_pd)
1832-
.IsNothing()) {
1833-
// An error will have been scheduled.
1834-
return;
1835-
}
1836-
1837-
args.GetReturnValue().Set(iterable_iterator);
1667+
args.GetReturnValue().Set(iter->object());
18381668
}
18391669

18401670
void StatementSync::Get(const FunctionCallbackInfo<Value>& args) {
@@ -2154,6 +1984,118 @@ BaseObjectPtr<StatementSync> StatementSync::Create(
21541984
return MakeBaseObject<StatementSync>(env, obj, std::move(db), stmt);
21551985
}
21561986

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

‎src/node_sqlite.h‎

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

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

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

0 commit comments

Comments
 (0)