Skip to content

Commit

Permalink
worker: use copy of process.env
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
  • Loading branch information
addaleax authored and BethGriggs committed Apr 4, 2019
1 parent 786a1d4 commit fc3a0da
Show file tree
Hide file tree
Showing 13 changed files with 229 additions and 51 deletions.
17 changes: 14 additions & 3 deletions doc/api/process.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
51 changes: 40 additions & 11 deletions doc/api/worker_threads.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()`][]
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
24 changes: 24 additions & 0 deletions lib/internal/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -253,6 +276,7 @@ function pipeWithoutWarning(source, dest) {
module.exports = {
ownsProcessState,
isMainThread,
SHARE_ENV,
threadId,
Worker,
};
2 changes: 2 additions & 0 deletions lib/worker_threads.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const {
isMainThread,
SHARE_ENV,
threadId,
Worker
} = require('internal/worker');
Expand All @@ -18,6 +19,7 @@ module.exports = {
MessageChannel,
moveMessagePortToContext,
threadId,
SHARE_ENV,
Worker,
parentPort: null,
workerData: null,
Expand Down
8 changes: 4 additions & 4 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/node_credentials.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Loading

0 comments on commit fc3a0da

Please sign in to comment.