Skip to content

Commit d76e16b

Browse files
bnoordhuisrichardlau
authored andcommitted
src: enter isolate before destructing IsolateData
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: #51129 PR-URL: #51138 Reviewed-By: Santiago Gimeno <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Stephen Belanger <[email protected]>
1 parent f79ac33 commit d76e16b

File tree

1 file changed

+7
-1
lines changed

1 file changed

+7
-1
lines changed

src/node_worker.cc

+7-1
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,13 @@ class WorkerThreadData {
217217
CHECK(!loop_init_failed_);
218218
bool platform_finished = false;
219219

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

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

0 commit comments

Comments
 (0)