Skip to content

Commit 2ec3329

Browse files
araujoguiaduh95
authored andcommitted
sqlite: use DeleteFnPtr for all SQLite handles
Replace the custom deleter structs and OnScopeLeave cleanups with DeleteFnPtr aliases, and pass session ownership to Session::Create. Signed-off-by: Guilherme Araújo <arauujogui@gmail.com> PR-URL: #66912 Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
1 parent 5bc98ec commit 2ec3329

2 files changed

Lines changed: 33 additions & 38 deletions

File tree

‎src/node_sqlite.cc‎

Lines changed: 18 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -743,9 +743,7 @@ class BackupJob : public ThreadPoolWork {
743743
}
744744

745745
void Cleanup() {
746-
if (backup_) {
747-
sqlite3_backup_finish(backup_.release());
748-
}
746+
backup_.reset();
749747

750748
if (dest_) {
751749
backup_status_ = sqlite3_errcode(dest_.get());
@@ -786,14 +784,8 @@ class BackupJob : public ThreadPoolWork {
786784
BaseObjectPtr<Database> source_;
787785
Global<Promise::Resolver> resolver_;
788786
Global<Function> progressFunc_;
789-
struct Sqlite3Deleter {
790-
void operator()(sqlite3* db) const { sqlite3_close_v2(db); }
791-
};
792-
struct BackupDeleter {
793-
void operator()(sqlite3_backup* b) const { sqlite3_backup_finish(b); }
794-
};
795-
std::unique_ptr<sqlite3, Sqlite3Deleter> dest_;
796-
std::unique_ptr<sqlite3_backup, BackupDeleter> backup_;
787+
ConnectionPtr dest_;
788+
BackupPtr backup_;
797789
int pages_;
798790
int backup_status_ = SQLITE_OK;
799791
std::string source_db_;
@@ -2101,6 +2093,8 @@ void Database::Close(const FunctionCallbackInfo<Value>& args) {
21012093
env, db->IsInCallback(), "database cannot be closed while in a callback");
21022094
db->FinalizeStatements();
21032095
db->DeleteSessions();
2096+
// Close through the raw handle instead of reset() so that a failed close
2097+
// keeps the handle, which the error below is read from.
21042098
int r = sqlite3_close_v2(db->connection_.get());
21052099
CHECK_ERROR_OR_THROW(env->isolate(), db, r, SQLITE_OK, void());
21062100
db->connection_.release();
@@ -2892,26 +2886,21 @@ void Database::CreateSession(const FunctionCallbackInfo<Value>& args) {
28922886
// getter, which may have closed the database since it was checked.
28932887
THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
28942888

2895-
sqlite3_session* pSession;
2896-
int r =
2897-
sqlite3session_create(db->connection_.get(), db_name.c_str(), &pSession);
2889+
sqlite3_session* raw_session;
2890+
int r = sqlite3session_create(
2891+
db->connection_.get(), db_name.c_str(), &raw_session);
28982892
CHECK_ERROR_OR_THROW(env->isolate(), db, r, SQLITE_OK, void());
2899-
bool wrapper_owns_session = false;
2900-
auto delete_session_on_failure = OnScopeLeave([&]() {
2901-
if (!wrapper_owns_session) {
2902-
sqlite3session_delete(pSession);
2903-
}
2904-
});
2893+
SessionPtr pSession(raw_session);
29052894

2906-
r = sqlite3session_attach(pSession, table == "" ? nullptr : table.c_str());
2895+
r = sqlite3session_attach(pSession.get(),
2896+
table == "" ? nullptr : table.c_str());
29072897
CHECK_ERROR_OR_THROW(env->isolate(), db, r, SQLITE_OK, void());
29082898

29092899
BaseObjectPtr<Session> session =
2910-
Session::Create(env, BaseObjectPtr<Database>(db), pSession);
2900+
Session::Create(env, BaseObjectPtr<Database>(db), std::move(pSession));
29112901
if (!session) {
29122902
return;
29132903
}
2914-
wrapper_owns_session = true;
29152904
args.GetReturnValue().Set(session->object());
29162905
}
29172906

@@ -4898,9 +4887,9 @@ void StatementIterator::Return(const FunctionCallbackInfo<Value>& args) {
48984887
Session::Session(Environment* env,
48994888
Local<Object> object,
49004889
BaseObjectPtr<Database> database,
4901-
sqlite3_session* session)
4890+
SessionPtr session)
49024891
: BaseObject(env, object),
4903-
session_(std::unique_ptr<sqlite3_session, SessionDeleter>(session)),
4892+
session_(std::move(session)),
49044893
database_(std::move(database)) {
49054894
database_->sessions_.insert(this);
49064895
MakeWeak();
@@ -4912,7 +4901,7 @@ Session::~Session() {
49124901

49134902
BaseObjectPtr<Session> Session::Create(Environment* env,
49144903
BaseObjectPtr<Database> database,
4915-
sqlite3_session* session) {
4904+
SessionPtr session) {
49164905
Local<Object> obj;
49174906
if (!GetConstructorTemplate(env)
49184907
->InstanceTemplate()
@@ -4921,7 +4910,8 @@ BaseObjectPtr<Session> Session::Create(Environment* env,
49214910
return nullptr;
49224911
}
49234912

4924-
return MakeBaseObject<Session>(env, obj, std::move(database), session);
4913+
return MakeBaseObject<Session>(
4914+
env, obj, std::move(database), std::move(session));
49254915
}
49264916

49274917
Local<FunctionTemplate> Session::GetConstructorTemplate(Environment* env) {
@@ -4970,7 +4960,7 @@ void Session::Changeset(const FunctionCallbackInfo<Value>& args) {
49704960
CHECK_ERROR_OR_THROW(
49714961
env->isolate(), session->database_.get(), r, SQLITE_OK, void());
49724962

4973-
auto freeChangeset = OnScopeLeave([&] { sqlite3_free(pChangeset); });
4963+
DeleteFnPtr<void, sqlite3_free> changeset(pChangeset);
49744964

49754965
Local<ArrayBuffer> buffer = ArrayBuffer::New(env->isolate(), nChangeset);
49764966
if (nChangeset > 0) {

‎src/node_sqlite.h‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,18 @@ inline void FinalizeStatement(sqlite3_stmt* stmt) {
173173
sqlite3_finalize(stmt);
174174
}
175175

176+
inline void CloseConnection(sqlite3* db) {
177+
sqlite3_close_v2(db);
178+
}
179+
180+
inline void FinishBackup(sqlite3_backup* backup) {
181+
sqlite3_backup_finish(backup);
182+
}
183+
176184
using StatementPtr = DeleteFnPtr<sqlite3_stmt, FinalizeStatement>;
185+
using ConnectionPtr = DeleteFnPtr<sqlite3, CloseConnection>;
186+
using BackupPtr = DeleteFnPtr<sqlite3_backup, FinishBackup>;
187+
using SessionPtr = DeleteFnPtr<sqlite3_session, sqlite3session_delete>;
177188

178189
class StatementExecutionHelper {
179190
public:
@@ -338,10 +349,7 @@ class Database : public BaseObject {
338349
DatabaseOpenConfiguration open_config_;
339350
bool allow_load_extension_;
340351
bool enable_load_extension_;
341-
struct ConnectionDeleter {
342-
void operator()(sqlite3* db) const { sqlite3_close_v2(db); }
343-
};
344-
std::unique_ptr<sqlite3, ConnectionDeleter> connection_;
352+
ConnectionPtr connection_;
345353
bool ignore_next_sqlite_error_;
346354
int callback_depth_ = 0;
347355
int destructor_depth_ = 0;
@@ -465,7 +473,7 @@ class Session : public BaseObject {
465473
Session(Environment* env,
466474
v8::Local<v8::Object> object,
467475
BaseObjectPtr<Database> database,
468-
sqlite3_session* session);
476+
SessionPtr session);
469477
~Session() override;
470478
template <Sqlite3ChangesetGenFunc sqliteChangesetFunc>
471479
static void Changeset(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -475,18 +483,15 @@ class Session : public BaseObject {
475483
Environment* env);
476484
static BaseObjectPtr<Session> Create(Environment* env,
477485
BaseObjectPtr<Database> database,
478-
sqlite3_session* session);
486+
SessionPtr session);
479487

480488
void MemoryInfo(MemoryTracker* tracker) const override;
481489
SET_MEMORY_INFO_NAME(Session)
482490
SET_SELF_SIZE(Session)
483491

484492
private:
485493
void Delete();
486-
struct SessionDeleter {
487-
void operator()(sqlite3_session* s) const { sqlite3session_delete(s); }
488-
};
489-
std::unique_ptr<sqlite3_session, SessionDeleter> session_;
494+
SessionPtr session_;
490495
BaseObjectPtr<Database> database_; // The Parent Database
491496
bool is_generating_changeset_ = false;
492497

0 commit comments

Comments
 (0)