Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: do not call into JS in the maxAsyncCallStackDepthChanged interrupt #26935

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,17 +157,6 @@ if (isMainThread) {
const { nativeHooks } = require('internal/async_hooks');
internalBinding('async_wrap').setupHooks(nativeHooks);

// XXX(joyeecheung): this has to be done after the initial load of
// `internal/async_hooks` otherwise `async_hooks` cannot require
// `internal/async_hooks`. Investigate why.
if (config.hasInspector) {
const {
enable,
disable
} = require('internal/inspector_async_hook');
internalBinding('inspector').registerAsyncHook(enable, disable);
}

const {
setupTaskQueue,
queueMicrotask
Expand Down
18 changes: 17 additions & 1 deletion lib/internal/bootstrap/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ function prepareMainThreadExecution() {
// Patch the process object with legacy properties and normalizations
patchProcessObject();
setupTraceCategoryState();

setupInspectorHooks();
setupWarningHandler();

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

function setupInspectorHooks() {
// 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.
if (internalBinding('config').hasInspector) {
const {
enable,
disable
} = require('internal/inspector_async_hook');
internalBinding('inspector').registerAsyncHook(enable, disable);
}
}

// In general deprecations are intialized wherever the APIs are implemented,
// this is used to deprecate APIs implemented in C++ where the deprecation
// utitlities are not easily accessible.
Expand Down Expand Up @@ -357,5 +372,6 @@ module.exports = {
initializeFrozenIntrinsics,
loadPreloadModules,
setupTraceCategoryState,
setupInspectorHooks,
initializeReport
};
2 changes: 2 additions & 0 deletions lib/internal/main/worker_thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
const {
patchProcessObject,
setupCoverageHooks,
setupInspectorHooks,
setupWarningHandler,
setupDebugEnv,
initializeDeprecations,
Expand Down Expand Up @@ -43,6 +44,7 @@ const {
const publicWorker = require('worker_threads');

patchProcessObject();
setupInspectorHooks();
setupDebugEnv();

const debug = require('internal/util/debuglog').debuglog('worker');
Expand Down
1 change: 1 addition & 0 deletions src/inspector_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,7 @@ void Agent::DisableAsyncHook() {

void Agent::ToggleAsyncHook(Isolate* isolate,
const node::Persistent<Function>& fn) {
CHECK(parent_env_->has_run_bootstrapping_code());
HandleScope handle_scope(isolate);
CHECK(!fn.IsEmpty());
auto context = parent_env_->context();
Expand Down