inspector: load network tracking modules eagerly

lib/internal/inspector/network.js

@@ -10,6 +10,7 @@ const {
 const dc = require('diagnostics_channel');
 const { now } = require('internal/perf/utils');
 const { MIMEType } = require('internal/mime');
+const { isNetworkDomainEnabled } = internalBinding('inspector');
 
 const kInspectorRequestId = Symbol('kInspectorRequestId');
 
@@ -87,6 +88,7 @@ module.exports = {
   kResourceType,
   getMonotonicTime,
   getNextRequestId,
+  isNetworkDomainEnabled,
   registerDiagnosticChannels,
   sniffMimeType,
 };

lib/internal/inspector/network_http.js

@@ -14,6 +14,7 @@ const {
   kResourceType,
   getMonotonicTime,
   getNextRequestId,
+  isNetworkDomainEnabled,
   registerDiagnosticChannels,
   sniffMimeType,
 } = require('internal/inspector/network');
@@ -74,6 +75,7 @@ const convertHeaderObject = (headers = kEmptyObject) => {
  * @param {{ request: import('http').ClientRequest }} event
  */
 function onClientRequestCreated({ request }) {
+  if (!isNetworkDomainEnabled()) return;
   request[kInspectorRequestId] = getNextRequestId();
 
   const { 0: headers, 1: host, 2: charset } = convertHeaderObject(request.getHeaders());

lib/internal/inspector/network_http2.js

@@ -13,6 +13,7 @@ const {
   kResourceType,
   getMonotonicTime,
   getNextRequestId,
+  isNetworkDomainEnabled,
   registerDiagnosticChannels,
   sniffMimeType,
 } = require('internal/inspector/network');
@@ -88,6 +89,7 @@ function convertHeaderObject(headers = kEmptyObject) {
  * @param {{ stream: import('http2').ClientHttp2Stream, headers: object }} event
  */
 function onClientStreamCreated({ stream, headers }) {
+  if (!isNetworkDomainEnabled()) return;
   stream[kInspectorRequestId] = getNextRequestId();
 
   const { 0: convertedHeaderObject, 1: url, 2: method, 4: charset } = convertHeaderObject(headers);

lib/internal/inspector/network_undici.js

@@ -10,6 +10,7 @@ const {
   kResourceType,
   getMonotonicTime,
   getNextRequestId,
+  isNetworkDomainEnabled,
   registerDiagnosticChannels,
   sniffMimeType,
 } = require('internal/inspector/network');
@@ -70,6 +71,7 @@ function responseHeadersArrayToDictionary(headers) {
  * @param {{ request: undici.Request }} event
  */
 function onClientRequestStart({ request }) {
+  if (!isNetworkDomainEnabled()) return;
   const url = `${request.origin}${request.path}`;
   request[kInspectorRequestId] = getNextRequestId();
 
@@ -210,6 +212,7 @@ function onClientResponseFinish({ request }) {
 // undici:websocket:open fires when the connection is established, but this results
 // in an inaccurate stack trace.
 function onWebSocketOpen({ websocket, handshakeResponse }) {
+  if (!isNetworkDomainEnabled()) return;
   websocket[kInspectorRequestId] = getNextRequestId();
   const url = websocket.url.toString();
   Network.webSocketCreated({

lib/internal/inspector_network_tracking.js

@@ -1,18 +1,29 @@
 'use strict';
 
-function enable() {
-  require('internal/inspector/network_http').enable();
-  require('internal/inspector/network_http2').enable();
-  require('internal/inspector/network_undici').enable();
-}
+// Network tracking is loaded and enabled at startup, when
+// --experimental-network-inspection is set, rather than lazily from the
+// inspector agent when a `Network.enable` message arrives. That message is
+// dispatched via an interrupt that can land in the middle of arbitrary
+// JavaScript execution -- including while another module is still being
+// loaded by require(). Calling into JS at that point (the tracking modules
+// pull in `inspector`, `worker_threads` and `stream`) can observe a
+// half-initialized module and throw, which the inspector agent turns into a
+// fatal, unrecoverable error that aborts the process.
+//
+// With the channels subscribed eagerly, `Network.enable`/`Network.disable`
+// only toggle a flag in C++ (NetworkInspector::enabled_) that gates event
+// emission, and never call into JS from the interrupt.
+// Refs: https://github.com/nodejs/node/issues/64308
+const networkHttp = require('internal/inspector/network_http');
+const networkHttp2 = require('internal/inspector/network_http2');
+const networkUndici = require('internal/inspector/network_undici');
 
-function disable() {
-  require('internal/inspector/network_http').disable();
-  require('internal/inspector/network_http2').disable();
-  require('internal/inspector/network_undici').disable();
+function enable() {
+  networkHttp.enable();
+  networkHttp2.enable();
+  networkUndici.enable();
 }
 
 module.exports = {
   enable,
-  disable,
 };

lib/internal/process/pre_execution.js

@@ -614,11 +614,11 @@ function setupInspectorHooks() {
 
 function setupNetworkInspection() {
   if (internalBinding('config').hasInspector && getOptionValue('--experimental-network-inspection')) {
-    const {
-      enable,
-      disable,
-    } = require('internal/inspector_network_tracking');
-    internalBinding('inspector').setupNetworkTracking(enable, disable);
+    // Subscribe the network tracking diagnostics channels right away, so that
+    // a `Network.enable` inspector message never has to call into JS from an
+    // interrupt. Event emission is gated natively until `Network.enable` is
+    // received.
+    require('internal/inspector_network_tracking').enable();
   }
 }
 

src/env_properties.h

@@ -509,9 +509,7 @@
   V(immediate_callback_function, v8::Function)                                 \
   V(inspector_console_extension_installer, v8::Function)                       \
   V(inspector_disable_async_hooks, v8::Function)                               \
-  V(inspector_disable_network_tracking, v8::Function)                          \
   V(inspector_enable_async_hooks, v8::Function)                                \
-  V(inspector_enable_network_tracking, v8::Function)                           \
   V(maybe_cache_generated_source_map, v8::Function)                            \
   V(messaging_deserialize_create_object, v8::Function)                         \
   V(message_port, v8::Object)                                                  \

src/inspector/network_inspector.cc

@@ -36,17 +36,18 @@ void NetworkInspector::emitNotification(v8::Local<v8::Context> context,
   }
 }
 
+// The network tracking diagnostics channels are subscribed at startup when
+// --experimental-network-inspection is set (see
+// lib/internal/process/pre_execution.js), so enabling and disabling only
+// toggles the flag that gates event emission. This must not call into JS:
+// Network.enable/Network.disable are dispatched via an interrupt that can
+// land in the middle of arbitrary JavaScript execution.
+// Refs: https://github.com/nodejs/node/issues/64308
 void NetworkInspector::Enable() {
-  if (auto agent = env_->inspector_agent()) {
-    agent->EnableNetworkTracking();
-  }
   enabled_ = true;
 }
 
 void NetworkInspector::Disable() {
-  if (auto agent = env_->inspector_agent()) {
-    agent->DisableNetworkTracking();
-  }
   enabled_ = false;
 }
 

src/inspector_agent.cc

@@ -374,6 +374,10 @@ class ChannelImpl final : public v8_inspector::V8Inspector::Channel,
     return retaining_context_;
   }
 
+  bool isNetworkDomainEnabled() {
+    return network_inspector_ && network_inspector_->IsEnabled();
+  }
+
  private:
   // v8_inspector::V8Inspector::Channel
   void sendResponse(
@@ -780,6 +784,13 @@ class NodeInspectorClient : public V8InspectorClient {
     return !channels_.empty();
   }
 
+  bool isNetworkDomainEnabled() {
+    for (const auto& id_channel : channels_) {
+      if (id_channel.second->isNetworkDomainEnabled()) return true;
+    }
+    return false;
+  }
+
  private:
   bool shouldRunMessageLoop() {
     if (waiting_for_frontend_)
@@ -996,60 +1007,6 @@ void Agent::EmitProtocolEvent(v8::Local<v8::Context> context,
   client_->emitNotification(context, event, params);
 }
 
-void Agent::SetupNetworkTracking(Local<Function> enable_function,
-                                 Local<Function> disable_function) {
-  parent_env_->set_inspector_enable_network_tracking(enable_function);
-  parent_env_->set_inspector_disable_network_tracking(disable_function);
-  if (pending_enable_network_tracking) {
-    pending_enable_network_tracking = false;
-    EnableNetworkTracking();
-  } else if (pending_disable_network_tracking) {
-    pending_disable_network_tracking = false;
-    DisableNetworkTracking();
-  }
-}
-
-void Agent::EnableNetworkTracking() {
-  if (network_tracking_enabled_) {
-    return;
-  }
-  HandleScope scope(parent_env_->isolate());
-  Local<Function> enable = parent_env_->inspector_enable_network_tracking();
-  if (enable.IsEmpty()) {
-    pending_enable_network_tracking = true;
-  } else {
-    ToggleNetworkTracking(parent_env_->isolate(), enable);
-    network_tracking_enabled_ = true;
-  }
-}
-
-void Agent::DisableNetworkTracking() {
-  if (!network_tracking_enabled_) {
-    return;
-  }
-  HandleScope scope(parent_env_->isolate());
-  Local<Function> disable = parent_env_->inspector_disable_network_tracking();
-  if (disable.IsEmpty()) {
-    pending_disable_network_tracking = true;
-  } else if (!client_->hasConnectedSessions()) {
-    ToggleNetworkTracking(parent_env_->isolate(), disable);
-    network_tracking_enabled_ = false;
-  }
-}
-
-void Agent::ToggleNetworkTracking(Isolate* isolate, Local<Function> fn) {
-  if (!parent_env_->can_call_into_js()) return;
-  auto context = parent_env_->context();
-  HandleScope scope(isolate);
-  CHECK(!fn.IsEmpty());
-  v8::TryCatch try_catch(isolate);
-  USE(fn->Call(context, Undefined(isolate), 0, nullptr));
-  if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
-    PrintCaughtException(isolate, context, try_catch);
-    UNREACHABLE("Cannot toggle network tracking, please report this.");
-  }
-}
-
 void Agent::WaitForDisconnect() {
   THROW_IF_INSUFFICIENT_PERMISSIONS(parent_env_,
                                     permission::PermissionScope::kInspector,
@@ -1219,6 +1176,10 @@ bool Agent::IsActive() {
   return io_ != nullptr || client_->IsActive();
 }
 
+bool Agent::IsNetworkDomainEnabled() {
+  return client_ != nullptr && client_->isNetworkDomainEnabled();
+}
+
 void Agent::SetParentHandle(
     std::unique_ptr<ParentInspectorHandle> parent_handle) {
   parent_handle_ = std::move(parent_handle);

src/inspector_agent.h

@@ -74,11 +74,8 @@ class Agent {
   void EmitProtocolEvent(v8::Local<v8::Context> context,
                          const v8_inspector::StringView& event,
                          v8::Local<v8::Object> params);
-
-  void SetupNetworkTracking(v8::Local<v8::Function> enable_function,
-                            v8::Local<v8::Function> disable_function);
-  void EnableNetworkTracking();
-  void DisableNetworkTracking();
+  // Returns true if any session has enabled the Network domain.
+  bool IsNetworkDomainEnabled();
 
   // Async stack traces instrumentation.
   void AsyncTaskScheduled(const v8_inspector::StringView& taskName, void* task,
@@ -134,7 +131,6 @@ class Agent {
 
  private:
   void SyncAsyncHookState();
-  void ToggleNetworkTracking(v8::Isolate* isolate, v8::Local<v8::Function> fn);
 
   node::Environment* parent_env_;
   // Encapsulates majority of the Inspector functionality
@@ -164,9 +160,6 @@ class Agent {
   void StopAcceptingIoThreadStarts();
   static void StopAcceptingIoThreadStartsHook(void* agent);
 
-  bool network_tracking_enabled_ = false;
-  bool pending_enable_network_tracking = false;
-  bool pending_disable_network_tracking = false;
   std::shared_ptr<NetworkResourceManager> network_resource_manager_;
 };
 

src/inspector_js_api.cc

@@ -359,21 +359,14 @@ void EmitProtocolEvent(const FunctionCallbackInfo<Value>& args) {
       params);
 }
 
-void SetupNetworkTracking(const FunctionCallbackInfo<Value>& args) {
+void IsEnabled(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
-
-  CHECK(args[0]->IsFunction());
-  Local<Function> enable_function = args[0].As<Function>();
-  CHECK(args[1]->IsFunction());
-  Local<Function> disable_function = args[1].As<Function>();
-
-  env->inspector_agent()->SetupNetworkTracking(enable_function,
-                                               disable_function);
+  args.GetReturnValue().Set(env->inspector_agent()->IsListening());
 }
 
-void IsEnabled(const FunctionCallbackInfo<Value>& args) {
+void IsNetworkDomainEnabled(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
-  args.GetReturnValue().Set(env->inspector_agent()->IsListening());
+  args.GetReturnValue().Set(env->inspector_agent()->IsNetworkDomainEnabled());
 }
 
 void Open(const FunctionCallbackInfo<Value>& args) {
@@ -473,8 +466,9 @@ void Initialize(Local<Object> target, Local<Value> unused,
 
   SetMethod(context, target, "registerAsyncHook", RegisterAsyncHookWrapper);
   SetMethodNoSideEffect(context, target, "isEnabled", IsEnabled);
+  SetMethodNoSideEffect(
+      context, target, "isNetworkDomainEnabled", IsNetworkDomainEnabled);
   SetMethod(context, target, "emitProtocolEvent", EmitProtocolEvent);
-  SetMethod(context, target, "setupNetworkTracking", SetupNetworkTracking);
   SetMethod(context, target, "putNetworkResource", PutNetworkResource);
 
   Local<String> console_string = FIXED_ONE_BYTE_STRING(isolate, "console");
@@ -509,8 +503,8 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
 
   registry->Register(RegisterAsyncHookWrapper);
   registry->Register(IsEnabled);
+  registry->Register(IsNetworkDomainEnabled);
   registry->Register(EmitProtocolEvent);
-  registry->Register(SetupNetworkTracking);
 
   registry->Register(JSBindingsConnection<LocalConnection>::New);
   registry->Register(JSBindingsConnection<LocalConnection>::Dispatch);