Skip to content

Commit e776b01

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 <anna@addaleax.net> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Minwoo Jung <minwoo@nodesource.com> Signed-off-by: Beth Griggs <Bethany.Griggs@uk.ibm.com>
1 parent 6cbd6b5 commit e776b01

File tree

4 files changed

+20
-12
lines changed

4 files changed

+20
-12
lines changed

lib/internal/bootstrap/node.js

-11
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

+17-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ function prepareMainThreadExecution() {
66
// Patch the process object with legacy properties and normalizations
77
patchProcessObject();
88
setupTraceCategoryState();
9-
9+
setupInspectorHooks();
1010
setupWarningHandler();
1111

1212
// Resolve the coverage directory to an absolute path, and
@@ -181,6 +181,21 @@ function setupTraceCategoryState() {
181181
toggleTraceCategoryState(isTraceCategoryEnabled('node.async_hooks'));
182182
}
183183

184+
function setupInspectorHooks() {
185+
// If Debugger.setAsyncCallStackDepth is sent during bootstrap,
186+
// we cannot immediately call into JS to enable the hooks, which could
187+
// interrupt the JS execution of bootstrap. So instead we save the
188+
// notification in the inspector agent if it's sent in the middle of
189+
// bootstrap, and process the notification later here.
190+
if (internalBinding('config').hasInspector) {
191+
const {
192+
enable,
193+
disable
194+
} = require('internal/inspector_async_hook');
195+
internalBinding('inspector').registerAsyncHook(enable, disable);
196+
}
197+
}
198+
184199
// In general deprecations are intialized wherever the APIs are implemented,
185200
// this is used to deprecate APIs implemented in C++ where the deprecation
186201
// utitlities are not easily accessible.
@@ -360,5 +375,6 @@ module.exports = {
360375
initializeFrozenIntrinsics,
361376
loadPreloadModules,
362377
setupTraceCategoryState,
378+
setupInspectorHooks,
363379
initializeReport
364380
};

lib/internal/main/worker_thread.js

+2
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

+1
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)