From e87dfa474dd89e20fd9361e7cf845aa88ae2ec1b Mon Sep 17 00:00:00 2001 From: Mohamed Akram Date: Sun, 6 Sep 2020 17:36:53 +0400 Subject: [PATCH] Fix segfaults (#1368) * Ensure JavaScript exceptions bubble up * Fix segfault * Fix mutex segfault * Fix segfault caused by memory leak If the database was being closed, and non-exclusive work was scheduled, it overrode the lock flag such that the state became open=false locked=false instead of open=false locked=true. This caused queued work to not be processed, leaking memory, which causes a segfault during napi cleanup. Make the same changes to other methods for safe measure. --- src/backup.cc | 29 +++++++--------- src/backup.h | 3 +- src/database.cc | 89 ++++++++++++++++++++++-------------------------- src/database.h | 3 +- src/macros.h | 21 +++++++----- src/statement.cc | 75 ++++++++++++++++++++-------------------- src/statement.h | 3 +- 7 files changed, 109 insertions(+), 114 deletions(-) diff --git a/src/backup.cc b/src/backup.cc index 934379b82..78570cddb 100644 --- a/src/backup.cc +++ b/src/backup.cc @@ -31,11 +31,10 @@ void Backup::Process() { } while (inited && !locked && !queue.empty()) { - Call* call = queue.front(); + std::unique_ptr call(queue.front()); queue.pop(); call->callback(call->baton); - delete call; } } @@ -86,21 +85,17 @@ void Backup::CleanQueue() { // Clear out the queue so that this object can get GC'ed. while (!queue.empty()) { - Call* call = queue.front(); + std::unique_ptr call(queue.front()); queue.pop(); - Napi::Function cb = call->baton->callback.Value(); + std::unique_ptr baton(call->baton); + Napi::Function cb = baton->callback.Value(); if (inited && !cb.IsEmpty() && cb.IsFunction()) { TRY_CATCH_CALL(Value(), cb, 1, argv); called = true; } - - // We don't call the actual callback, so we have to make sure that - // the baton gets destroyed. - delete call->baton; - delete call; } // When we couldn't call a callback function, emit an error on the @@ -113,13 +108,12 @@ void Backup::CleanQueue() { else while (!queue.empty()) { // Just delete all items in the queue; we already fired an event when // initializing the backup failed. - Call* call = queue.front(); + std::unique_ptr call(queue.front()); queue.pop(); // We don't call the actual callback, so we have to make sure that // the baton gets destroyed. delete call->baton; - delete call; } } @@ -220,13 +214,14 @@ void Backup::Work_Initialize(napi_env e, void* data) { } void Backup::Work_AfterInitialize(napi_env e, napi_status status, void* data) { - BACKUP_INIT(InitializeBaton); + std::unique_ptr baton(static_cast(data)); + Backup* backup = baton->backup; Napi::Env env = backup->Env(); Napi::HandleScope scope(env); if (backup->status != SQLITE_OK) { - Error(baton); + Error(baton.get()); backup->FinishAll(); } else { @@ -282,7 +277,8 @@ void Backup::Work_Step(napi_env e, void* data) { } void Backup::Work_AfterStep(napi_env e, napi_status status, void* data) { - BACKUP_INIT(StepBaton); + std::unique_ptr baton(static_cast(data)); + Backup* backup = baton->backup; Napi::Env env = backup->Env(); Napi::HandleScope scope(env); @@ -294,7 +290,7 @@ void Backup::Work_AfterStep(napi_env e, napi_status status, void* data) { } if (backup->status != SQLITE_OK && backup->status != SQLITE_DONE) { - Error(baton); + Error(baton.get()); } else { // Fire callbacks. @@ -329,7 +325,8 @@ void Backup::Work_Finish(napi_env e, void* data) { } void Backup::Work_AfterFinish(napi_env e, napi_status status, void* data) { - BACKUP_INIT(Baton); + std::unique_ptr baton(static_cast(data)); + Backup* backup = baton->backup; Napi::Env env = backup->Env(); Napi::HandleScope scope(env); diff --git a/src/backup.h b/src/backup.h index c44e02b89..c15b77bfe 100644 --- a/src/backup.h +++ b/src/backup.h @@ -96,7 +96,7 @@ class Backup : public Napi::ObjectWrap { static Napi::Object Init(Napi::Env env, Napi::Object exports); struct Baton { - napi_async_work request; + napi_async_work request = NULL; Backup* backup; Napi::FunctionReference callback; @@ -105,6 +105,7 @@ class Backup : public Napi::ObjectWrap { callback.Reset(cb_, 1); } virtual ~Baton() { + if (request) napi_delete_async_work(backup->Env(), request); backup->Unref(); callback.Reset(); } diff --git a/src/database.cc b/src/database.cc index c3507b232..c8d125cb9 100644 --- a/src/database.cc +++ b/src/database.cc @@ -50,17 +50,14 @@ void Database::Process() { // Call all callbacks with the error object. while (!queue.empty()) { - Call* call = queue.front(); - Napi::Function cb = call->baton->callback.Value(); + std::unique_ptr call(queue.front()); + queue.pop(); + std::unique_ptr baton(call->baton); + Napi::Function cb = baton->callback.Value(); if (!cb.IsUndefined() && cb.IsFunction()) { TRY_CATCH_CALL(this->Value(), cb, 1, argv); called = true; } - queue.pop(); - // We don't call the actual callback, so we have to make sure that - // the baton gets destroyed. - delete call->baton; - delete call; } // When we couldn't call a callback function, emit an error on the @@ -73,16 +70,16 @@ void Database::Process() { } while (open && (!locked || pending == 0) && !queue.empty()) { - Call* call = queue.front(); + Call *c = queue.front(); - if (call->exclusive && pending > 0) { + if (c->exclusive && pending > 0) { break; } queue.pop(); + std::unique_ptr call(c); locked = call->exclusive; call->callback(call->baton); - delete call; if (locked) break; } @@ -95,6 +92,9 @@ void Database::Schedule(Work_Callback callback, Baton* baton, bool exclusive) { if (!open && locked) { EXCEPTION(Napi::String::New(env, "Database is closed"), SQLITE_MISUSE, exception); Napi::Function cb = baton->callback.Value(); + // We don't call the actual callback, so we have to make sure that + // the baton gets destroyed. + delete baton; if (!cb.IsUndefined() && cb.IsFunction()) { Napi::Value argv[] = { exception }; TRY_CATCH_CALL(Value(), cb, 1, argv); @@ -181,7 +181,7 @@ void Database::Work_Open(napi_env e, void* data) { } void Database::Work_AfterOpen(napi_env e, napi_status status, void* data) { - OpenBaton* baton = static_cast(data); + std::unique_ptr baton(static_cast(data)); Database* db = baton->db; @@ -213,9 +213,6 @@ void Database::Work_AfterOpen(napi_env e, napi_status status, void* data) { EMIT_EVENT(db->Value(), 1, info); db->Process(); } - - napi_delete_async_work(e, baton->request); - delete baton; } Napi::Value Database::OpenGetter(const Napi::CallbackInfo& info) { @@ -241,6 +238,7 @@ void Database::Work_BeginClose(Baton* baton) { assert(baton->db->_handle); assert(baton->db->pending == 0); + baton->db->pending++; baton->db->RemoveCallbacks(); baton->db->closing = true; @@ -269,13 +267,14 @@ void Database::Work_Close(napi_env e, void* data) { } void Database::Work_AfterClose(napi_env e, napi_status status, void* data) { - Baton* baton = static_cast(data); + std::unique_ptr baton(static_cast(data)); Database* db = baton->db; Napi::Env env = db->Env(); Napi::HandleScope scope(env); + db->pending--; db->closing = false; Napi::Value argv[1]; @@ -306,9 +305,6 @@ void Database::Work_AfterClose(napi_env e, napi_status status, void* data) { EMIT_EVENT(db->Value(), 1, info); db->Process(); } - - napi_delete_async_work(e, baton->request); - delete baton; } Napi::Value Database::Serialize(const Napi::CallbackInfo& info) { @@ -320,7 +316,7 @@ Napi::Value Database::Serialize(const Napi::CallbackInfo& info) { db->serialize = true; if (!callback.IsEmpty() && callback.IsFunction()) { - TRY_CATCH_CALL(info.This(), callback, 0, NULL); + TRY_CATCH_CALL(info.This(), callback, 0, NULL, info.This()); db->serialize = before; } @@ -338,7 +334,7 @@ Napi::Value Database::Parallelize(const Napi::CallbackInfo& info) { db->serialize = false; if (!callback.IsEmpty() && callback.IsFunction()) { - TRY_CATCH_CALL(info.This(), callback, 0, NULL); + TRY_CATCH_CALL(info.This(), callback, 0, NULL, info.This()); db->serialize = before; } @@ -407,17 +403,18 @@ Napi::Value Database::Interrupt(const Napi::CallbackInfo& info) { return info.This(); } -void Database::SetBusyTimeout(Baton* baton) { +void Database::SetBusyTimeout(Baton* b) { + std::unique_ptr baton(b); + assert(baton->db->open); assert(baton->db->_handle); // Abuse the status field for passing the timeout. sqlite3_busy_timeout(baton->db->_handle, baton->status); - - delete baton; } -void Database::RegisterTraceCallback(Baton* baton) { +void Database::RegisterTraceCallback(Baton* b) { + std::unique_ptr baton(b); assert(baton->db->open); assert(baton->db->_handle); Database* db = baton->db; @@ -433,8 +430,6 @@ void Database::RegisterTraceCallback(Baton* baton) { db->debug_trace->finish(); db->debug_trace = NULL; } - - delete baton; } void Database::TraceCallback(void* db, const char* sql) { @@ -443,7 +438,8 @@ void Database::TraceCallback(void* db, const char* sql) { static_cast(db)->debug_trace->send(new std::string(sql)); } -void Database::TraceCallback(Database* db, std::string* sql) { +void Database::TraceCallback(Database* db, std::string* s) { + std::unique_ptr sql(s); // Note: This function is called in the main V8 thread. Napi::Env env = db->Env(); Napi::HandleScope scope(env); @@ -453,10 +449,10 @@ void Database::TraceCallback(Database* db, std::string* sql) { Napi::String::New(env, sql->c_str()) }; EMIT_EVENT(db->Value(), 2, argv); - delete sql; } -void Database::RegisterProfileCallback(Baton* baton) { +void Database::RegisterProfileCallback(Baton* b) { + std::unique_ptr baton(b); assert(baton->db->open); assert(baton->db->_handle); Database* db = baton->db; @@ -472,8 +468,6 @@ void Database::RegisterProfileCallback(Baton* baton) { db->debug_profile->finish(); db->debug_profile = NULL; } - - delete baton; } void Database::ProfileCallback(void* db, const char* sql, sqlite3_uint64 nsecs) { @@ -485,7 +479,8 @@ void Database::ProfileCallback(void* db, const char* sql, sqlite3_uint64 nsecs) static_cast(db)->debug_profile->send(info); } -void Database::ProfileCallback(Database *db, ProfileInfo* info) { +void Database::ProfileCallback(Database *db, ProfileInfo* i) { + std::unique_ptr info(i); Napi::Env env = db->Env(); Napi::HandleScope scope(env); @@ -495,10 +490,10 @@ void Database::ProfileCallback(Database *db, ProfileInfo* info) { Napi::Number::New(env, (double)info->nsecs / 1000000.0) }; EMIT_EVENT(db->Value(), 3, argv); - delete info; } -void Database::RegisterUpdateCallback(Baton* baton) { +void Database::RegisterUpdateCallback(Baton* b) { + std::unique_ptr baton(b); assert(baton->db->open); assert(baton->db->_handle); Database* db = baton->db; @@ -514,8 +509,6 @@ void Database::RegisterUpdateCallback(Baton* baton) { db->update_event->finish(); db->update_event = NULL; } - - delete baton; } void Database::UpdateCallback(void* db, int type, const char* database, @@ -530,7 +523,8 @@ void Database::UpdateCallback(void* db, int type, const char* database, static_cast(db)->update_event->send(info); } -void Database::UpdateCallback(Database *db, UpdateInfo* info) { +void Database::UpdateCallback(Database *db, UpdateInfo* i) { + std::unique_ptr info(i); Napi::Env env = db->Env(); Napi::HandleScope scope(env); @@ -541,7 +535,6 @@ void Database::UpdateCallback(Database *db, UpdateInfo* info) { Napi::Number::New(env, info->rowid), }; EMIT_EVENT(db->Value(), 4, argv); - delete info; } Napi::Value Database::Exec(const Napi::CallbackInfo& info) { @@ -562,6 +555,7 @@ void Database::Work_BeginExec(Baton* baton) { assert(baton->db->open); assert(baton->db->_handle); assert(baton->db->pending == 0); + baton->db->pending++; Napi::Env env = baton->db->Env(); int status = napi_create_async_work( env, NULL, Napi::String::New(env, "sqlite3.Database.Exec"), @@ -590,9 +584,10 @@ void Database::Work_Exec(napi_env e, void* data) { } void Database::Work_AfterExec(napi_env e, napi_status status, void* data) { - ExecBaton* baton = static_cast(data); + std::unique_ptr baton(static_cast(data)); Database* db = baton->db; + db->pending--; Napi::Env env = db->Env(); Napi::HandleScope scope(env); @@ -617,9 +612,6 @@ void Database::Work_AfterExec(napi_env e, napi_status status, void* data) { } db->Process(); - - napi_delete_async_work(e, baton->request); - delete baton; } Napi::Value Database::Wait(const Napi::CallbackInfo& info) { @@ -634,7 +626,9 @@ Napi::Value Database::Wait(const Napi::CallbackInfo& info) { return info.This(); } -void Database::Work_Wait(Baton* baton) { +void Database::Work_Wait(Baton* b) { + std::unique_ptr baton(b); + Napi::Env env = baton->db->Env(); Napi::HandleScope scope(env); @@ -650,8 +644,6 @@ void Database::Work_Wait(Baton* baton) { } baton->db->Process(); - - delete baton; } Napi::Value Database::LoadExtension(const Napi::CallbackInfo& info) { @@ -672,6 +664,7 @@ void Database::Work_BeginLoadExtension(Baton* baton) { assert(baton->db->open); assert(baton->db->_handle); assert(baton->db->pending == 0); + baton->db->pending++; Napi::Env env = baton->db->Env(); int status = napi_create_async_work( env, NULL, Napi::String::New(env, "sqlite3.Database.LoadExtension"), @@ -703,9 +696,10 @@ void Database::Work_LoadExtension(napi_env e, void* data) { } void Database::Work_AfterLoadExtension(napi_env e, napi_status status, void* data) { - LoadExtensionBaton* baton = static_cast(data); + std::unique_ptr baton(static_cast(data)); Database* db = baton->db; + db->pending--; Napi::Env env = db->Env(); Napi::HandleScope scope(env); @@ -730,9 +724,6 @@ void Database::Work_AfterLoadExtension(napi_env e, napi_status status, void* dat } db->Process(); - - napi_delete_async_work(e, baton->request); - delete baton; } void Database::RemoveCallbacks() { diff --git a/src/database.h b/src/database.h index 13af919f9..ba8785fff 100644 --- a/src/database.h +++ b/src/database.h @@ -41,7 +41,7 @@ class Database : public Napi::ObjectWrap { } struct Baton { - napi_async_work request; + napi_async_work request = NULL; Database* db; Napi::FunctionReference callback; int status; @@ -55,6 +55,7 @@ class Database : public Napi::ObjectWrap { } } virtual ~Baton() { + if (request) napi_delete_async_work(db->Env(), request); db->Unref(); callback.Reset(); } diff --git a/src/macros.h b/src/macros.h index ce9448403..1c1a7c445 100644 --- a/src/macros.h +++ b/src/macros.h @@ -119,13 +119,14 @@ inline bool OtherIsInt(Napi::Number source) { // The Mac OS compiler complains when argv is NULL unless we // first assign it to a locally defined variable. -#define TRY_CATCH_CALL(context, callback, argc, argv) \ +#define TRY_CATCH_CALL(context, callback, argc, argv, ...) \ Napi::Value* passed_argv = argv;\ std::vector args;\ if ((argc != 0) && (passed_argv != NULL)) {\ args.assign(passed_argv, passed_argv + argc);\ }\ - (callback).MakeCallback(Napi::Value(context), args); + Napi::Value res = (callback).MakeCallback(Napi::Value(context), args); \ + if (res.IsEmpty()) return __VA_ARGS__; #define WORK_DEFINITION(name) \ Napi::Value name(const Napi::CallbackInfo& info); \ @@ -153,15 +154,21 @@ inline bool OtherIsInt(Napi::Number source) { type* baton = static_cast(data); \ Statement* stmt = baton->stmt; +#define STATEMENT_MUTEX(name) \ + if (!stmt->db->_handle) { \ + stmt->status = SQLITE_MISUSE; \ + stmt->message = "Database handle is closed"; \ + return; \ + } \ + sqlite3_mutex* name = sqlite3_db_mutex(stmt->db->_handle); + #define STATEMENT_END() \ assert(stmt->locked); \ assert(stmt->db->pending); \ stmt->locked = false; \ stmt->db->pending--; \ stmt->Process(); \ - stmt->db->Process(); \ - napi_delete_async_work(e, baton->request); \ - delete baton; + stmt->db->Process(); #define BACKUP_BEGIN(type) \ assert(baton); \ @@ -189,9 +196,7 @@ inline bool OtherIsInt(Napi::Number source) { backup->locked = false; \ backup->db->pending--; \ backup->Process(); \ - backup->db->Process(); \ - napi_delete_async_work(e, baton->request); \ - delete baton; + backup->db->Process(); #define DELETE_FIELD(field) \ if (field != NULL) { \ diff --git a/src/statement.cc b/src/statement.cc index 088707800..23842e1dc 100644 --- a/src/statement.cc +++ b/src/statement.cc @@ -42,11 +42,10 @@ void Statement::Process() { } while (prepared && !locked && !queue.empty()) { - Call* call = queue.front(); + std::unique_ptr call(queue.front()); queue.pop(); call->callback(call->baton); - delete call; } } @@ -133,7 +132,7 @@ void Statement::Work_Prepare(napi_env e, void* data) { // In case preparing fails, we use a mutex to make sure we get the associated // error message. - sqlite3_mutex* mtx = sqlite3_db_mutex(baton->db->_handle); + STATEMENT_MUTEX(mtx); sqlite3_mutex_enter(mtx); stmt->status = sqlite3_prepare_v2( @@ -153,13 +152,14 @@ void Statement::Work_Prepare(napi_env e, void* data) { } void Statement::Work_AfterPrepare(napi_env e, napi_status status, void* data) { - STATEMENT_INIT(PrepareBaton); + std::unique_ptr baton(static_cast(data)); + Statement* stmt = baton->stmt; Napi::Env env = stmt->Env(); Napi::HandleScope scope(env); if (stmt->status != SQLITE_OK) { - Error(baton); + Error(baton.get()); stmt->Finalize_(); } else { @@ -347,20 +347,21 @@ void Statement::Work_BeginBind(Baton* baton) { void Statement::Work_Bind(napi_env e, void* data) { STATEMENT_INIT(Baton); - sqlite3_mutex* mtx = sqlite3_db_mutex(stmt->db->_handle); + STATEMENT_MUTEX(mtx); sqlite3_mutex_enter(mtx); stmt->Bind(baton->parameters); sqlite3_mutex_leave(mtx); } void Statement::Work_AfterBind(napi_env e, napi_status status, void* data) { - STATEMENT_INIT(Baton); + std::unique_ptr baton(static_cast(data)); + Statement* stmt = baton->stmt; Napi::Env env = stmt->Env(); Napi::HandleScope scope(env); if (stmt->status != SQLITE_OK) { - Error(baton); + Error(baton.get()); } else { // Fire callbacks. @@ -399,7 +400,7 @@ void Statement::Work_Get(napi_env e, void* data) { STATEMENT_INIT(RowBaton); if (stmt->status != SQLITE_DONE || baton->parameters.size()) { - sqlite3_mutex* mtx = sqlite3_db_mutex(stmt->db->_handle); + STATEMENT_MUTEX(mtx); sqlite3_mutex_enter(mtx); if (stmt->Bind(baton->parameters)) { @@ -420,13 +421,14 @@ void Statement::Work_Get(napi_env e, void* data) { } void Statement::Work_AfterGet(napi_env e, napi_status status, void* data) { - STATEMENT_INIT(RowBaton); + std::unique_ptr baton(static_cast(data)); + Statement* stmt = baton->stmt; Napi::Env env = stmt->Env(); Napi::HandleScope scope(env); if (stmt->status != SQLITE_ROW && stmt->status != SQLITE_DONE) { - Error(baton); + Error(baton.get()); } else { // Fire callbacks. @@ -469,7 +471,7 @@ void Statement::Work_BeginRun(Baton* baton) { void Statement::Work_Run(napi_env e, void* data) { STATEMENT_INIT(RunBaton); - sqlite3_mutex* mtx = sqlite3_db_mutex(stmt->db->_handle); + STATEMENT_MUTEX(mtx); sqlite3_mutex_enter(mtx); // Make sure that we also reset when there are no parameters. @@ -493,13 +495,14 @@ void Statement::Work_Run(napi_env e, void* data) { } void Statement::Work_AfterRun(napi_env e, napi_status status, void* data) { - STATEMENT_INIT(RunBaton); + std::unique_ptr baton(static_cast(data)); + Statement* stmt = baton->stmt; Napi::Env env = stmt->Env(); Napi::HandleScope scope(env); if (stmt->status != SQLITE_ROW && stmt->status != SQLITE_DONE) { - Error(baton); + Error(baton.get()); } else { // Fire callbacks. @@ -538,7 +541,7 @@ void Statement::Work_BeginAll(Baton* baton) { void Statement::Work_All(napi_env e, void* data) { STATEMENT_INIT(RowsBaton); - sqlite3_mutex* mtx = sqlite3_db_mutex(stmt->db->_handle); + STATEMENT_MUTEX(mtx); sqlite3_mutex_enter(mtx); // Make sure that we also reset when there are no parameters. @@ -562,13 +565,14 @@ void Statement::Work_All(napi_env e, void* data) { } void Statement::Work_AfterAll(napi_env e, napi_status status, void* data) { - STATEMENT_INIT(RowsBaton); + std::unique_ptr baton(static_cast(data)); + Statement* stmt = baton->stmt; Napi::Env env = stmt->Env(); Napi::HandleScope scope(env); if (stmt->status != SQLITE_DONE) { - Error(baton); + Error(baton.get()); } else { // Fire callbacks. @@ -580,8 +584,8 @@ void Statement::Work_AfterAll(napi_env e, napi_status status, void* data) { Rows::const_iterator it = baton->rows.begin(); Rows::const_iterator end = baton->rows.end(); for (int i = 0; it < end; ++it, i++) { - (result).Set(i, RowToJS(env,*it)); - delete *it; + std::unique_ptr row(*it); + (result).Set(i, RowToJS(env,row.get())); } Napi::Value argv[] = { env.Null(), result }; @@ -640,7 +644,7 @@ void Statement::Work_Each(napi_env e, void* data) { Async* async = baton->async; - sqlite3_mutex* mtx = sqlite3_db_mutex(stmt->db->_handle); + STATEMENT_MUTEX(mtx); int retrieved = 0; @@ -710,10 +714,10 @@ void Statement::AsyncEach(uv_async_t* handle) { Rows::const_iterator it = rows.begin(); Rows::const_iterator end = rows.end(); for (int i = 0; it < end; ++it, i++) { - argv[1] = RowToJS(env,*it); + std::unique_ptr row(*it); + argv[1] = RowToJS(env,row.get()); async->retrieved++; TRY_CATCH_CALL(async->stmt->Value(), cb, 2, argv); - delete *it; } } } @@ -733,13 +737,14 @@ void Statement::AsyncEach(uv_async_t* handle) { } void Statement::Work_AfterEach(napi_env e, napi_status status, void* data) { - STATEMENT_INIT(EachBaton); + std::unique_ptr baton(static_cast(data)); + Statement* stmt = baton->stmt; Napi::Env env = stmt->Env(); Napi::HandleScope scope(env); if (stmt->status != SQLITE_DONE) { - Error(baton); + Error(baton.get()); } STATEMENT_END(); @@ -769,7 +774,8 @@ void Statement::Work_Reset(napi_env e, void* data) { } void Statement::Work_AfterReset(napi_env e, napi_status status, void* data) { - STATEMENT_INIT(Baton); + std::unique_ptr baton(static_cast(data)); + Statement* stmt = baton->stmt; Napi::Env env = stmt->Env(); Napi::HandleScope scope(env); @@ -865,7 +871,8 @@ Napi::Value Statement::Finalize_(const Napi::CallbackInfo& info) { return stmt->db->Value(); } -void Statement::Finalize_(Baton* baton) { +void Statement::Finalize_(Baton* b) { + std::unique_ptr baton(b); Napi::Env env = baton->stmt->Env(); Napi::HandleScope scope(env); @@ -876,8 +883,6 @@ void Statement::Finalize_(Baton* baton) { if (!cb.IsUndefined() && cb.IsFunction()) { TRY_CATCH_CALL(baton->stmt->Value(), cb, 0, NULL); } - - delete baton; } void Statement::Finalize_() { @@ -904,21 +909,17 @@ void Statement::CleanQueue() { // Clear out the queue so that this object can get GC'ed. while (!queue.empty()) { - Call* call = queue.front(); + std::unique_ptr call(queue.front()); queue.pop(); - Napi::Function cb = call->baton->callback.Value(); + std::unique_ptr baton(call->baton); + Napi::Function cb = baton->callback.Value(); if (prepared && !cb.IsEmpty() && cb.IsFunction()) { TRY_CATCH_CALL(Value(), cb, 1, argv); called = true; } - - // We don't call the actual callback, so we have to make sure that - // the baton gets destroyed. - delete call->baton; - delete call; } // When we couldn't call a callback function, emit an error on the @@ -931,12 +932,10 @@ void Statement::CleanQueue() { else while (!queue.empty()) { // Just delete all items in the queue; we already fired an event when // preparing the statement failed. - Call* call = queue.front(); + std::unique_ptr call(queue.front()); queue.pop(); - // We don't call the actual callback, so we have to make sure that // the baton gets destroyed. delete call->baton; - delete call; } } diff --git a/src/statement.h b/src/statement.h index 1d2a559ff..904e52175 100644 --- a/src/statement.h +++ b/src/statement.h @@ -76,7 +76,7 @@ class Statement : public Napi::ObjectWrap { static Napi::Value New(const Napi::CallbackInfo& info); struct Baton { - napi_async_work request; + napi_async_work request = NULL; Statement* stmt; Napi::FunctionReference callback; Parameters parameters; @@ -90,6 +90,7 @@ class Statement : public Napi::ObjectWrap { Values::Field* field = parameters[i]; DELETE_FIELD(field); } + if (request) napi_delete_async_work(stmt->Env(), request); stmt->Unref(); callback.Reset(); }