Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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 <me@gus.host>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
addaleax authored and MylesBorins committed Jul 27, 2020
1 parent 3fda3d4 commit d6ee1fd
Showing 1 changed file with 6 additions and 1 deletion.
7 changes: 6 additions & 1 deletion src/inspector_agent.cc
Expand Up @@ -917,13 +917,18 @@ void Agent::DisableAsyncHook() {

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

0 comments on commit d6ee1fd

Please sign in to comment.