Skip to content

Commit 8bd7909

Browse files
addaleaxBethGriggs
authored andcommitted
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: #24947 PR-URL: #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> Signed-off-by: Beth Griggs <Bethany.Griggs@uk.ibm.com>
1 parent 729e2f2 commit 8bd7909

13 files changed

+229
-51
lines changed

doc/api/process.md

+14-3
Original file line numberDiff line numberDiff line change
@@ -954,6 +954,11 @@ emitMyWarning();
954954
<!-- YAML
955955
added: v0.1.27
956956
changes:
957+
- version: REPLACEME
958+
pr-url: https://github.com/nodejs/node/pull/26544
959+
description: Worker threads will now use a copy of the parent thread’s
960+
`process.env` by default, configurable through the `env`
961+
option of the `Worker` constructor.
957962
- version: v10.0.0
958963
pr-url: https://github.com/nodejs/node/pull/18990
959964
description: Implicit conversion of variable value to string is deprecated.
@@ -983,8 +988,9 @@ An example of this object looks like:
983988
```
984989

985990
It is possible to modify this object, but such modifications will not be
986-
reflected outside the Node.js process. In other words, the following example
987-
would not work:
991+
reflected outside the Node.js process, or (unless explicitly requested)
992+
to other [`Worker`][] threads.
993+
In other words, the following example would not work:
988994

989995
```console
990996
$ node -e 'process.env.foo = "bar"' && echo $foo
@@ -1027,7 +1033,12 @@ console.log(process.env.test);
10271033
// => 1
10281034
```
10291035

1030-
`process.env` is read-only in [`Worker`][] threads.
1036+
Unless explicitly specified when creating a [`Worker`][] instance,
1037+
each [`Worker`][] thread has its own copy of `process.env`, based on its
1038+
parent thread’s `process.env`, or whatever was specified as the `env` option
1039+
to the [`Worker`][] constructor. Changes to `process.env` will not be visible
1040+
across [`Worker`][] threads, and only the main thread can make changes that
1041+
are visible to the operating system or to native add-ons.
10311042

10321043
## process.execArgv
10331044
<!-- YAML

doc/api/worker_threads.md

+40-11
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,25 @@ if (isMainThread) {
125125
}
126126
```
127127

128+
## worker.SHARE_ENV
129+
<!-- YAML
130+
added: REPLACEME
131+
-->
132+
133+
* {symbol}
134+
135+
A special value that can be passed as the `env` option of the [`Worker`][]
136+
constructor, to indicate that the current thread and the Worker thread should
137+
share read and write access to the same set of environment variables.
138+
139+
```js
140+
const { Worker, SHARE_ENV } = require('worker_threads');
141+
new Worker('process.env.SET_IN_WORKER = "foo"', { eval: true, env: SHARE_ENV })
142+
.on('exit', () => {
143+
console.log(process.env.SET_IN_WORKER); // Prints 'foo'.
144+
});
145+
```
146+
128147
## worker.threadId
129148
<!-- YAML
130149
added: v10.5.0
@@ -380,7 +399,11 @@ Notable differences inside a Worker environment are:
380399
and [`process.abort()`][] is not available.
381400
- [`process.chdir()`][] and `process` methods that set group or user ids
382401
are not available.
383-
- [`process.env`][] is a read-only reference to the environment variables.
402+
- [`process.env`][] is a copy of the parent thread's environment variables,
403+
unless otherwise specified. Changes to one copy will not be visible in other
404+
threads, and will not be visible to native add-ons (unless
405+
[`worker.SHARE_ENV`][] has been passed as the `env` option to the
406+
[`Worker`][] constructor).
384407
- [`process.title`][] cannot be modified.
385408
- Signals will not be delivered through [`process.on('...')`][Signals events].
386409
- Execution may stop at any point as a result of [`worker.terminate()`][]
@@ -439,25 +462,30 @@ if (isMainThread) {
439462
If `options.eval` is `true`, this is a string containing JavaScript code
440463
rather than a path.
441464
* `options` {Object}
465+
* `env` {Object} If set, specifies the initial value of `process.env` inside
466+
the Worker thread. As a special value, [`worker.SHARE_ENV`][] may be used
467+
to specify that the parent thread and the child thread should share their
468+
environment variables; in that case, changes to one thread’s `process.env`
469+
object will affect the other thread as well. **Default:** `process.env`.
442470
* `eval` {boolean} If `true`, interpret the first argument to the constructor
443471
as a script that is executed once the worker is online.
444-
* `workerData` {any} Any JavaScript value that will be cloned and made
445-
available as [`require('worker_threads').workerData`][]. The cloning will
446-
occur as described in the [HTML structured clone algorithm][], and an error
447-
will be thrown if the object cannot be cloned (e.g. because it contains
448-
`function`s).
472+
* `execArgv` {string[]} List of node CLI options passed to the worker.
473+
V8 options (such as `--max-old-space-size`) and options that affect the
474+
process (such as `--title`) are not supported. If set, this will be provided
475+
as [`process.execArgv`][] inside the worker. By default, options will be
476+
inherited from the parent thread.
449477
* `stdin` {boolean} If this is set to `true`, then `worker.stdin` will
450478
provide a writable stream whose contents will appear as `process.stdin`
451479
inside the Worker. By default, no data is provided.
452480
* `stdout` {boolean} If this is set to `true`, then `worker.stdout` will
453481
not automatically be piped through to `process.stdout` in the parent.
454482
* `stderr` {boolean} If this is set to `true`, then `worker.stderr` will
455483
not automatically be piped through to `process.stderr` in the parent.
456-
* `execArgv` {string[]} List of node CLI options passed to the worker.
457-
V8 options (such as `--max-old-space-size`) and options that affect the
458-
process (such as `--title`) are not supported. If set, this will be provided
459-
as [`process.execArgv`][] inside the worker. By default, options will be
460-
inherited from the parent thread.
484+
* `workerData` {any} Any JavaScript value that will be cloned and made
485+
available as [`require('worker_threads').workerData`][]. The cloning will
486+
occur as described in the [HTML structured clone algorithm][], and an error
487+
will be thrown if the object cannot be cloned (e.g. because it contains
488+
`function`s).
461489

462490
### Event: 'error'
463491
<!-- YAML
@@ -628,6 +656,7 @@ active handle in the event system. If the worker is already `unref()`ed calling
628656
[`vm`]: vm.html
629657
[`worker.on('message')`]: #worker_threads_event_message_1
630658
[`worker.postMessage()`]: #worker_threads_worker_postmessage_value_transferlist
659+
[`worker.SHARE_ENV`]: #worker_threads_worker_share_env
631660
[`worker.terminate()`]: #worker_threads_worker_terminate_callback
632661
[`worker.threadId`]: #worker_threads_worker_threadid_1
633662
[Addons worker support]: addons.html#addons_worker_support

lib/internal/worker.js

+24
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ const kOnCouldNotSerializeErr = Symbol('kOnCouldNotSerializeErr');
4545
const kOnErrorMessage = Symbol('kOnErrorMessage');
4646
const kParentSideStdio = Symbol('kParentSideStdio');
4747

48+
const SHARE_ENV = Symbol.for('nodejs.worker_threads.SHARE_ENV');
49+
4850
let debuglog;
4951
function debug(...args) {
5052
if (!debuglog) {
@@ -79,12 +81,33 @@ class Worker extends EventEmitter {
7981
}
8082
}
8183

84+
let env;
85+
if (typeof options.env === 'object' && options.env !== null) {
86+
env = Object.create(null);
87+
for (const [ key, value ] of Object.entries(options.env))
88+
env[key] = `${value}`;
89+
} else if (options.env == null) {
90+
env = process.env;
91+
} else if (options.env !== SHARE_ENV) {
92+
throw new ERR_INVALID_ARG_TYPE(
93+
'options.env',
94+
['object', 'undefined', 'null', 'worker_threads.SHARE_ENV'],
95+
options.env);
96+
}
97+
8298
const url = options.eval ? null : pathToFileURL(filename);
8399
// Set up the C++ handle for the worker, as well as some internal wiring.
84100
this[kHandle] = new WorkerImpl(url, options.execArgv);
85101
if (this[kHandle].invalidExecArgv) {
86102
throw new ERR_WORKER_INVALID_EXEC_ARGV(this[kHandle].invalidExecArgv);
87103
}
104+
if (env === process.env) {
105+
// This may be faster than manually cloning the object in C++, especially
106+
// when recursively spawning Workers.
107+
this[kHandle].cloneParentEnvVars();
108+
} else if (env !== undefined) {
109+
this[kHandle].setEnvVars(env);
110+
}
88111
this[kHandle].onexit = (code) => this[kOnExit](code);
89112
this[kPort] = this[kHandle].messagePort;
90113
this[kPort].on('message', (data) => this[kOnMessage](data));
@@ -253,6 +276,7 @@ function pipeWithoutWarning(source, dest) {
253276
module.exports = {
254277
ownsProcessState,
255278
isMainThread,
279+
SHARE_ENV,
256280
threadId,
257281
Worker,
258282
};

lib/worker_threads.js

+2
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
const {
44
isMainThread,
5+
SHARE_ENV,
56
threadId,
67
Worker
78
} = require('internal/worker');
@@ -18,6 +19,7 @@ module.exports = {
1819
MessageChannel,
1920
moveMessagePortToContext,
2021
threadId,
22+
SHARE_ENV,
2123
Worker,
2224
parentPort: null,
2325
workerData: null,

src/env-inl.h

+4-4
Original file line numberDiff line numberDiff line change
@@ -447,12 +447,12 @@ inline uint64_t Environment::timer_base() const {
447447
return timer_base_;
448448
}
449449

450-
inline std::shared_ptr<KVStore> Environment::envvars() {
451-
return envvars_;
450+
inline std::shared_ptr<KVStore> Environment::env_vars() {
451+
return env_vars_;
452452
}
453453

454-
inline void Environment::set_envvars(std::shared_ptr<KVStore> envvars) {
455-
envvars_ = envvars;
454+
inline void Environment::set_env_vars(std::shared_ptr<KVStore> env_vars) {
455+
env_vars_ = env_vars;
456456
}
457457

458458
inline bool Environment::printed_error() const {

src/env.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ Environment::Environment(IsolateData* isolate_data,
207207
set_as_callback_data_template(templ);
208208
}
209209

210-
set_envvars(per_process::real_environment);
210+
set_env_vars(per_process::system_environment);
211211

212212
// We create new copies of the per-Environment option sets, so that it is
213213
// easier to modify them after Environment creation. The defaults are

src/env.h

+5-5
Original file line numberDiff line numberDiff line change
@@ -548,11 +548,11 @@ class KVStore {
548548
virtual v8::Maybe<bool> AssignFromObject(v8::Local<v8::Context> context,
549549
v8::Local<v8::Object> entries);
550550

551-
static std::shared_ptr<KVStore> CreateGenericKVStore();
551+
static std::shared_ptr<KVStore> CreateMapKVStore();
552552
};
553553

554554
namespace per_process {
555-
extern std::shared_ptr<KVStore> real_environment;
555+
extern std::shared_ptr<KVStore> system_environment;
556556
}
557557

558558
class AsyncHooks {
@@ -804,8 +804,8 @@ class Environment {
804804
inline ImmediateInfo* immediate_info();
805805
inline TickInfo* tick_info();
806806
inline uint64_t timer_base() const;
807-
inline std::shared_ptr<KVStore> envvars();
808-
inline void set_envvars(std::shared_ptr<KVStore> envvars);
807+
inline std::shared_ptr<KVStore> env_vars();
808+
inline void set_env_vars(std::shared_ptr<KVStore> env_vars);
809809

810810
inline IsolateData* isolate_data() const;
811811

@@ -1100,7 +1100,7 @@ class Environment {
11001100
ImmediateInfo immediate_info_;
11011101
TickInfo tick_info_;
11021102
const uint64_t timer_base_;
1103-
std::shared_ptr<KVStore> envvars_;
1103+
std::shared_ptr<KVStore> env_vars_;
11041104
bool printed_error_ = false;
11051105
bool emit_env_nonstring_warning_ = true;
11061106
bool emit_err_name_warning_ = true;

src/node_credentials.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ bool SafeGetenv(const char* key, std::string* text, Environment* env) {
4343
if (env != nullptr) {
4444
HandleScope handle_scope(env->isolate());
4545
TryCatch ignore_errors(env->isolate());
46-
MaybeLocal<String> value = env->envvars()->Get(
46+
MaybeLocal<String> value = env->env_vars()->Get(
4747
env->isolate(),
4848
String::NewFromUtf8(env->isolate(), key, NewStringType::kNormal)
4949
.ToLocalChecked());

0 commit comments

Comments
 (0)