process: patch more process properties during pre-execution

Delay the creation of process properties that depend on runtime states and properties that should not be accessed during bootstrap and patch them during pre-execution: - process.argv - process.execPath - process.title - process.pid - process.ppid - process.REVERT_* - process.debugPort PR-URL: #26945 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
nodejs · Apr 3, 2019 · 9dba96d · 9dba96d
1 parent d005f38
commit 9dba96d
Show file tree

Hide file tree

Showing 7 changed files with 73 additions and 64 deletions.
diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js
@@ -3,9 +3,9 @@
 const { getOptionValue } = require('internal/options');
 const { Buffer } = require('buffer');
 
-function prepareMainThreadExecution() {
+function prepareMainThreadExecution(expandArgv1 = false) {
   // Patch the process object with legacy properties and normalizations
-  patchProcessObject();
+  patchProcessObject(expandArgv1);
   setupTraceCategoryState();
   setupInspectorHooks();
   setupWarningHandler();
@@ -48,14 +48,26 @@ function prepareMainThreadExecution() {
   loadPreloadModules();
 }
 
-function patchProcessObject() {
+function patchProcessObject(expandArgv1) {
+  const {
+    patchProcessObject: patchProcessObjectNative
+  } = internalBinding('process_methods');
+
+  patchProcessObjectNative(process);
+
   Object.defineProperty(process, 'argv0', {
     enumerable: true,
     configurable: false,
     value: process.argv[0]
   });
   process.argv[0] = process.execPath;
 
+  if (expandArgv1 && process.argv[1] && !process.argv[1].startsWith('-')) {
+    // Expand process.argv[1] into a full path.
+    const path = require('path');
+    process.argv[1] = path.resolve(process.argv[1]);
+  }
+
   // TODO(joyeecheung): most of these should be deprecated and removed,
   // execpt some that we need to be able to mutate during run time.
   addReadOnlyProcessAlias('_eval', '--eval');

diff --git a/lib/internal/main/check_syntax.js b/lib/internal/main/check_syntax.js
@@ -18,20 +18,18 @@ const {
   stripShebang, stripBOM
 } = require('internal/modules/cjs/helpers');
 
-let CJSModule;
-function CJSModuleInit() {
-  if (!CJSModule)
-    CJSModule = require('internal/modules/cjs/loader');
-}
+// TODO(joyeecheung): not every one of these are necessary
+prepareMainThreadExecution(true);
 
 if (process.argv[1] && process.argv[1] !== '-') {
   // Expand process.argv[1] into a full path.
   const path = require('path');
   process.argv[1] = path.resolve(process.argv[1]);
 
-  // TODO(joyeecheung): not every one of these are necessary
-  prepareMainThreadExecution();
-  CJSModuleInit();
+  // This has to be done after prepareMainThreadExecution because it
+  // relies on process.execPath
+  const CJSModule = require('internal/modules/cjs/loader');
+
   // Read the source.
   const filename = CJSModule._resolveFilename(process.argv[1]);
 
@@ -42,9 +40,6 @@ if (process.argv[1] && process.argv[1] !== '-') {
 
   checkSyntax(source, filename);
 } else {
-  // TODO(joyeecheung): not every one of these are necessary
-  prepareMainThreadExecution();
-  CJSModuleInit();
   markBootstrapComplete();
 
   readStdin((code) => {
@@ -56,6 +51,10 @@ function checkSyntax(source, filename) {
   // Remove Shebang.
   source = stripShebang(source);
 
+  // This has to be done after prepareMainThreadExecution because it
+  // relies on process.execPath
+  const CJSModule = require('internal/modules/cjs/loader');
+
   const { getOptionValue } = require('internal/options');
   const experimentalModules = getOptionValue('--experimental-modules');
   if (experimentalModules) {

diff --git a/lib/internal/main/run_main_module.js b/lib/internal/main/run_main_module.js
@@ -4,11 +4,7 @@ const {
   prepareMainThreadExecution
 } = require('internal/bootstrap/pre_execution');
 
-// Expand process.argv[1] into a full path.
-const path = require('path');
-process.argv[1] = path.resolve(process.argv[1]);
-
-prepareMainThreadExecution();
+prepareMainThreadExecution(true);
 
 const CJSModule = require('internal/modules/cjs/loader');
 

diff --git a/lib/internal/process/warning.js b/lib/internal/process/warning.js
@@ -1,6 +1,5 @@
 'use strict';
 
-const prefix = `(${process.release.name}:${process.pid}) `;
 const { ERR_INVALID_ARG_TYPE } = require('internal/errors').codes;
 
 // Lazily loaded
@@ -55,7 +54,7 @@ function onWarning(warning) {
   if (isDeprecation && process.noDeprecation) return;
   const trace = process.traceProcessWarnings ||
                 (isDeprecation && process.traceDeprecation);
-  var msg = prefix;
+  var msg = `(${process.release.name}:${process.pid}) `;
   if (warning.code)
     msg += `[${warning.code}] `;
   if (trace && warning.stack) {

diff --git a/src/node_process.h b/src/node_process.h
@@ -35,7 +35,7 @@ v8::MaybeLocal<v8::Object> CreateProcessObject(
     Environment* env,
     const std::vector<std::string>& args,
     const std::vector<std::string>& exec_args);
-
+void PatchProcessObject(const v8::FunctionCallbackInfo<v8::Value>& args);
 }  // namespace node
 #endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 #endif  // SRC_NODE_PROCESS_H_
diff --git a/src/node_process_methods.cc b/src/node_process_methods.cc
@@ -426,6 +426,7 @@ static void InitializeProcessMethods(Local<Object> target,
   env->SetMethod(target, "dlopen", binding::DLOpen);
   env->SetMethod(target, "reallyExit", ReallyExit);
   env->SetMethodNoSideEffect(target, "uptime", Uptime);
+  env->SetMethod(target, "patchProcessObject", PatchProcessObject);
 }
 
 }  // namespace node

diff --git a/src/node_process_object.cc b/src/node_process_object.cc
@@ -13,6 +13,7 @@ using v8::Context;
 using v8::DEFAULT;
 using v8::EscapableHandleScope;
 using v8::Function;
+using v8::FunctionCallbackInfo;
 using v8::FunctionTemplate;
 using v8::HandleScope;
 using v8::Integer;
@@ -84,20 +85,6 @@ MaybeLocal<Object> CreateProcessObject(
     return MaybeLocal<Object>();
   }
 
-  // process.title
-  auto title_string = FIXED_ONE_BYTE_STRING(env->isolate(), "title");
-  CHECK(process
-            ->SetAccessor(
-                env->context(),
-                title_string,
-                ProcessTitleGetter,
-                env->owns_process_state() ? ProcessTitleSetter : nullptr,
-                env->as_callback_data(),
-                DEFAULT,
-                None,
-                SideEffectType::kHasNoSideEffect)
-            .FromJust());
-
   // process.version
   READONLY_PROPERTY(process,
                     "version",
@@ -140,34 +127,56 @@ MaybeLocal<Object> CreateProcessObject(
 #endif  // _WIN32
 #endif  // NODE_HAS_RELEASE_URLS
 
+  // process._rawDebug: may be overwritten later in JS land, but should be
+  // availbale from the begining for debugging purposes
+  env->SetMethod(process, "_rawDebug", RawDebug);
+
+  return scope.Escape(process);
+}
+
+void PatchProcessObject(const FunctionCallbackInfo<Value>& args) {
+  Isolate* isolate = args.GetIsolate();
+  Local<Context> context = isolate->GetCurrentContext();
+  Environment* env = Environment::GetCurrent(context);
+  CHECK(args[0]->IsObject());
+  Local<Object> process = args[0].As<Object>();
+
+  // process.title
+  CHECK(process
+            ->SetAccessor(
+                context,
+                FIXED_ONE_BYTE_STRING(isolate, "title"),
+                ProcessTitleGetter,
+                env->owns_process_state() ? ProcessTitleSetter : nullptr,
+                env->as_callback_data(),
+                DEFAULT,
+                None,
+                SideEffectType::kHasNoSideEffect)
+            .FromJust());
+
   // process.argv
-  process->Set(env->context(),
-               FIXED_ONE_BYTE_STRING(env->isolate(), "argv"),
-               ToV8Value(env->context(), args).ToLocalChecked()).FromJust();
+  process->Set(context,
+               FIXED_ONE_BYTE_STRING(isolate, "argv"),
+               ToV8Value(context, env->argv()).ToLocalChecked()).FromJust();
 
   // process.execArgv
-  process->Set(env->context(),
-               FIXED_ONE_BYTE_STRING(env->isolate(), "execArgv"),
-               ToV8Value(env->context(), exec_args)
+  process->Set(context,
+               FIXED_ONE_BYTE_STRING(isolate, "execArgv"),
+               ToV8Value(context, env->exec_argv())
                    .ToLocalChecked()).FromJust();
 
   READONLY_PROPERTY(process, "pid",
-                    Integer::New(env->isolate(), uv_os_getpid()));
+                    Integer::New(isolate, uv_os_getpid()));
 
-  CHECK(process->SetAccessor(env->context(),
-                             FIXED_ONE_BYTE_STRING(env->isolate(), "ppid"),
+  CHECK(process->SetAccessor(context,
+                             FIXED_ONE_BYTE_STRING(isolate, "ppid"),
                              GetParentProcessId).FromJust());
 
-  // TODO(joyeecheung): make this available in JS during pre-execution.
-  // Note that to use this in releases the code doing the revert need to be
-  // careful to delay the check until after the bootstrap but that may not
-  // be possible depending on the feature being reverted.
-
   // --security-revert flags
 #define V(code, _, __)                                                        \
   do {                                                                        \
     if (IsReverted(SECURITY_REVERT_ ## code)) {                               \
-      READONLY_PROPERTY(process, "REVERT_" #code, True(env->isolate()));      \
+      READONLY_PROPERTY(process, "REVERT_" #code, True(isolate));             \
     }                                                                         \
   } while (0);
   SECURITY_REVERSIONS(V)
@@ -181,7 +190,7 @@ MaybeLocal<Object> CreateProcessObject(
     if (uv_exepath(exec_path_buf, &exec_path_len) == 0) {
       exec_path = std::string(exec_path_buf, exec_path_len);
     } else {
-      exec_path = args[0];
+      exec_path = env->argv()[0];
     }
     // On OpenBSD process.execPath will be relative unless we
     // get the full path before process.execPath is used.
@@ -196,9 +205,9 @@ MaybeLocal<Object> CreateProcessObject(
     uv_fs_req_cleanup(&req);
 #endif
     process
-        ->Set(env->context(),
-              FIXED_ONE_BYTE_STRING(env->isolate(), "execPath"),
-              String::NewFromUtf8(env->isolate(),
+        ->Set(context,
+              FIXED_ONE_BYTE_STRING(isolate, "execPath"),
+              String::NewFromUtf8(isolate,
                                   exec_path.c_str(),
                                   NewStringType::kInternalized,
                                   exec_path.size())
@@ -207,20 +216,13 @@ MaybeLocal<Object> CreateProcessObject(
   }
 
   // process.debugPort
-  auto debug_port_string = FIXED_ONE_BYTE_STRING(env->isolate(), "debugPort");
   CHECK(process
-            ->SetAccessor(env->context(),
-                          debug_port_string,
+            ->SetAccessor(context,
+                          FIXED_ONE_BYTE_STRING(isolate, "debugPort"),
                           DebugPortGetter,
                           env->owns_process_state() ? DebugPortSetter : nullptr,
                           env->as_callback_data())
             .FromJust());
-
-  // process._rawDebug: may be overwritten later in JS land, but should be
-  // availbale from the begining for debugging purposes
-  env->SetMethod(process, "_rawDebug", RawDebug);
-
-  return scope.Escape(process);
 }
 
 }  // namespace node