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]>
nodejs · Mar 30, 2019 · 7aad63b · 7aad63b
1 parent 58bf615
commit 7aad63b
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 12 deletions.
diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
@@ -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

diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js
@@ -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
@@ -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.
@@ -361,5 +376,6 @@ module.exports = {
   initializeFrozenIntrinsics,
   loadPreloadModules,
   setupTraceCategoryState,
+  setupInspectorHooks,
   initializeReport
 };
diff --git a/lib/internal/main/worker_thread.js b/lib/internal/main/worker_thread.js
@@ -6,6 +6,7 @@
 const {
   patchProcessObject,
   setupCoverageHooks,
+  setupInspectorHooks,
   setupWarningHandler,
   setupDebugEnv,
   initializeDeprecations,
@@ -43,6 +44,7 @@ const {
 const publicWorker = require('worker_threads');
 
 patchProcessObject();
+setupInspectorHooks();
 setupDebugEnv();
 
 const debug = require('internal/util/debuglog').debuglog('worker');

diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc
@@ -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();