Skip to content

Commit 4e1fe29

Browse files
joyeecheungBethGriggs
authored andcommitted
src: do not call into JS in the maxAsyncCallStackDepthChanged interrupt
If Debugger.setAsyncCallStackDepth is sent during bootstrap, we cannot immediately call into JS to enable the hooks, which could interrupt the JS execution of bootstrap. So instead we save the notification in the inspector agent if it's sent in the middle of bootstrap, and process the notification later here. Refs: #26798 PR-URL: #26935 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Eugene Ostroukhov <[email protected]> Reviewed-By: Minwoo Jung <[email protected]>
1 parent 82ffd94 commit 4e1fe29

File tree

4 files changed

+20
-12
lines changed

4 files changed

+20
-12
lines changed

lib/internal/bootstrap/node.js

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -157,17 +157,6 @@ if (isMainThread) {
157157
const { nativeHooks } = require('internal/async_hooks');
158158
internalBinding('async_wrap').setupHooks(nativeHooks);
159159

160-
// XXX(joyeecheung): this has to be done after the initial load of
161-
// `internal/async_hooks` otherwise `async_hooks` cannot require
162-
// `internal/async_hooks`. Investigate why.
163-
if (config.hasInspector) {
164-
const {
165-
enable,
166-
disable
167-
} = require('internal/inspector_async_hook');
168-
internalBinding('inspector').registerAsyncHook(enable, disable);
169-
}
170-
171160
const {
172161
setupTaskQueue,
173162
queueMicrotask

lib/internal/bootstrap/pre_execution.js

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ function prepareMainThreadExecution() {
77
// Patch the process object with legacy properties and normalizations
88
patchProcessObject();
99
setupTraceCategoryState();
10-
10+
setupInspectorHooks();
1111
setupWarningHandler();
1212

1313
// Resolve the coverage directory to an absolute path, and
@@ -168,6 +168,21 @@ function setupTraceCategoryState() {
168168
toggleTraceCategoryState(isTraceCategoryEnabled('node.async_hooks'));
169169
}
170170

171+
function setupInspectorHooks() {
172+
// If Debugger.setAsyncCallStackDepth is sent during bootstrap,
173+
// we cannot immediately call into JS to enable the hooks, which could
174+
// interrupt the JS execution of bootstrap. So instead we save the
175+
// notification in the inspector agent if it's sent in the middle of
176+
// bootstrap, and process the notification later here.
177+
if (internalBinding('config').hasInspector) {
178+
const {
179+
enable,
180+
disable
181+
} = require('internal/inspector_async_hook');
182+
internalBinding('inspector').registerAsyncHook(enable, disable);
183+
}
184+
}
185+
171186
// In general deprecations are intialized wherever the APIs are implemented,
172187
// this is used to deprecate APIs implemented in C++ where the deprecation
173188
// utitlities are not easily accessible.
@@ -361,5 +376,6 @@ module.exports = {
361376
initializeFrozenIntrinsics,
362377
loadPreloadModules,
363378
setupTraceCategoryState,
379+
setupInspectorHooks,
364380
initializeReport
365381
};

lib/internal/main/worker_thread.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
const {
77
patchProcessObject,
88
setupCoverageHooks,
9+
setupInspectorHooks,
910
setupWarningHandler,
1011
setupDebugEnv,
1112
initializeDeprecations,
@@ -43,6 +44,7 @@ const {
4344
const publicWorker = require('worker_threads');
4445

4546
patchProcessObject();
47+
setupInspectorHooks();
4648
setupDebugEnv();
4749

4850
const debug = require('internal/util/debuglog').debuglog('worker');

src/inspector_agent.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -835,6 +835,7 @@ void Agent::DisableAsyncHook() {
835835

836836
void Agent::ToggleAsyncHook(Isolate* isolate,
837837
const node::Persistent<Function>& fn) {
838+
CHECK(parent_env_->has_run_bootstrapping_code());
838839
HandleScope handle_scope(isolate);
839840
CHECK(!fn.IsEmpty());
840841
auto context = parent_env_->context();

0 commit comments

Comments
 (0)