process: start coverage collection before bootstrap

This patch moves the dispatch of `Profiler.takePreciseCoverage`
to a point before the bootstrap scripts are run to ensure that
we can collect coverage data for all the scripts run after
the inspector agent is ready.

Before this patch `lib/internal/bootstrap/primordials.js` was not
covered by `make coverage`, after this patch it is.

PR-URL: https://github.com/nodejs/node/pull/26006
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Coe <bencoe@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

lib/internal/bootstrap/loaders.js

@@ -311,18 +311,5 @@ NativeModule.prototype.compile = function() {
   }
 };
 
-// Coverage must be turned on early, so that we can collect
-// it for Node.js' own internal libraries.
-if (process.env.NODE_V8_COVERAGE) {
-  if (internalBinding('config').hasInspector) {
-    const coverage =
-      NativeModule.require('internal/coverage-gen/with_profiler');
-    // Inform the profiler to start collecting coverage
-    coverage.startCoverageCollection();
-  } else {
-    process._rawDebug('NODE_V8_COVERAGE cannot be used without inspector');
-  }
-}
-
 // This will be passed to internal/bootstrap/node.js.
 return loaderExports;

lib/internal/coverage-gen/with_profiler.js

@@ -3,14 +3,9 @@
 // Implements coverage collection exposed by the `NODE_V8_COVERAGE`
 // environment variable which can also be used in the user land.
 
-let coverageConnection = null;
 let coverageDirectory;
 
 function writeCoverage() {
-  if (!coverageConnection && coverageDirectory) {
-    return;
-  }
-
   const { join } = require('path');
   const { mkdirSync, writeFileSync } = require('fs');
   const { threadId } = require('internal/worker');
@@ -28,21 +23,14 @@ function writeCoverage() {
   const target = join(coverageDirectory, filename);
   try {
     disableAllAsyncHooks();
-    let msg;
+    internalBinding('coverage').end((msg) => {
-    coverageConnection._coverageCallback = function(_msg) {
+      const coverageInfo = JSON.parse(msg).result;
-      msg = _msg;
+      if (coverageInfo) {
-    };
+        writeFileSync(target, JSON.stringify(coverageInfo));
-    coverageConnection.dispatch(JSON.stringify({
+      }
-      id: 3,
+    });
-      method: 'Profiler.takePreciseCoverage'
-    }));
-    const coverageInfo = JSON.parse(msg).result;
-    writeFileSync(target, JSON.stringify(coverageInfo));
   } catch (err) {
     console.error(err);
-  } finally {
-    coverageConnection.disconnect();
-    coverageConnection = null;
   }
 }
 
@@ -52,33 +40,11 @@ function disableAllAsyncHooks() {
   hooks_array.forEach((hook) => { hook.disable(); });
 }
 
-function startCoverageCollection() {
-  const { Connection } = internalBinding('inspector');
-  coverageConnection = new Connection((res) => {
-    if (coverageConnection._coverageCallback) {
-      coverageConnection._coverageCallback(res);
-    }
-  });
-  coverageConnection.dispatch(JSON.stringify({
-    id: 1,
-    method: 'Profiler.enable'
-  }));
-  coverageConnection.dispatch(JSON.stringify({
-    id: 2,
-    method: 'Profiler.startPreciseCoverage',
-    params: {
-      callCount: true,
-      detailed: true
-    }
-  }));
-}
-
 function setCoverageDirectory(dir) {
   coverageDirectory = dir;
 }
 
 module.exports = {
-  startCoverageCollection,
   writeCoverage,
   setCoverageDirectory
 };

src/env.h

@@ -331,6 +331,7 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2;
   V(async_wrap_ctor_template, v8::FunctionTemplate)                            \
   V(async_wrap_object_ctor_template, v8::FunctionTemplate)                     \
   V(buffer_prototype_object, v8::Object)                                       \
+  V(coverage_connection, v8::Object)                                           \
   V(context, v8::Context)                                                      \
   V(crypto_key_object_constructor, v8::Function)                               \
   V(domain_callback, v8::Function)                                             \
@@ -366,6 +367,7 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2;
   V(message_event_object_template, v8::ObjectTemplate)                         \
   V(message_port_constructor_template, v8::FunctionTemplate)                   \
   V(native_module_require, v8::Function)                                       \
+  V(on_coverage_message_function, v8::Function)                                \
   V(performance_entry_callback, v8::Function)                                  \
   V(performance_entry_template, v8::Function)                                  \
   V(pipe_constructor_template, v8::FunctionTemplate)                           \
@@ -450,9 +452,10 @@ struct ContextInfo {
 
 // Listing the AsyncWrap provider types first enables us to cast directly
 // from a provider type to a debug category.
-#define DEBUG_CATEGORY_NAMES(V) \
+#define DEBUG_CATEGORY_NAMES(V)                                                \
-    NODE_ASYNC_PROVIDER_TYPES(V) \
+  NODE_ASYNC_PROVIDER_TYPES(V)                                                 \
-    V(INSPECTOR_SERVER)
+  V(INSPECTOR_SERVER)                                                          \
+  V(COVERAGE)
 
 enum class DebugCategory {
 #define V(name) name,

src/inspector/node_inspector.gypi

@@ -45,6 +45,7 @@
     '../../src/inspector_io.cc',
     '../../src/inspector_agent.h',
     '../../src/inspector_io.h',
+    '../../src/inspector_coverage.cc',
     '../../src/inspector_js_api.cc',
     '../../src/inspector_socket.cc',
     '../../src/inspector_socket.h',

src/inspector_coverage.cc

@@ -0,0 +1,168 @@
+#include "base_object-inl.h"
+#include "debug_utils.h"
+#include "inspector_agent.h"
+#include "node_internals.h"
+#include "v8-inspector.h"
+
+namespace node {
+namespace coverage {
+
+using v8::Context;
+using v8::Function;
+using v8::FunctionCallbackInfo;
+using v8::HandleScope;
+using v8::Isolate;
+using v8::Local;
+using v8::MaybeLocal;
+using v8::NewStringType;
+using v8::Object;
+using v8::ObjectTemplate;
+using v8::String;
+using v8::Value;
+
+using v8_inspector::StringBuffer;
+using v8_inspector::StringView;
+
+std::unique_ptr<StringBuffer> ToProtocolString(Isolate* isolate,
+                                               Local<Value> value) {
+  TwoByteValue buffer(isolate, value);
+  return StringBuffer::create(StringView(*buffer, buffer.length()));
+}
+
+class V8CoverageConnection : public BaseObject {
+ public:
+  class V8CoverageSessionDelegate : public inspector::InspectorSessionDelegate {
+   public:
+    explicit V8CoverageSessionDelegate(V8CoverageConnection* connection)
+        : connection_(connection) {}
+
+    void SendMessageToFrontend(
+        const v8_inspector::StringView& message) override {
+      Environment* env = connection_->env();
+      Local<Function> fn = connection_->env()->on_coverage_message_function();
+      bool ending = !fn.IsEmpty();
+      Debug(env,
+            DebugCategory::COVERAGE,
+            "Sending message to frontend, ending = %s\n",
+            ending ? "true" : "false");
+      if (!ending) {
+        return;
+      }
+      Isolate* isolate = env->isolate();
+
+      HandleScope handle_scope(isolate);
+      Context::Scope context_scope(env->context());
+      MaybeLocal<String> v8string =
+          String::NewFromTwoByte(isolate,
+                                 message.characters16(),
+                                 NewStringType::kNormal,
+                                 message.length());
+      Local<Value> args[] = {v8string.ToLocalChecked().As<Value>()};
+      USE(MakeCallback(isolate,
+                       connection_->object(),
+                       fn,
+                       arraysize(args),
+                       args,
+                       async_context{0, 0}));
+    }
+
+   private:
+    V8CoverageConnection* connection_;
+  };
+
+  SET_MEMORY_INFO_NAME(V8CoverageConnection)
+  SET_SELF_SIZE(V8CoverageConnection)
+
+  void MemoryInfo(MemoryTracker* tracker) const override {
+    tracker->TrackFieldWithSize(
+        "session", sizeof(*session_), "InspectorSession");
+  }
+
+  explicit V8CoverageConnection(Environment* env)
+      : BaseObject(env, env->coverage_connection()), session_(nullptr) {
+    inspector::Agent* inspector = env->inspector_agent();
+    std::unique_ptr<inspector::InspectorSession> session = inspector->Connect(
+        std::make_unique<V8CoverageSessionDelegate>(this), false);
+    session_ = std::move(session);
+    MakeWeak();
+  }
+
+  void Start() {
+    Debug(this->env(),
+          DebugCategory::COVERAGE,
+          "Sending Profiler.startPreciseCoverage\n");
+    Isolate* isolate = this->env()->isolate();
+    Local<Value> enable = FIXED_ONE_BYTE_STRING(
+        isolate, "{\"id\": 1, \"method\": \"Profiler.enable\"}");
+    Local<Value> start = FIXED_ONE_BYTE_STRING(
+        isolate,
+        "{"
+        "\"id\": 2,"
+        "\"method\": \"Profiler.startPreciseCoverage\","
+        "\"params\": {\"callCount\": true, \"detailed\": true}"
+        "}");
+    session_->Dispatch(ToProtocolString(isolate, enable)->string());
+    session_->Dispatch(ToProtocolString(isolate, start)->string());
+  }
+
+  void End() {
+    Debug(this->env(),
+          DebugCategory::COVERAGE,
+          "Sending Profiler.takePreciseCoverage\n");
+    Isolate* isolate = this->env()->isolate();
+    Local<Value> end =
+        FIXED_ONE_BYTE_STRING(isolate,
+                              "{"
+                              "\"id\": 3,"
+                              "\"method\": \"Profiler.takePreciseCoverage\""
+                              "}");
+    session_->Dispatch(ToProtocolString(isolate, end)->string());
+  }
+
+  friend class V8CoverageSessionDelegate;
+
+ private:
+  std::unique_ptr<inspector::InspectorSession> session_;
+};
+
+bool StartCoverageCollection(Environment* env) {
+  HandleScope scope(env->isolate());
+
+  Local<ObjectTemplate> t = ObjectTemplate::New(env->isolate());
+  t->SetInternalFieldCount(1);
+  Local<Object> obj;
+  if (!t->NewInstance(env->context()).ToLocal(&obj)) {
+    return false;
+  }
+
+  obj->SetAlignedPointerInInternalField(0, nullptr);
+
+  CHECK(env->coverage_connection().IsEmpty());
+  env->set_coverage_connection(obj);
+  V8CoverageConnection* connection = new V8CoverageConnection(env);
+  connection->Start();
+  return true;
+}
+
+static void EndCoverageCollection(const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+  CHECK(args[0]->IsFunction());
+  Debug(env, DebugCategory::COVERAGE, "Ending coverage collection\n");
+  env->set_on_coverage_message_function(args[0].As<Function>());
+  V8CoverageConnection* connection =
+      Unwrap<V8CoverageConnection>(env->coverage_connection());
+  CHECK_NOT_NULL(connection);
+  connection->End();
+}
+
+static void Initialize(Local<Object> target,
+                       Local<Value> unused,
+                       Local<Context> context,
+                       void* priv) {
+  Environment* env = Environment::GetCurrent(context);
+  env->SetMethod(target, "end", EndCoverageCollection);
+}
+}  // namespace coverage
+}  // namespace node
+
+NODE_MODULE_CONTEXT_AWARE_INTERNAL(coverage, node::coverage::Initialize)

src/node.cc

@@ -19,6 +19,7 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
+#include "debug_utils.h"
 #include "node_binding.h"
 #include "node_buffer.h"
 #include "node_constants.h"
@@ -230,6 +231,18 @@ MaybeLocal<Value> RunBootstrapping(Environment* env) {
   Isolate* isolate = env->isolate();
   Local<Context> context = env->context();
 
+  std::string coverage;
+  bool rc = credentials::SafeGetenv("NODE_V8_COVERAGE", &coverage);
+  if (rc && !coverage.empty()) {
+#if HAVE_INSPECTOR
+    if (!coverage::StartCoverageCollection(env)) {
+      return MaybeLocal<Value>();
+    }
+#else
+    fprintf(stderr, "NODE_V8_COVERAGE cannot be used without inspector");
+#endif  // HAVE_INSPECTOR
+  }
+
   // Add a reference to the global object
   Local<Object> global = context->Global();
 

src/node_binding.cc

@@ -21,6 +21,12 @@
 #define NODE_BUILTIN_REPORT_MODULES(V)
 #endif
 
+#if HAVE_INSPECTOR
+#define NODE_BUILTIN_COVERAGE_MODULES(V) V(coverage)
+#else
+#define NODE_BUILTIN_COVERAGE_MODULES(V)
+#endif
+
 // A list of built-in modules. In order to do module registration
 // in node::Init(), need to add built-in modules in the following list.
 // Then in binding::RegisterBuiltinModules(), it calls modules' registration
@@ -77,7 +83,8 @@
   NODE_BUILTIN_STANDARD_MODULES(V)                                             \
   NODE_BUILTIN_OPENSSL_MODULES(V)                                              \
   NODE_BUILTIN_ICU_MODULES(V)                                                  \
-  NODE_BUILTIN_REPORT_MODULES(V)
+  NODE_BUILTIN_REPORT_MODULES(V)                                               \
+  NODE_BUILTIN_COVERAGE_MODULES(V)
 
 // This is used to load built-in modules. Instead of using
 // __attribute__((constructor)), we call the _register_<modname>

src/node_internals.h

@@ -269,7 +269,9 @@ void DefineZlibConstants(v8::Local<v8::Object> target);
 v8::MaybeLocal<v8::Value> RunBootstrapping(Environment* env);
 v8::MaybeLocal<v8::Value> StartExecution(Environment* env,
                                          const char* main_script_id);
-
+namespace coverage {
+bool StartCoverageCollection(Environment* env);
+}
 }  // namespace node
 
 #endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS