Skip to content

Commit 687dbd8

Browse files
committed
src: do not crash if ToggleAsyncHook fails during termination
In the termination case, we should not crash. There’s also no harm being done by ignoring the termination exception here, since the thread is about to be torn down anyway. Also, add a guard against running this during shutdown. That is the likely cause of #34361. Fixes: #34361 PR-URL: #34362 Fixes: #27261 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 08b6335 commit 687dbd8

File tree

1 file changed

+6
-1
lines changed

1 file changed

+6
-1
lines changed

src/inspector_agent.cc

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -841,13 +841,18 @@ void Agent::DisableAsyncHook() {
841841

842842
void Agent::ToggleAsyncHook(Isolate* isolate,
843843
const Global<Function>& fn) {
844+
// Guard against running this during cleanup -- no async events will be
845+
// emitted anyway at that point anymore, and calling into JS is not possible.
846+
// This should probably not be something we're attempting in the first place,
847+
// Refs: https://github.com/nodejs/node/pull/34362#discussion_r456006039
848+
if (!parent_env_->can_call_into_js()) return;
844849
CHECK(parent_env_->has_run_bootstrapping_code());
845850
HandleScope handle_scope(isolate);
846851
CHECK(!fn.IsEmpty());
847852
auto context = parent_env_->context();
848853
v8::TryCatch try_catch(isolate);
849854
USE(fn.Get(isolate)->Call(context, Undefined(isolate), 0, nullptr));
850-
if (try_catch.HasCaught()) {
855+
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
851856
PrintCaughtException(isolate, context, try_catch);
852857
FatalError("\nnode::inspector::Agent::ToggleAsyncHook",
853858
"Cannot toggle Inspector's AsyncHook, please report this.");

0 commit comments

Comments
 (0)