Skip to content

Commit 54dd978

Browse files
bnoordhuisRafaelGSS
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 864ecb0 commit 54dd978

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)