src: simplify embedder entry point execution

Previously we wrapped the embedder entry point callback into a
binding and then invoke the binding from JS land which was a bit
convoluted. Now we just call it directly from C++.
The main scripts that needed to tail call the embedder callback now
return the arguments in an array so that the C++ land can extract
the arguments and pass them to the callback. We also set
`PauseOnNextJavascriptStatement()` for --inspect-brk and mark
the bootstrap complete milestone directly in C++ for these
execution modes.

PR-URL: nodejs#51557
Reviewed-By: Daniel Lemire <daniel@lemire.me>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

lib/internal/main/embedding.js

@@ -1,18 +1,15 @@
 'use strict';
 const {
   prepareMainThreadExecution,
-  markBootstrapComplete,
 } = require('internal/process/pre_execution');
 const { isExperimentalSeaWarningNeeded } = internalBinding('sea');
 const { emitExperimentalWarning } = require('internal/util');
 const { embedderRequire, embedderRunCjs } = require('internal/util/embedding');
-const { runEmbedderEntryPoint } = internalBinding('mksnapshot');
 
 prepareMainThreadExecution(false, true);
-markBootstrapComplete();
 
 if (isExperimentalSeaWarningNeeded()) {
   emitExperimentalWarning('Single executable application');
 }
 
-return runEmbedderEntryPoint(process, embedderRequire, embedderRunCjs);
+return [process, embedderRequire, embedderRunCjs];

lib/internal/main/mksnapshot.js

@@ -11,7 +11,6 @@ const {
 
 const { BuiltinModule: { normalizeRequirableId } } = require('internal/bootstrap/realm');
 const {
-  runEmbedderEntryPoint,
   compileSerializeMain,
   anonymousMainPath,
 } = internalBinding('mksnapshot');
@@ -20,9 +19,6 @@ const { isExperimentalSeaWarningNeeded } = internalBinding('sea');
 
 const { emitExperimentalWarning } = require('internal/util');
 const { emitWarningSync } = require('internal/process/warning');
-const {
-  getOptionValue,
-} = require('internal/options');
 
 const {
   initializeCallbacks,
@@ -179,18 +175,11 @@ function main() {
     return fn(requireForUserSnapshot, filename, dirname);
   }
 
-  const serializeMainArgs = [process, requireForUserSnapshot, minimalRunCjs];
-
   if (isExperimentalSeaWarningNeeded()) {
     emitExperimentalWarning('Single executable application');
   }
 
-  if (getOptionValue('--inspect-brk')) {
-    internalBinding('inspector').callAndPauseOnStart(
-      runEmbedderEntryPoint, undefined, ...serializeMainArgs);
-  } else {
-    runEmbedderEntryPoint(...serializeMainArgs);
-  }
+  return [process, requireForUserSnapshot, minimalRunCjs];
 }
 
-main();
+return main();

src/env-inl.h

@@ -430,14 +430,6 @@ inline builtins::BuiltinLoader* Environment::builtin_loader() {
   return &builtin_loader_;
 }
 
-inline const StartExecutionCallback& Environment::embedder_entry_point() const {
-  return embedder_entry_point_;
-}
-
-inline void Environment::set_embedder_entry_point(StartExecutionCallback&& fn) {
-  embedder_entry_point_ = std::move(fn);
-}
-
 inline double Environment::new_async_id() {
   async_hooks()->async_id_fields()[AsyncHooks::kAsyncIdCounter] += 1;
   return async_hooks()->async_id_fields()[AsyncHooks::kAsyncIdCounter];

src/env.h

@@ -999,9 +999,6 @@ class Environment : public MemoryRetainer {
 
 #endif  // HAVE_INSPECTOR
 
-  inline const StartExecutionCallback& embedder_entry_point() const;
-  inline void set_embedder_entry_point(StartExecutionCallback&& fn);
-
   inline void set_process_exit_handler(
       std::function<void(Environment*, ExitCode)>&& handler);
 
@@ -1207,7 +1204,6 @@ class Environment : public MemoryRetainer {
   std::unique_ptr<PrincipalRealm> principal_realm_ = nullptr;
 
   builtins::BuiltinLoader builtin_loader_;
-  StartExecutionCallback embedder_entry_point_;
 
   // Used by allocate_managed_buffer() and release_managed_buffer() to keep
   // track of the BackingStore for a given pointer.

src/node.cc

@@ -131,7 +131,10 @@
 
 namespace node {
 
+using v8::Array;
+using v8::Context;
 using v8::EscapableHandleScope;
+using v8::Function;
 using v8::Isolate;
 using v8::Local;
 using v8::MaybeLocal;
@@ -282,26 +285,65 @@ MaybeLocal<Value> StartExecution(Environment* env, const char* main_script_id) {
   return scope.EscapeMaybe(realm->ExecuteBootstrapper(main_script_id));
 }
 
+// Convert the result returned by an intermediate main script into
+// StartExecutionCallbackInfo. Currently the result is an array containing
+// [process, requireFunction, cjsRunner]
+std::optional<StartExecutionCallbackInfo> CallbackInfoFromArray(
+    Local<Context> context, Local<Value> result) {
+  CHECK(result->IsArray());
+  Local<Array> args = result.As<Array>();
+  CHECK_EQ(args->Length(), 3);
+  Local<Value> process_obj, require_fn, runcjs_fn;
+  if (!args->Get(context, 0).ToLocal(&process_obj) ||
+      !args->Get(context, 1).ToLocal(&require_fn) ||
+      !args->Get(context, 2).ToLocal(&runcjs_fn)) {
+    return std::nullopt;
+  }
+  CHECK(process_obj->IsObject());
+  CHECK(require_fn->IsFunction());
+  CHECK(runcjs_fn->IsFunction());
+  node::StartExecutionCallbackInfo info{process_obj.As<Object>(),
+                                        require_fn.As<Function>(),
+                                        runcjs_fn.As<Function>()};
+  return info;
+}
+
 MaybeLocal<Value> StartExecution(Environment* env, StartExecutionCallback cb) {
   InternalCallbackScope callback_scope(
       env,
       Object::New(env->isolate()),
       { 1, 0 },
       InternalCallbackScope::kSkipAsyncHooks);
 
+  // Only snapshot builder or embedder applications set the
+  // callback.
   if (cb != nullptr) {
     EscapableHandleScope scope(env->isolate());
-    // TODO(addaleax): pass the callback to the main script more directly,
-    // e.g. by making StartExecution(env, builtin) parametrizable
-    env->set_embedder_entry_point(std::move(cb));
-    auto reset_entry_point =
-        OnScopeLeave([&]() { env->set_embedder_entry_point({}); });
 
-    const char* entry = env->isolate_data()->is_building_snapshot()
-                            ? "internal/main/mksnapshot"
-                            : "internal/main/embedding";
+    Local<Value> result;
+    if (env->isolate_data()->is_building_snapshot()) {
+      if (!StartExecution(env, "internal/main/mksnapshot").ToLocal(&result)) {
+        return MaybeLocal<Value>();
+      }
+    } else {
+      if (!StartExecution(env, "internal/main/embedding").ToLocal(&result)) {
+        return MaybeLocal<Value>();
+      }
+    }
+
+    auto info = CallbackInfoFromArray(env->context(), result);
+    if (!info.has_value()) {
+      MaybeLocal<Value>();
+    }
+#if HAVE_INSPECTOR
+    if (env->options()->debug_options().break_first_line) {
+      env->inspector_agent()->PauseOnNextJavascriptStatement("Break on start");
+    }
+#endif
 
-    return scope.EscapeMaybe(StartExecution(env, entry));
+    env->performance_state()->Mark(
+        performance::NODE_PERFORMANCE_MILESTONE_BOOTSTRAP_COMPLETE);
+    return scope.EscapeMaybe(cb(info.value()));
   }
 
   CHECK(!env->isolate_data()->is_building_snapshot());

src/node_snapshotable.cc

@@ -41,7 +41,6 @@ using v8::FunctionCallbackInfo;
 using v8::HandleScope;
 using v8::Isolate;
 using v8::Local;
-using v8::MaybeLocal;
 using v8::Object;
 using v8::ObjectTemplate;
 using v8::ScriptCompiler;
@@ -1411,25 +1410,6 @@ void SerializeSnapshotableObjects(Realm* realm,
   });
 }
 
-static void RunEmbedderEntryPoint(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-  Local<Value> process_obj = args[0];
-  Local<Value> require_fn = args[1];
-  Local<Value> runcjs_fn = args[2];
-  CHECK(process_obj->IsObject());
-  CHECK(require_fn->IsFunction());
-  CHECK(runcjs_fn->IsFunction());
-
-  const node::StartExecutionCallback& callback = env->embedder_entry_point();
-  node::StartExecutionCallbackInfo info{process_obj.As<Object>(),
-                                        require_fn.As<Function>(),
-                                        runcjs_fn.As<Function>()};
-  MaybeLocal<Value> retval = callback(info);
-  if (!retval.IsEmpty()) {
-    args.GetReturnValue().Set(retval.ToLocalChecked());
-  }
-}
-
 void CompileSerializeMain(const FunctionCallbackInfo<Value>& args) {
   CHECK(args[0]->IsString());
   Local<String> filename = args[0].As<String>();
@@ -1553,7 +1533,6 @@ void CreatePerContextProperties(Local<Object> target,
 void CreatePerIsolateProperties(IsolateData* isolate_data,
                                 Local<ObjectTemplate> target) {
   Isolate* isolate = isolate_data->isolate();
-  SetMethod(isolate, target, "runEmbedderEntryPoint", RunEmbedderEntryPoint);
   SetMethod(isolate, target, "compileSerializeMain", CompileSerializeMain);
   SetMethod(isolate, target, "setSerializeCallback", SetSerializeCallback);
   SetMethod(isolate, target, "setDeserializeCallback", SetDeserializeCallback);
@@ -1566,7 +1545,6 @@ void CreatePerIsolateProperties(IsolateData* isolate_data,
 }
 
 void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
-  registry->Register(RunEmbedderEntryPoint);
   registry->Register(CompileSerializeMain);
   registry->Register(SetSerializeCallback);
   registry->Register(SetDeserializeCallback);