From 8cf4170c94fecebf19751300c93138092869e808 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Fri, 22 Feb 2019 21:01:22 +0100
Subject: [PATCH] worker: provide process.execArgv
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Provide `process.execArgv`. If an `execArgv` option is passed to the
`Worker` constructor, that option is used as its value; if not,
the parent’s `process.execArgv` is inherited (since that also goes
for the actual options in that case).

PR-URL: https://github.com/nodejs/node/pull/26267
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 doc/api/worker_threads.md               |  5 ++-
 src/env-inl.h                           |  4 +++
 src/env.cc                              |  1 +
 src/env.h                               |  2 ++
 src/node_worker.cc                      | 16 ++++++---
 src/node_worker.h                       |  4 ++-
 test/parallel/test-process-exec-argv.js | 48 +++++++++++++++++--------
 test/parallel/test-worker-execargv.js   |  7 ++--
 8 files changed, 65 insertions(+), 22 deletions(-)

diff --git a/doc/api/worker_threads.md b/doc/api/worker_threads.md
index 54bd66a6ea683e..8d4c703f480de2 100644
--- a/doc/api/worker_threads.md
+++ b/doc/api/worker_threads.md
@@ -428,7 +428,9 @@ if (isMainThread) {
     not automatically be piped through to `process.stderr` in the parent.
   * `execArgv` {string[]} List of node CLI options passed to the worker.
     V8 options (such as `--max-old-space-size`) and options that affect the
-    process (such as `--title`) are not supported.
+    process (such as `--title`) are not supported. If set, this will be provided
+    as [`process.execArgv`][] inside the worker. By default, options will be
+    inherited from the parent thread.
 
 ### Event: 'error'
 <!-- YAML
@@ -582,6 +584,7 @@ active handle in the event system. If the worker is already `unref()`ed calling
 [`process.abort()`]: process.html#process_process_abort
 [`process.chdir()`]: process.html#process_process_chdir_directory
 [`process.env`]: process.html#process_process_env
+[`process.execArgv`]: process.html#process_process_execargv
 [`process.exit()`]: process.html#process_process_exit_code
 [`process.stderr`]: process.html#process_process_stderr
 [`process.stdin`]: process.html#process_process_stdin
diff --git a/src/env-inl.h b/src/env-inl.h
index 824b7529a9945d..b233d2fddfe997 100644
--- a/src/env-inl.h
+++ b/src/env-inl.h
@@ -605,6 +605,10 @@ inline std::shared_ptr<EnvironmentOptions> Environment::options() {
   return options_;
 }
 
+inline const std::vector<std::string>& Environment::exec_argv() {
+  return exec_argv_;
+}
+
 inline std::shared_ptr<HostPort> Environment::inspector_host_port() {
   return inspector_host_port_;
 }
diff --git a/src/env.cc b/src/env.cc
index 2e4ef80567c9fe..aec43a0312d9ca 100644
--- a/src/env.cc
+++ b/src/env.cc
@@ -386,6 +386,7 @@ MaybeLocal<Object> Environment::ProcessCliArgs(
                                       std::move(traced_value));
   }
 
+  exec_argv_ = exec_args;
   Local<Object> process_object =
       node::CreateProcessObject(this, args, exec_args)
           .FromMaybe(Local<Object>());
diff --git a/src/env.h b/src/env.h
index 87b2d3abd38cd8..8379171ac8b524 100644
--- a/src/env.h
+++ b/src/env.h
@@ -674,6 +674,7 @@ class Environment {
   v8::MaybeLocal<v8::Object> ProcessCliArgs(
       const std::vector<std::string>& args,
       const std::vector<std::string>& exec_args);
+  inline const std::vector<std::string>& exec_argv();
 
   typedef void (*HandleCleanupCb)(Environment* env,
                                   uv_handle_t* handle,
@@ -1064,6 +1065,7 @@ class Environment {
   // the inspector_host_port_->port() will be the actual port being
   // used.
   std::shared_ptr<HostPort> inspector_host_port_;
+  std::vector<std::string> exec_argv_;
 
   uint32_t module_id_counter_ = 0;
   uint32_t script_id_counter_ = 0;
diff --git a/src/node_worker.cc b/src/node_worker.cc
index 0f1535074cca6c..b6a56b9b94b08e 100644
--- a/src/node_worker.cc
+++ b/src/node_worker.cc
@@ -101,10 +101,12 @@ void AsyncRequest::MemoryInfo(MemoryTracker* tracker) const {
 Worker::Worker(Environment* env,
                Local<Object> wrap,
                const std::string& url,
-               std::shared_ptr<PerIsolateOptions> per_isolate_opts)
+               std::shared_ptr<PerIsolateOptions> per_isolate_opts,
+               std::vector<std::string>&& exec_argv)
     : AsyncWrap(env, wrap, AsyncWrap::PROVIDER_WORKER),
       url_(url),
       per_isolate_opts_(per_isolate_opts),
+      exec_argv_(exec_argv),
       platform_(env->isolate_data()->platform()),
       profiler_idle_notifier_started_(env->profiler_idle_notifier_started()),
       thread_id_(Environment::AllocateThreadId()) {
@@ -284,7 +286,7 @@ void Worker::Run() {
 
         env_->Start(profiler_idle_notifier_started_);
         env_->ProcessCliArgs(std::vector<std::string>{},
-                             std::vector<std::string>{});
+                             std::move(exec_argv_));
       }
 
       Debug(this, "Created Environment for worker with id %llu", thread_id_);
@@ -434,6 +436,9 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
   std::string url;
   std::shared_ptr<PerIsolateOptions> per_isolate_opts = nullptr;
 
+  std::vector<std::string> exec_argv_out;
+  bool has_explicit_exec_argv = false;
+
   // Argument might be a string or URL
   if (args.Length() > 0 && !args[0]->IsNullOrUndefined()) {
     Utf8Value value(
@@ -445,6 +450,7 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
       v8::Local<v8::Array> array = args[1].As<v8::Array>();
       // The first argument is reserved for program name, but we don't need it
       // in workers.
+      has_explicit_exec_argv = true;
       std::vector<std::string> exec_argv = {""};
       uint32_t length = array->Length();
       for (uint32_t i = 0; i < length; i++) {
@@ -472,7 +478,7 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
       // options for the per isolate parser.
       options_parser::PerIsolateOptionsParser::instance.Parse(
           &exec_argv,
-          nullptr,
+          &exec_argv_out,
           &invalid_args,
           per_isolate_opts.get(),
           kDisallowedInEnvironment,
@@ -492,7 +498,9 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
       }
     }
   }
-  new Worker(env, args.This(), url, per_isolate_opts);
+  if (!has_explicit_exec_argv)
+    exec_argv_out = env->exec_argv();
+  new Worker(env, args.This(), url, per_isolate_opts, std::move(exec_argv_out));
 }
 
 void Worker::StartThread(const FunctionCallbackInfo<Value>& args) {
diff --git a/src/node_worker.h b/src/node_worker.h
index fb94bdc307e8b6..442056eaac1078 100644
--- a/src/node_worker.h
+++ b/src/node_worker.h
@@ -38,7 +38,8 @@ class Worker : public AsyncWrap {
   Worker(Environment* env,
          v8::Local<v8::Object> wrap,
          const std::string& url,
-         std::shared_ptr<PerIsolateOptions> per_isolate_opts);
+         std::shared_ptr<PerIsolateOptions> per_isolate_opts,
+         std::vector<std::string>&& exec_argv);
   ~Worker() override;
 
   // Run the worker. This is only called from the worker thread.
@@ -74,6 +75,7 @@ class Worker : public AsyncWrap {
   const std::string url_;
 
   std::shared_ptr<PerIsolateOptions> per_isolate_opts_;
+  std::vector<std::string> exec_argv_;
   MultiIsolatePlatform* platform_;
   v8::Isolate* isolate_ = nullptr;
   bool profiler_idle_notifier_started_;
diff --git a/test/parallel/test-process-exec-argv.js b/test/parallel/test-process-exec-argv.js
index 578ee7b5d1a5e3..6321f151774665 100644
--- a/test/parallel/test-process-exec-argv.js
+++ b/test/parallel/test-process-exec-argv.js
@@ -20,26 +20,46 @@
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 'use strict';
-require('../common');
+const common = require('../common');
 const assert = require('assert');
 const spawn = require('child_process').spawn;
+const { Worker, isMainThread } = require('worker_threads');
 
-if (process.argv[2] === 'child') {
-  process.stdout.write(JSON.stringify(process.execArgv));
+if (process.argv[2] === 'child' || !isMainThread) {
+  if (process.argv[3] === 'cp+worker')
+    new Worker(__filename);
+  else
+    process.stdout.write(JSON.stringify(process.execArgv));
 } else {
   for (const extra of [ [], [ '--' ] ]) {
-    const execArgv = ['--stack-size=256'];
-    const args = [__filename, 'child', 'arg0'];
-    const child = spawn(process.execPath, [...execArgv, ...extra, ...args]);
-    let out = '';
+    for (const kind of [ 'cp', 'worker', 'cp+worker' ]) {
+      const execArgv = ['--pending-deprecation'];
+      const args = [__filename, 'child', kind];
+      let child;
+      switch (kind) {
+        case 'cp':
+          child = spawn(process.execPath, [...execArgv, ...extra, ...args]);
+          break;
+        case 'worker':
+          child = new Worker(__filename, {
+            execArgv: [...execArgv, ...extra],
+            stdout: true
+          });
+          break;
+        case 'cp+worker':
+          child = spawn(process.execPath, [...execArgv, ...args]);
+          break;
+      }
 
-    child.stdout.setEncoding('utf8');
-    child.stdout.on('data', function(chunk) {
-      out += chunk;
-    });
+      let out = '';
+      child.stdout.setEncoding('utf8');
+      child.stdout.on('data', (chunk) => {
+        out += chunk;
+      });
 
-    child.on('close', function() {
-      assert.deepStrictEqual(JSON.parse(out), execArgv);
-    });
+      child.stdout.on('end', common.mustCall(() => {
+        assert.deepStrictEqual(JSON.parse(out), execArgv);
+      }));
+    }
   }
 }
diff --git a/test/parallel/test-worker-execargv.js b/test/parallel/test-worker-execargv.js
index e04f015e03e2ff..16e46f2468dfe4 100644
--- a/test/parallel/test-worker-execargv.js
+++ b/test/parallel/test-worker-execargv.js
@@ -5,11 +5,13 @@ const assert = require('assert');
 // This test ensures that Workers have the ability to get
 // their own command line flags.
 
-const { Worker, isMainThread } = require('worker_threads');
+const { Worker } = require('worker_threads');
 const { StringDecoder } = require('string_decoder');
 const decoder = new StringDecoder('utf8');
 
-if (isMainThread) {
+// Do not use isMainThread so that this test itself can be run inside a Worker.
+if (!process.env.HAS_STARTED_WORKER) {
+  process.env.HAS_STARTED_WORKER = 1;
   const w = new Worker(__filename, { execArgv: ['--trace-warnings'] });
   w.stderr.on('data', common.mustCall((chunk) => {
     const error = decoder.write(chunk);
@@ -19,4 +21,5 @@ if (isMainThread) {
   }));
 } else {
   process.emitWarning('some warning');
+  assert.deepStrictEqual(process.execArgv, ['--trace-warnings']);
 }