Skip to content

Commit

Permalink
Fix segfaults (#1368)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
mohd-akram committed Sep 6, 2020
1 parent c9caae4 commit e87dfa4
Show file tree
Hide file tree
Showing 7 changed files with 109 additions and 114 deletions.
29 changes: 13 additions & 16 deletions src/backup.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,10 @@ void Backup::Process() {
}

while (inited && !locked && !queue.empty()) {
Call* call = queue.front();
std::unique_ptr<Call> call(queue.front());
queue.pop();

call->callback(call->baton);
delete call;
}
}

Expand Down Expand Up @@ -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> call(queue.front());
queue.pop();

Napi::Function cb = call->baton->callback.Value();
std::unique_ptr<Baton> 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
Expand All @@ -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> 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;
}
}

Expand Down Expand Up @@ -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<InitializeBaton> baton(static_cast<InitializeBaton*>(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 {
Expand Down Expand Up @@ -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<StepBaton> baton(static_cast<StepBaton*>(data));
Backup* backup = baton->backup;

Napi::Env env = backup->Env();
Napi::HandleScope scope(env);
Expand All @@ -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.
Expand Down Expand Up @@ -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> baton(static_cast<Baton*>(data));
Backup* backup = baton->backup;

Napi::Env env = backup->Env();
Napi::HandleScope scope(env);
Expand Down
3 changes: 2 additions & 1 deletion src/backup.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ class Backup : public Napi::ObjectWrap<Backup> {
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;

Expand All @@ -105,6 +105,7 @@ class Backup : public Napi::ObjectWrap<Backup> {
callback.Reset(cb_, 1);
}
virtual ~Baton() {
if (request) napi_delete_async_work(backup->Env(), request);
backup->Unref();
callback.Reset();
}
Expand Down
Loading

0 comments on commit e87dfa4

Please sign in to comment.