FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

sqlite: ensure statement finalization on db close · nodejs/node@94e2ea6 · GitHub

/ node Public

Commit 94e2ea6

Browse files
cjihrig
authored and
targos
committed
sqlite: ensure statement finalization on db close
This commit adds statement tracking to the DatabaseSync class. When a database is closed manually or via garbage collection, it will force all associated prepared statements to be finalized. This should mitigate "zombie" connections which can introduce test flakiness in the CI on Windows. PR-URL: #54014 Fixes: #54006 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
1 parent 51b8576 commit 94e2ea6

2 files changed

Lines changed: 92 additions & 26 deletions

File tree

‎src/node_sqlite.cc‎

Lines changed: 79 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -91,16 +91,19 @@ DatabaseSync::DatabaseSync(Environment* env,
9191
}
9292

9393
DatabaseSync::~DatabaseSync() {
94-
sqlite3_close_v2(connection_);
95-
connection_ = nullptr;
94+
if (IsOpen()) {
95+
FinalizeStatements();
96+
sqlite3_close_v2(connection_);
97+
connection_ = nullptr;
98+
}
9699
}
97100

98101
void DatabaseSync::MemoryInfo(MemoryTracker* tracker) const {
99102
tracker->TrackField("location", location_);
100103
}
101104

102105
bool DatabaseSync::Open() {
103-
if (connection_ != nullptr) {
106+
if (IsOpen()) {
104107
node::THROW_ERR_INVALID_STATE(env(), "database is already open");
105108
return false;
106109
}
@@ -112,6 +115,29 @@ bool DatabaseSync::Open() {
112115
return true;
113116
}
114117

118+
void DatabaseSync::FinalizeStatements() {
119+
for (auto stmt : statements_) {
120+
stmt->Finalize();
121+
}
122+
123+
statements_.clear();
124+
}
125+
126+
void DatabaseSync::UntrackStatement(StatementSync* statement) {
127+
auto it = statements_.find(statement);
128+
if (it != statements_.end()) {
129+
statements_.erase(it);
130+
}
131+
}
132+
133+
inline bool DatabaseSync::IsOpen() {
134+
return connection_ != nullptr;
135+
}
136+
137+
inline sqlite3* DatabaseSync::Connection() {
138+
return connection_;
139+
}
140+
115141
void DatabaseSync::New(const FunctionCallbackInfo<Value>& args) {
116142
Environment* env = Environment::GetCurrent(args);
117143

@@ -164,8 +190,8 @@ void DatabaseSync::Close(const FunctionCallbackInfo<Value>& args) {
164190
DatabaseSync* db;
165191
ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
166192
Environment* env = Environment::GetCurrent(args);
167-
THROW_AND_RETURN_ON_BAD_STATE(
168-
env, db->connection_ == nullptr, "database is not open");
193+
THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
194+
db->FinalizeStatements();
169195
int r = sqlite3_close_v2(db->connection_);
170196
CHECK_ERROR_OR_THROW(env->isolate(), db->connection_, r, SQLITE_OK, void());
171197
db->connection_ = nullptr;
@@ -175,8 +201,7 @@ void DatabaseSync::Prepare(const FunctionCallbackInfo<Value>& args) {
175201
DatabaseSync* db;
176202
ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
177203
Environment* env = Environment::GetCurrent(args);
178-
THROW_AND_RETURN_ON_BAD_STATE(
179-
env, db->connection_ == nullptr, "database is not open");
204+
THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
180205

181206
if (!args[0]->IsString()) {
182207
node::THROW_ERR_INVALID_ARG_TYPE(env->isolate(),
@@ -188,17 +213,16 @@ void DatabaseSync::Prepare(const FunctionCallbackInfo<Value>& args) {
188213
sqlite3_stmt* s = nullptr;
189214
int r = sqlite3_prepare_v2(db->connection_, *sql, -1, &s, 0);
190215
CHECK_ERROR_OR_THROW(env->isolate(), db->connection_, r, SQLITE_OK, void());
191-
BaseObjectPtr<StatementSync> stmt =
192-
StatementSync::Create(env, db->connection_, s);
216+
BaseObjectPtr<StatementSync> stmt = StatementSync::Create(env, db, s);
217+
db->statements_.insert(stmt.get());
193218
args.GetReturnValue().Set(stmt->object());
194219
}
195220

196221
void DatabaseSync::Exec(const FunctionCallbackInfo<Value>& args) {
197222
DatabaseSync* db;
198223
ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
199224
Environment* env = Environment::GetCurrent(args);
200-
THROW_AND_RETURN_ON_BAD_STATE(
201-
env, db->connection_ == nullptr, "database is not open");
225+
THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
202226

203227
if (!args[0]->IsString()) {
204228
node::THROW_ERR_INVALID_ARG_TYPE(env->isolate(),
@@ -213,7 +237,7 @@ void DatabaseSync::Exec(const FunctionCallbackInfo<Value>& args) {
213237

214238
StatementSync::StatementSync(Environment* env,
215239
Local<Object> object,
216-
sqlite3* db,
240+
DatabaseSync* db,
217241
sqlite3_stmt* stmt)
218242
: BaseObject(env, object) {
219243
MakeWeak();
@@ -227,13 +251,25 @@ StatementSync::StatementSync(Environment* env,
227251
}
228252

229253
StatementSync::~StatementSync() {
254+
if (!IsFinalized()) {
255+
db_->UntrackStatement(this);
256+
Finalize();
257+
}
258+
}
259+
260+
void StatementSync::Finalize() {
230261
sqlite3_finalize(statement_);
231262
statement_ = nullptr;
232263
}
233264

265+
inline bool StatementSync::IsFinalized() {
266+
return statement_ == nullptr;
267+
}
268+
234269
bool StatementSync::BindParams(const FunctionCallbackInfo<Value>& args) {
235270
int r = sqlite3_clear_bindings(statement_);
236-
CHECK_ERROR_OR_THROW(env()->isolate(), db_, r, SQLITE_OK, false);
271+
CHECK_ERROR_OR_THROW(
272+
env()->isolate(), db_->Connection(), r, SQLITE_OK, false);
237273

238274
int anon_idx = 1;
239275
int anon_start = 0;
@@ -364,7 +400,8 @@ bool StatementSync::BindValue(const Local<Value>& value, const int index) {
364400
return false;
365401
}
366402

367-
CHECK_ERROR_OR_THROW(env()->isolate(), db_, r, SQLITE_OK, false);
403+
CHECK_ERROR_OR_THROW(
404+
env()->isolate(), db_->Connection(), r, SQLITE_OK, false);
368405
return true;
369406
}
370407

@@ -435,8 +472,11 @@ void StatementSync::All(const FunctionCallbackInfo<Value>& args) {
435472
StatementSync* stmt;
436473
ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This());
437474
Environment* env = Environment::GetCurrent(args);
475+
THROW_AND_RETURN_ON_BAD_STATE(
476+
env, stmt->IsFinalized(), "statement has been finalized");
438477
int r = sqlite3_reset(stmt->statement_);
439-
CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_, r, SQLITE_OK, void());
478+
CHECK_ERROR_OR_THROW(
479+
env->isolate(), stmt->db_->Connection(), r, SQLITE_OK, void());
440480

441481
if (!stmt->BindParams(args)) {
442482
return;
@@ -462,7 +502,8 @@ void StatementSync::All(const FunctionCallbackInfo<Value>& args) {
462502
rows.emplace_back(row);
463503
}
464504

465-
CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_, r, SQLITE_DONE, void());
505+
CHECK_ERROR_OR_THROW(
506+
env->isolate(), stmt->db_->Connection(), r, SQLITE_DONE, void());
466507
args.GetReturnValue().Set(
467508
Array::New(env->isolate(), rows.data(), rows.size()));
468509
}
@@ -471,8 +512,11 @@ void StatementSync::Get(const FunctionCallbackInfo<Value>& args) {
471512
StatementSync* stmt;
472513
ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This());
473514
Environment* env = Environment::GetCurrent(args);
515+
THROW_AND_RETURN_ON_BAD_STATE(
516+
env, stmt->IsFinalized(), "statement has been finalized");
474517
int r = sqlite3_reset(stmt->statement_);
475-
CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_, r, SQLITE_OK, void());
518+
CHECK_ERROR_OR_THROW(
519+
env->isolate(), stmt->db_->Connection(), r, SQLITE_OK, void());
476520

477521
if (!stmt->BindParams(args)) {
478522
return;
@@ -482,7 +526,7 @@ void StatementSync::Get(const FunctionCallbackInfo<Value>& args) {
482526
r = sqlite3_step(stmt->statement_);
483527
if (r == SQLITE_DONE) return;
484528
if (r != SQLITE_ROW) {
485-
THROW_ERR_SQLITE_ERROR(env->isolate(), stmt->db_);
529+
THROW_ERR_SQLITE_ERROR(env->isolate(), stmt->db_->Connection());
486530
return;
487531
}
488532

@@ -511,8 +555,11 @@ void StatementSync::Run(const FunctionCallbackInfo<Value>& args) {
511555
StatementSync* stmt;
512556
ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This());
513557
Environment* env = Environment::GetCurrent(args);
558+
THROW_AND_RETURN_ON_BAD_STATE(
559+
env, stmt->IsFinalized(), "statement has been finalized");
514560
int r = sqlite3_reset(stmt->statement_);
515-
CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_, r, SQLITE_OK, void());
561+
CHECK_ERROR_OR_THROW(
562+
env->isolate(), stmt->db_->Connection(), r, SQLITE_OK, void());
516563

517564
if (!stmt->BindParams(args)) {
518565
return;
@@ -521,7 +568,7 @@ void StatementSync::Run(const FunctionCallbackInfo<Value>& args) {
521568
auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt->statement_); });
522569
r = sqlite3_step(stmt->statement_);
523570
if (r != SQLITE_ROW && r != SQLITE_DONE) {
524-
THROW_ERR_SQLITE_ERROR(env->isolate(), stmt->db_);
571+
THROW_ERR_SQLITE_ERROR(env->isolate(), stmt->db_->Connection());
525572
return;
526573
}
527574

@@ -530,8 +577,9 @@ void StatementSync::Run(const FunctionCallbackInfo<Value>& args) {
530577
FIXED_ONE_BYTE_STRING(env->isolate(), "lastInsertRowid");
531578
Local<String> changes_string =
532579
FIXED_ONE_BYTE_STRING(env->isolate(), "changes");
533-
sqlite3_int64 last_insert_rowid = sqlite3_last_insert_rowid(stmt->db_);
534-
sqlite3_int64 changes = sqlite3_changes64(stmt->db_);
580+
sqlite3_int64 last_insert_rowid =
581+
sqlite3_last_insert_rowid(stmt->db_->Connection());
582+
sqlite3_int64 changes = sqlite3_changes64(stmt->db_->Connection());
535583
Local<Value> last_insert_rowid_val;
536584
Local<Value> changes_val;
537585

@@ -557,6 +605,8 @@ void StatementSync::SourceSQL(const FunctionCallbackInfo<Value>& args) {
557605
StatementSync* stmt;
558606
ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This());
559607
Environment* env = Environment::GetCurrent(args);
608+
THROW_AND_RETURN_ON_BAD_STATE(
609+
env, stmt->IsFinalized(), "statement has been finalized");
560610
Local<String> sql;
561611
if (!String::NewFromUtf8(env->isolate(), sqlite3_sql(stmt->statement_))
562612
.ToLocal(&sql)) {
@@ -569,6 +619,8 @@ void StatementSync::ExpandedSQL(const FunctionCallbackInfo<Value>& args) {
569619
StatementSync* stmt;
570620
ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This());
571621
Environment* env = Environment::GetCurrent(args);
622+
THROW_AND_RETURN_ON_BAD_STATE(
623+
env, stmt->IsFinalized(), "statement has been finalized");
572624
char* expanded = sqlite3_expanded_sql(stmt->statement_);
573625
auto maybe_expanded = String::NewFromUtf8(env->isolate(), expanded);
574626
sqlite3_free(expanded);
@@ -584,6 +636,8 @@ void StatementSync::SetAllowBareNamedParameters(
584636
StatementSync* stmt;
585637
ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This());
586638
Environment* env = Environment::GetCurrent(args);
639+
THROW_AND_RETURN_ON_BAD_STATE(
640+
env, stmt->IsFinalized(), "statement has been finalized");
587641

588642
if (!args[0]->IsBoolean()) {
589643
node::THROW_ERR_INVALID_ARG_TYPE(
@@ -599,6 +653,8 @@ void StatementSync::SetReadBigInts(const FunctionCallbackInfo<Value>& args) {
599653
StatementSync* stmt;
600654
ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This());
601655
Environment* env = Environment::GetCurrent(args);
656+
THROW_AND_RETURN_ON_BAD_STATE(
657+
env, stmt->IsFinalized(), "statement has been finalized");
602658

603659
if (!args[0]->IsBoolean()) {
604660
node::THROW_ERR_INVALID_ARG_TYPE(
@@ -640,7 +696,7 @@ Local<FunctionTemplate> StatementSync::GetConstructorTemplate(
640696
}
641697

642698
BaseObjectPtr<StatementSync> StatementSync::Create(Environment* env,
643-
sqlite3* db,
699+
DatabaseSync* db,
644700
sqlite3_stmt* stmt) {
645701
Local<Object> obj;
646702
if (!GetConstructorTemplate(env)

‎src/node_sqlite.h‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,13 @@
99
#include "util.h"
1010

1111
#include <map>
12+
#include <unordered_set>
1213

1314
namespace node {
1415
namespace sqlite {
1516

17+
class StatementSync;
18+
1619
class DatabaseSync : public BaseObject {
1720
public:
1821
DatabaseSync(Environment* env,
@@ -25,6 +28,10 @@ class DatabaseSync : public BaseObject {
2528
static void Close(const v8::FunctionCallbackInfo<v8::Value>& args);
2629
static void Prepare(const v8::FunctionCallbackInfo<v8::Value>& args);
2730
static void Exec(const v8::FunctionCallbackInfo<v8::Value>& args);
31+
void FinalizeStatements();
32+
void UntrackStatement(StatementSync* statement);
33+
bool IsOpen();
34+
sqlite3* Connection();
2835

2936
SET_MEMORY_INFO_NAME(DatabaseSync)
3037
SET_SELF_SIZE(DatabaseSync)
@@ -35,19 +42,20 @@ class DatabaseSync : public BaseObject {
3542
~DatabaseSync() override;
3643
std::string location_;
3744
sqlite3* connection_;
45+
std::unordered_set<StatementSync*> statements_;
3846
};
3947

4048
class StatementSync : public BaseObject {
4149
public:
4250
StatementSync(Environment* env,
4351
v8::Local<v8::Object> object,
44-
sqlite3* db,
52+
DatabaseSync* db,
4553
sqlite3_stmt* stmt);
4654
void MemoryInfo(MemoryTracker* tracker) const override;
4755
static v8::Local<v8::FunctionTemplate> GetConstructorTemplate(
4856
Environment* env);
4957
static BaseObjectPtr<StatementSync> Create(Environment* env,
50-
sqlite3* db,
58+
DatabaseSync* db,
5159
sqlite3_stmt* stmt);
5260
static void All(const v8::FunctionCallbackInfo<v8::Value>& args);
5361
static void Get(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -57,13 +65,15 @@ class StatementSync : public BaseObject {
5765
static void SetAllowBareNamedParameters(
5866
const v8::FunctionCallbackInfo<v8::Value>& args);
5967
static void SetReadBigInts(const v8::FunctionCallbackInfo<v8::Value>& args);
68+
void Finalize();
69+
bool IsFinalized();
6070

6171
SET_MEMORY_INFO_NAME(StatementSync)
6272
SET_SELF_SIZE(StatementSync)
6373

6474
private:
6575
~StatementSync() override;
66-
sqlite3* db_;
76+
DatabaseSync* db_;
6777
sqlite3_stmt* statement_;
6878
bool use_big_ints_;
6979
bool allow_bare_named_params_;

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL