worker: use copy of process.env

doc/api/process.md

@@ -954,6 +954,11 @@ emitMyWarning();
 <!-- YAML
 added: v0.1.27
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/26544
+    description: Worker threads will now use a copy of the parent thread’s
+                 `process.env` by default, configurable through the `env`
+                 option of the `Worker` constructor.
   - version: v10.0.0
     pr-url: https://github.com/nodejs/node/pull/18990
     description: Implicit conversion of variable value to string is deprecated.
@@ -983,8 +988,9 @@ An example of this object looks like:
 ```
 
 It is possible to modify this object, but such modifications will not be
-reflected outside the Node.js process. In other words, the following example
-would not work:
+reflected outside the Node.js process, or (unless explicitly requested)
+to other [`Worker`][] threads.
+In other words, the following example would not work:
 
 ```console
 $ node -e 'process.env.foo = "bar"' && echo $foo
@@ -1027,7 +1033,12 @@ console.log(process.env.test);
 // => 1
 ```
 
-`process.env` is read-only in [`Worker`][] threads.
+Unless explicitly specified when creating a [`Worker`][] instance,
+each [`Worker`][] thread has its own copy of `process.env`, based on its
+parent thread’s `process.env`, or whatever was specified as the `env` option
+to the [`Worker`][] constructor. Changes to `process.env` will not be visible
+across [`Worker`][] threads, and only the main thread can make changes that
+are visible to the operating system or to native add-ons.
 
 ## process.execArgv
 <!-- YAML

doc/api/worker_threads.md

@@ -125,6 +125,25 @@ if (isMainThread) {
 }
 ```
 
+## worker.SHARE_ENV
+<!-- YAML
+added: REPLACEME
+-->
+
+* {symbol}
+
+A special value that can be passed as the `env` option of the [`Worker`][]
+constructor, to indicate that the current thread and the Worker thread should
+share read and write access to the same set of environment variables.
+
+```js
+const { Worker, SHARE_ENV } = require('worker_threads');
+new Worker('process.env.SET_IN_WORKER = "foo"', { eval: true, env: SHARE_ENV })
+  .on('exit', () => {
+    console.log(process.env.SET_IN_WORKER);  // Prints 'foo'.
+  });
+```
+
 ## worker.threadId
 <!-- YAML
 added: v10.5.0
@@ -380,7 +399,11 @@ Notable differences inside a Worker environment are:
   and [`process.abort()`][] is not available.
 - [`process.chdir()`][] and `process` methods that set group or user ids
   are not available.
-- [`process.env`][] is a read-only reference to the environment variables.
+- [`process.env`][] is a copy of the parent thread's environment variables,
+  unless otherwise specified. Changes to one copy will not be visible in other
+  threads, and will not be visible to native add-ons (unless
+  [`worker.SHARE_ENV`][] has been passed as the `env` option to the
+  [`Worker`][] constructor).
 - [`process.title`][] cannot be modified.
 - Signals will not be delivered through [`process.on('...')`][Signals events].
 - Execution may stop at any point as a result of [`worker.terminate()`][]
@@ -439,25 +462,30 @@ if (isMainThread) {
   If `options.eval` is `true`, this is a string containing JavaScript code
   rather than a path.
 * `options` {Object}
+  * `env` {Object} If set, specifies the initial value of `process.env` inside
+    the Worker thread. As a special value, [`worker.SHARE_ENV`][] may be used
+    to specify that the parent thread and the child thread should share their
+    environment variables; in that case, changes to one thread’s `process.env`
+    object will affect the other thread as well. **Default:** `process.env`.
   * `eval` {boolean} If `true`, interpret the first argument to the constructor
     as a script that is executed once the worker is online.
-  * `workerData` {any} Any JavaScript value that will be cloned and made
-    available as [`require('worker_threads').workerData`][]. The cloning will
-    occur as described in the [HTML structured clone algorithm][], and an error
-    will be thrown if the object cannot be cloned (e.g. because it contains
-    `function`s).
+  * `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. If set, this will be provided
+    as [`process.execArgv`][] inside the worker. By default, options will be
+    inherited from the parent thread.
   * `stdin` {boolean} If this is set to `true`, then `worker.stdin` will
     provide a writable stream whose contents will appear as `process.stdin`
     inside the Worker. By default, no data is provided.
   * `stdout` {boolean} If this is set to `true`, then `worker.stdout` will
     not automatically be piped through to `process.stdout` in the parent.
   * `stderr` {boolean} If this is set to `true`, then `worker.stderr` will
     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. If set, this will be provided
-    as [`process.execArgv`][] inside the worker. By default, options will be
-    inherited from the parent thread.
+  * `workerData` {any} Any JavaScript value that will be cloned and made
+    available as [`require('worker_threads').workerData`][]. The cloning will
+    occur as described in the [HTML structured clone algorithm][], and an error
+    will be thrown if the object cannot be cloned (e.g. because it contains
+    `function`s).
 
 ### Event: 'error'
 <!-- YAML
@@ -628,6 +656,7 @@ active handle in the event system. If the worker is already `unref()`ed calling
 [`vm`]: vm.html
 [`worker.on('message')`]: #worker_threads_event_message_1
 [`worker.postMessage()`]: #worker_threads_worker_postmessage_value_transferlist
+[`worker.SHARE_ENV`]: #worker_threads_worker_share_env
 [`worker.terminate()`]: #worker_threads_worker_terminate_callback
 [`worker.threadId`]: #worker_threads_worker_threadid_1
 [Addons worker support]: addons.html#addons_worker_support

lib/internal/worker.js

@@ -45,6 +45,8 @@ const kOnCouldNotSerializeErr = Symbol('kOnCouldNotSerializeErr');
 const kOnErrorMessage = Symbol('kOnErrorMessage');
 const kParentSideStdio = Symbol('kParentSideStdio');
 
+const SHARE_ENV = Symbol.for('nodejs.worker_threads.SHARE_ENV');
+
 let debuglog;
 function debug(...args) {
   if (!debuglog) {
@@ -79,12 +81,33 @@ class Worker extends EventEmitter {
       }
     }
 
+    let env;
+    if (typeof options.env === 'object' && options.env !== null) {
+      env = Object.create(null);
+      for (const [ key, value ] of Object.entries(options.env))
+        env[key] = `${value}`;
+    } else if (options.env == null) {
+      env = process.env;
+    } else if (options.env !== SHARE_ENV) {
+      throw new ERR_INVALID_ARG_TYPE(
+        'options.env',
+        ['object', 'undefined', 'null', 'worker_threads.SHARE_ENV'],
+        options.env);
+    }
+
     const url = options.eval ? null : pathToFileURL(filename);
     // Set up the C++ handle for the worker, as well as some internal wiring.
     this[kHandle] = new WorkerImpl(url, options.execArgv);
     if (this[kHandle].invalidExecArgv) {
       throw new ERR_WORKER_INVALID_EXEC_ARGV(this[kHandle].invalidExecArgv);
     }
+    if (env === process.env) {
+      // This may be faster than manually cloning the object in C++, especially
+      // when recursively spawning Workers.
+      this[kHandle].cloneParentEnvVars();
+    } else if (env !== undefined) {
+      this[kHandle].setEnvVars(env);
+    }
     this[kHandle].onexit = (code) => this[kOnExit](code);
     this[kPort] = this[kHandle].messagePort;
     this[kPort].on('message', (data) => this[kOnMessage](data));
@@ -253,6 +276,7 @@ function pipeWithoutWarning(source, dest) {
 module.exports = {
   ownsProcessState,
   isMainThread,
+  SHARE_ENV,
   threadId,
   Worker,
 };

lib/worker_threads.js

@@ -2,6 +2,7 @@
 
 const {
   isMainThread,
+  SHARE_ENV,
   threadId,
   Worker
 } = require('internal/worker');
@@ -18,6 +19,7 @@ module.exports = {
   MessageChannel,
   moveMessagePortToContext,
   threadId,
+  SHARE_ENV,
   Worker,
   parentPort: null,
   workerData: null,

src/env-inl.h

@@ -447,6 +447,14 @@ inline uint64_t Environment::timer_base() const {
   return timer_base_;
 }
 
+inline std::shared_ptr<KVStore> Environment::env_vars() {
+  return env_vars_;
+}
+
+inline void Environment::set_env_vars(std::shared_ptr<KVStore> env_vars) {
+  env_vars_ = env_vars;
+}
+
 inline bool Environment::printed_error() const {
   return printed_error_;
 }

src/env.cc

@@ -178,6 +178,8 @@ Environment::Environment(IsolateData* isolate_data,
     set_as_callback_data_template(templ);
   }
 
+  set_env_vars(per_process::system_environment);
+
   // We create new copies of the per-Environment option sets, so that it is
   // easier to modify them after Environment creation. The defaults are
   // part of the per-Isolate option set, for which in turn the defaults are
@@ -221,7 +223,7 @@ Environment::Environment(IsolateData* isolate_data,
   should_abort_on_uncaught_toggle_[0] = 1;
 
   std::string debug_cats;
-  credentials::SafeGetenv("NODE_DEBUG_NATIVE", &debug_cats);
+  credentials::SafeGetenv("NODE_DEBUG_NATIVE", &debug_cats, this);
   set_debug_categories(debug_cats, true);
 
   isolate()->GetHeapProfiler()->AddBuildEmbedderGraphCallback(

src/env.h

@@ -540,6 +540,29 @@ class AsyncRequest : public MemoryRetainer {
   std::atomic_bool stopped_ {true};
 };
 
+class KVStore {
+ public:
+  virtual v8::Local<v8::String> Get(v8::Isolate* isolate,
+                                    v8::Local<v8::String> key) const = 0;
+  virtual void Set(v8::Isolate* isolate,
+                   v8::Local<v8::String> key,
+                   v8::Local<v8::String> value) = 0;
+  virtual int32_t Query(v8::Isolate* isolate,
+                        v8::Local<v8::String> key) const = 0;
+  virtual void Delete(v8::Isolate* isolate, v8::Local<v8::String> key) = 0;
+  virtual v8::Local<v8::Array> Enumerate(v8::Isolate* isolate) const = 0;
+
+  virtual std::shared_ptr<KVStore> Clone(v8::Isolate* isolate) const;
+  virtual v8::Maybe<bool> AssignFromObject(v8::Local<v8::Context> context,
+                                           v8::Local<v8::Object> entries);
+
+  static std::shared_ptr<KVStore> CreateMapKVStore();
+};
+
+namespace per_process {
+extern std::shared_ptr<KVStore> system_environment;
+}
+
 class AsyncHooks {
  public:
   // Reason for both UidFields and Fields are that one is stored as a double*
@@ -789,6 +812,8 @@ class Environment {
   inline ImmediateInfo* immediate_info();
   inline TickInfo* tick_info();
   inline uint64_t timer_base() const;
+  inline std::shared_ptr<KVStore> env_vars();
+  inline void set_env_vars(std::shared_ptr<KVStore> env_vars);
 
   inline IsolateData* isolate_data() const;
 
@@ -1075,6 +1100,7 @@ class Environment {
   ImmediateInfo immediate_info_;
   TickInfo tick_info_;
   const uint64_t timer_base_;
+  std::shared_ptr<KVStore> env_vars_;
   bool printed_error_ = false;
   bool emit_env_nonstring_warning_ = true;
   bool emit_err_name_warning_ = true;

src/node_credentials.cc

@@ -15,11 +15,14 @@ using v8::Array;
 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::String;
+using v8::TryCatch;
 using v8::Uint32;
 using v8::Value;
 
@@ -30,13 +33,27 @@ bool linux_at_secure = false;
 namespace credentials {
 
 // Look up environment variable unless running as setuid root.
-bool SafeGetenv(const char* key, std::string* text) {
+bool SafeGetenv(const char* key, std::string* text, Environment* env) {
 #if !defined(__CloudABI__) && !defined(_WIN32)
   if (per_process::linux_at_secure || getuid() != geteuid() ||
       getgid() != getegid())
     goto fail;
 #endif
 
+  if (env != nullptr) {
+    HandleScope handle_scope(env->isolate());
+    TryCatch ignore_errors(env->isolate());
+    MaybeLocal<String> value = env->env_vars()->Get(
+        env->isolate(),
+        String::NewFromUtf8(env->isolate(), key, NewStringType::kNormal)
+            .ToLocalChecked());
+    if (value.IsEmpty()) goto fail;
+    String::Utf8Value utf8_value(env->isolate(), value.ToLocalChecked());
+    if (*utf8_value == nullptr) goto fail;
+    *text = std::string(*utf8_value, utf8_value.length());
+    return true;
+  }
+
   {
     Mutex::ScopedLock lock(per_process::env_var_mutex);
     if (const char* value = getenv(key)) {
@@ -52,10 +69,11 @@ bool SafeGetenv(const char* key, std::string* text) {
 
 static void SafeGetenv(const FunctionCallbackInfo<Value>& args) {
   CHECK(args[0]->IsString());
-  Isolate* isolate = args.GetIsolate();
+  Environment* env = Environment::GetCurrent(args);
+  Isolate* isolate = env->isolate();
   Utf8Value strenvtag(isolate, args[0]);
   std::string text;
-  if (!SafeGetenv(*strenvtag, &text)) return;
+  if (!SafeGetenv(*strenvtag, &text, env)) return;
   Local<Value> result =
       ToV8Value(isolate->GetCurrentContext(), text).ToLocalChecked();
   args.GetReturnValue().Set(result);