Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit a1da012

Browse filesBrowse files
addaleaxBethGriggs
authored andcommitted
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>
1 parent 4d027b7 commit a1da012
Copy full SHA for a1da012

File tree

Expand file treeCollapse file tree

1 file changed

+6
-1
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

1 file changed

+6
-1
lines changed
Open diff view settings
Collapse file

‎src/inspector_agent.cc‎

Copy file name to clipboardExpand all lines: src/inspector_agent.cc
+6-1Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -928,13 +928,18 @@ void Agent::DisableAsyncHook() {
928928

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

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.