Skip to content

Commit

Permalink
src: enter isolate before destructing IsolateData
Browse files Browse the repository at this point in the history
MVP fix for a worker_threads crash where ~WorkerThreadData() ->
~IsolateData() -> Isolate::DetachCppHeap() kicks off a round of
garbage collection that expects an entered isolate.

No test because the crash is not reliably reproducable but the bug
is pretty clearly described in the linked issue and is obvious once
you see it, IMO.

Fixes: nodejs#51129
  • Loading branch information
bnoordhuis committed Dec 12, 2023
1 parent 1b60054 commit 6bc6746
Showing 1 changed file with 7 additions and 1 deletion.
8 changes: 7 additions & 1 deletion src/node_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,13 @@ class WorkerThreadData {
CHECK(!loop_init_failed_);
bool platform_finished = false;

isolate_data_.reset();
// https://github.com/nodejs/node/issues/51129 - IsolateData destructor
// can kick off GC before teardown, so ensure the isolate is entered.
{
Locker locker(isolate);
Isolate::Scope isolate_scope(isolate);
isolate_data_.reset();
}

w_->platform_->AddIsolateFinishedCallback(isolate, [](void* data) {
*static_cast<bool*>(data) = true;
Expand Down

0 comments on commit 6bc6746

Please sign in to comment.