worker: use copy of process.env

Instead of sharing the OS-backed store for all `process.env` instances,
create a copy of `process.env` for every worker that is created.

The copies do not interact. Native-addons do not see modifications to
`process.env` from Worker threads, but child processes started from
Workers do default to the Worker’s copy of `process.env`.

This makes Workers behave like child processes as far as `process.env`
is concerned, and an option corresponding to the `child_process`
module’s `env` option is added to the constructor.

Fixes: https://github.com/nodejs/node/issues/24947

PR-URL: https://github.com/nodejs/node/pull/26544
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

7 files changed, 63 insertions, 37 deletions

diff --git a/src/env-inl.h b/src/env-inl.h
index 3eedbb857b..ef13ffdcc0 100644
--- a/src/env-inl.h
+++ b/src/env-inl.h
@@ -447,12 +447,12 @@ inline uint64_t Environment::timer_base() const {
   return timer_base_;
 }
 
-inline std::shared_ptr<KVStore> Environment::envvars() {
-  return envvars_;
+inline std::shared_ptr<KVStore> Environment::env_vars() {
+  return env_vars_;
 }
 
-inline void Environment::set_envvars(std::shared_ptr<KVStore> envvars) {
-  envvars_ = envvars;
+inline void Environment::set_env_vars(std::shared_ptr<KVStore> env_vars) {
+  env_vars_ = env_vars;
 }
 
 inline bool Environment::printed_error() const {
diff --git a/src/env.cc b/src/env.cc
index ffdb4289bb..46f807a3c4 100644
--- a/src/env.cc
+++ b/src/env.cc
@@ -178,7 +178,7 @@ Environment::Environment(IsolateData* isolate_data,
     set_as_callback_data_template(templ);
   }
 
-  set_envvars(per_process::real_environment);
+  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
diff --git a/src/env.h b/src/env.h
index 4ed48a3342..f22d57171d 100644
--- a/src/env.h
+++ b/src/env.h
@@ -556,11 +556,11 @@ class KVStore {
   virtual v8::Maybe<bool> AssignFromObject(v8::Local<v8::Context> context,
                                            v8::Local<v8::Object> entries);
 
-  static std::shared_ptr<KVStore> CreateGenericKVStore();
+  static std::shared_ptr<KVStore> CreateMapKVStore();
 };
 
 namespace per_process {
-extern std::shared_ptr<KVStore> real_environment;
+extern std::shared_ptr<KVStore> system_environment;
 }
 
 class AsyncHooks {
@@ -812,8 +812,8 @@ class Environment {
   inline ImmediateInfo* immediate_info();
   inline TickInfo* tick_info();
   inline uint64_t timer_base() const;
-  inline std::shared_ptr<KVStore> envvars();
-  inline void set_envvars(std::shared_ptr<KVStore> envvars);
+  inline std::shared_ptr<KVStore> env_vars();
+  inline void set_env_vars(std::shared_ptr<KVStore> env_vars);
 
   inline IsolateData* isolate_data() const;
 
@@ -1100,7 +1100,7 @@ class Environment {
   ImmediateInfo immediate_info_;
   TickInfo tick_info_;
   const uint64_t timer_base_;
-  std::shared_ptr<KVStore> envvars_;
+  std::shared_ptr<KVStore> env_vars_;
   bool printed_error_ = false;
   bool emit_env_nonstring_warning_ = true;
   bool emit_err_name_warning_ = true;
diff --git a/src/node_credentials.cc b/src/node_credentials.cc
index 23b9ad2893..765d1caba9 100644
--- a/src/node_credentials.cc
+++ b/src/node_credentials.cc
@@ -43,7 +43,7 @@ bool SafeGetenv(const char* key, std::string* text, Environment* env) {
   if (env != nullptr) {
     HandleScope handle_scope(env->isolate());
     TryCatch ignore_errors(env->isolate());
-    MaybeLocal<String> value = env->envvars()->Get(
+    MaybeLocal<String> value = env->env_vars()->Get(
         env->isolate(),
         String::NewFromUtf8(env->isolate(), key, NewStringType::kNormal)
             .ToLocalChecked());
diff --git a/src/node_env_var.cc b/src/node_env_var.cc
index 2df7275b17..b7b862304d 100644
--- a/src/node_env_var.cc
+++ b/src/node_env_var.cc
@@ -40,7 +40,7 @@ class RealEnvStore final : public KVStore {
   Local<Array> Enumerate(Isolate* isolate) const override;
 };
 
-class GenericKVStore final : public KVStore {
+class MapKVStore final : public KVStore {
  public:
   Local<String> Get(Isolate* isolate, Local<String> key) const override;
   void Set(Isolate* isolate, Local<String> key, Local<String> value) override;
@@ -50,8 +50,8 @@ class GenericKVStore final : public KVStore {
 
   std::shared_ptr<KVStore> Clone(Isolate* isolate) const override;
 
-  GenericKVStore() {}
-  GenericKVStore(const GenericKVStore& other) : map_(other.map_) {}
+  MapKVStore() {}
+  MapKVStore(const MapKVStore& other) : map_(other.map_) {}
 
  private:
   mutable Mutex mutex_;
@@ -60,7 +60,7 @@ class GenericKVStore final : public KVStore {
 
 namespace per_process {
 Mutex env_var_mutex;
-std::shared_ptr<KVStore> real_environment = std::make_shared<RealEnvStore>();
+std::shared_ptr<KVStore> system_environment = std::make_shared<RealEnvStore>();
 }  // namespace per_process
 
 Local<String> RealEnvStore::Get(Isolate* isolate,
@@ -207,7 +207,7 @@ std::shared_ptr<KVStore> KVStore::Clone(v8::Isolate* isolate) const {
   HandleScope handle_scope(isolate);
   Local<Context> context = isolate->GetCurrentContext();
 
-  std::shared_ptr<KVStore> copy = KVStore::CreateGenericKVStore();
+  std::shared_ptr<KVStore> copy = KVStore::CreateMapKVStore();
   Local<Array> keys = Enumerate(isolate);
   uint32_t keys_length = keys->Length();
   for (uint32_t i = 0; i < keys_length; i++) {
@@ -218,9 +218,9 @@ std::shared_ptr<KVStore> KVStore::Clone(v8::Isolate* isolate) const {
   return copy;
 }
 
-Local<String> GenericKVStore::Get(Isolate* isolate, Local<String> key) const {
+Local<String> MapKVStore::Get(Isolate* isolate, Local<String> key) const {
   Mutex::ScopedLock lock(mutex_);
-  String::Utf8Value str(isolate, key);
+  Utf8Value str(isolate, key);
   auto it = map_.find(std::string(*str, str.length()));
   if (it == map_.end()) return Local<String>();
   return String::NewFromUtf8(isolate, it->second.data(),
@@ -228,31 +228,30 @@ Local<String> GenericKVStore::Get(Isolate* isolate, Local<String> key) const {
       .ToLocalChecked();
 }
 
-void GenericKVStore::Set(Isolate* isolate, Local<String> key,
-                         Local<String> value) {
+void MapKVStore::Set(Isolate* isolate, Local<String> key, Local<String> value) {
   Mutex::ScopedLock lock(mutex_);
-  String::Utf8Value key_str(isolate, key);
-  String::Utf8Value value_str(isolate, value);
+  Utf8Value key_str(isolate, key);
+  Utf8Value value_str(isolate, value);
   if (*key_str != nullptr && *value_str != nullptr) {
     map_[std::string(*key_str, key_str.length())] =
         std::string(*value_str, value_str.length());
   }
 }
 
-int32_t GenericKVStore::Query(Isolate* isolate, Local<String> key) const {
+int32_t MapKVStore::Query(Isolate* isolate, Local<String> key) const {
   Mutex::ScopedLock lock(mutex_);
-  String::Utf8Value str(isolate, key);
+  Utf8Value str(isolate, key);
   auto it = map_.find(std::string(*str, str.length()));
   return it == map_.end() ? -1 : 0;
 }
 
-void GenericKVStore::Delete(Isolate* isolate, Local<String> key) {
+void MapKVStore::Delete(Isolate* isolate, Local<String> key) {
   Mutex::ScopedLock lock(mutex_);
-  String::Utf8Value str(isolate, key);
+  Utf8Value str(isolate, key);
   map_.erase(std::string(*str, str.length()));
 }
 
-Local<Array> GenericKVStore::Enumerate(Isolate* isolate) const {
+Local<Array> MapKVStore::Enumerate(Isolate* isolate) const {
   Mutex::ScopedLock lock(mutex_);
   std::vector<Local<Value>> values;
   values.reserve(map_.size());
@@ -265,12 +264,12 @@ Local<Array> GenericKVStore::Enumerate(Isolate* isolate) const {
   return Array::New(isolate, values.data(), values.size());
 }
 
-std::shared_ptr<KVStore> GenericKVStore::Clone(Isolate* isolate) const {
-  return std::make_shared<GenericKVStore>(*this);
+std::shared_ptr<KVStore> MapKVStore::Clone(Isolate* isolate) const {
+  return std::make_shared<MapKVStore>(*this);
 }
 
-std::shared_ptr<KVStore> KVStore::CreateGenericKVStore() {
-  return std::make_shared<GenericKVStore>();
+std::shared_ptr<KVStore> KVStore::CreateMapKVStore() {
+  return std::make_shared<MapKVStore>();
 }
 
 Maybe<bool> KVStore::AssignFromObject(Local<Context> context,
@@ -307,7 +306,7 @@ static void EnvGetter(Local<Name> property,
   }
   CHECK(property->IsString());
   info.GetReturnValue().Set(
-      env->envvars()->Get(env->isolate(), property.As<String>()));
+      env->env_vars()->Get(env->isolate(), property.As<String>()));
 }
 
 static void EnvSetter(Local<Name> property,
@@ -338,7 +337,7 @@ static void EnvSetter(Local<Name> property,
     return;
   }
 
-  env->envvars()->Set(env->isolate(), key, value_string);
+  env->env_vars()->Set(env->isolate(), key, value_string);
 
   // Whether it worked or not, always return value.
   info.GetReturnValue().Set(value);
@@ -348,7 +347,7 @@ static void EnvQuery(Local<Name> property,
                      const PropertyCallbackInfo<Integer>& info) {
   Environment* env = Environment::GetCurrent(info);
   if (property->IsString()) {
-    int32_t rc = env->envvars()->Query(env->isolate(), property.As<String>());
+    int32_t rc = env->env_vars()->Query(env->isolate(), property.As<String>());
     if (rc != -1) info.GetReturnValue().Set(rc);
   }
 }
@@ -357,7 +356,7 @@ static void EnvDeleter(Local<Name> property,
                        const PropertyCallbackInfo<Boolean>& info) {
   Environment* env = Environment::GetCurrent(info);
   if (property->IsString()) {
-    env->envvars()->Delete(env->isolate(), property.As<String>());
+    env->env_vars()->Delete(env->isolate(), property.As<String>());
   }
 
   // process.env never has non-configurable properties, so always
@@ -369,7 +368,7 @@ static void EnvEnumerator(const PropertyCallbackInfo<Array>& info) {
   Environment* env = Environment::GetCurrent(info);
 
   info.GetReturnValue().Set(
-      env->envvars()->Enumerate(env->isolate()));
+      env->env_vars()->Enumerate(env->isolate()));
 }
 
 MaybeLocal<Object> CreateEnvVarProxy(Local<Context> context,
diff --git a/src/node_worker.cc b/src/node_worker.cc
index d18d8023a2..8fabbffba4 100644
--- a/src/node_worker.cc
+++ b/src/node_worker.cc
@@ -68,7 +68,8 @@ Worker::Worker(Environment* env,
       exec_argv_(exec_argv),
       platform_(env->isolate_data()->platform()),
       profiler_idle_notifier_started_(env->profiler_idle_notifier_started()),
-      thread_id_(Environment::AllocateThreadId()) {
+      thread_id_(Environment::AllocateThreadId()),
+      env_vars_(env->env_vars()) {
   Debug(this, "Creating new worker instance with thread id %llu", thread_id_);
 
   // Set up everything that needs to be set up in the parent environment.
@@ -254,6 +255,7 @@ void Worker::Run() {
                                    Environment::kNoFlags,
                                    thread_id_));
         CHECK_NOT_NULL(env_);
+        env_->set_env_vars(std::move(env_vars_));
         env_->set_abort_on_uncaught_exception(false);
         env_->set_worker_context(this);
 
@@ -469,6 +471,25 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
   new Worker(env, args.This(), url, per_isolate_opts, std::move(exec_argv_out));
 }
 
+void Worker::CloneParentEnvVars(const FunctionCallbackInfo<Value>& args) {
+  Worker* w;
+  ASSIGN_OR_RETURN_UNWRAP(&w, args.This());
+  CHECK(w->thread_joined_);  // The Worker has not started yet.
+
+  w->env_vars_ = w->env()->env_vars()->Clone(args.GetIsolate());
+}
+
+void Worker::SetEnvVars(const FunctionCallbackInfo<Value>& args) {
+  Worker* w;
+  ASSIGN_OR_RETURN_UNWRAP(&w, args.This());
+  CHECK(w->thread_joined_);  // The Worker has not started yet.
+
+  CHECK(args[0]->IsObject());
+  w->env_vars_ = KVStore::CreateMapKVStore();
+  w->env_vars_->AssignFromObject(args.GetIsolate()->GetCurrentContext(),
+                                args[0].As<Object>());
+}
+
 void Worker::StartThread(const FunctionCallbackInfo<Value>& args) {
   Worker* w;
   ASSIGN_OR_RETURN_UNWRAP(&w, args.This());
@@ -566,6 +587,8 @@ void InitWorker(Local<Object> target,
     w->InstanceTemplate()->SetInternalFieldCount(1);
     w->Inherit(AsyncWrap::GetConstructorTemplate(env));
 
+    env->SetProtoMethod(w, "setEnvVars", Worker::SetEnvVars);
+    env->SetProtoMethod(w, "cloneParentEnvVars", Worker::CloneParentEnvVars);
     env->SetProtoMethod(w, "startThread", Worker::StartThread);
     env->SetProtoMethod(w, "stopThread", Worker::StopThread);
     env->SetProtoMethod(w, "ref", Worker::Ref);
diff --git a/src/node_worker.h b/src/node_worker.h
index b23ab704b0..94af8160c6 100644
--- a/src/node_worker.h
+++ b/src/node_worker.h
@@ -43,6 +43,9 @@ class Worker : public AsyncWrap {
   bool is_stopped() const;
 
   static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
+  static void CloneParentEnvVars(
+      const v8::FunctionCallbackInfo<v8::Value>& args);
+  static void SetEnvVars(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void StartThread(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void StopThread(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Ref(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -78,6 +81,7 @@ class Worker : public AsyncWrap {
   static constexpr size_t kStackBufferSize = 192 * 1024;
 
   std::unique_ptr<MessagePortData> child_port_data_;
+  std::shared_ptr<KVStore> env_vars_;
 
   // The child port is kept alive by the child Environment's persistent
   // handle to it, as long as that child Environment exists.