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 <santiago.gimeno@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
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

Lines changed: 7 additions & 1 deletion
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)