Skip to content

Commit aa87b77

Browse files
authored
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 c64e603 commit aa87b77

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)