Skip to content

Commit 40fcf9b

Browse files
committed
child_process: build the default env block in one native pass
When spawn()/spawnSync() are called without options.env, normalizeSpawnArguments() copied process.env with a spread and then walked the copy to build the KEY=value array uv_spawn() takes. Spreading the process.env proxy costs one enumerator callback plus a query and a getter interceptor per variable, each doing a linear getenv() scan and allocating; with a couple of hundred variables that was the single largest JS-side cost of spawning a process. Add KVStore::Pairs() (Enumerate() + Get() by default, one uv_os_environ() pass for the real environment, skipping hidden variables on Windows exactly like Enumerate() does), expose it as process_wrap.getEnvPairs(), and use it for the default-environment case. A user supplied options.env and the permission model case keep the existing code. On Windows the same sort/first-wins-case-insensitive filter is applied to the pairs. The variables copyProcessEnvToEnv() propagates are part of the real environment by definition, and its entries cannot contain null bytes, so those steps only remain on the options.env path.
1 parent 30bff4a commit 40fcf9b

5 files changed

Lines changed: 245 additions & 50 deletions

File tree

‎lib/child_process.js‎

Lines changed: 95 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ const {
4040
RegExpPrototypeExec,
4141
SafeSet,
4242
StringPrototypeIncludes,
43+
StringPrototypeIndexOf,
4344
StringPrototypeSlice,
4445
StringPrototypeToUpperCase,
4546
SymbolDispose,
@@ -93,6 +94,8 @@ const {
9394
stdioStringToArray,
9495
} = child_process;
9596

97+
const { getEnvPairs } = internalBinding('process_wrap');
98+
9699
const MAX_BUFFER = 1024 * 1024;
97100

98101
const permission = require('internal/process/permission');
@@ -697,60 +700,74 @@ function normalizeSpawnArguments(file, args, options) {
697700
ArrayPrototypeUnshift(args, file);
698701
}
699702

700-
// Shallow copy to guarantee changes won't impact process.env
701-
const env = options.env || { ...process.env };
702-
const envPairs = [];
703-
704-
// process.env.NODE_V8_COVERAGE always propagates, making it possible to
705-
// collect coverage for programs that spawn with white-listed environment.
706-
copyProcessEnvToEnv(env, 'NODE_V8_COVERAGE', options.env);
707-
708-
if (isZOS) {
709-
// The following environment variables must always propagate if set.
710-
copyProcessEnvToEnv(env, '_BPXK_AUTOCVT', options.env);
711-
copyProcessEnvToEnv(env, '_CEE_RUNOPTS', options.env);
712-
copyProcessEnvToEnv(env, '_TAG_REDIR_ERR', options.env);
713-
copyProcessEnvToEnv(env, '_TAG_REDIR_IN', options.env);
714-
copyProcessEnvToEnv(env, '_TAG_REDIR_OUT', options.env);
715-
copyProcessEnvToEnv(env, 'STEPLIB', options.env);
716-
copyProcessEnvToEnv(env, 'LIBPATH', options.env);
717-
copyProcessEnvToEnv(env, '_EDC_SIG_DFLT', options.env);
718-
copyProcessEnvToEnv(env, '_EDC_SUSV3', options.env);
719-
}
703+
let envPairs;
704+
if (!options.env && !permission.isEnabled()) {
705+
// Default: the child inherits this process's environment. Take it as
706+
// 'KEY=value' strings in one native pass over the environment block
707+
// instead of copying process.env (one interceptor round trip and one
708+
// getenv() scan per variable). Everything copyProcessEnvToEnv() would
709+
// propagate below is part of it by definition, and entries of the real
710+
// environment block cannot contain null bytes.
711+
envPairs = getEnvPairs();
712+
if (process.platform === 'win32') {
713+
envPairs = dedupeWindowsEnvPairs(envPairs);
714+
}
715+
} else {
716+
// Shallow copy to guarantee changes won't impact process.env
717+
const env = options.env || { ...process.env };
718+
envPairs = [];
719+
720+
// process.env.NODE_V8_COVERAGE always propagates, making it possible to
721+
// collect coverage for programs that spawn with white-listed environment.
722+
copyProcessEnvToEnv(env, 'NODE_V8_COVERAGE', options.env);
723+
724+
if (isZOS) {
725+
// The following environment variables must always propagate if set.
726+
copyProcessEnvToEnv(env, '_BPXK_AUTOCVT', options.env);
727+
copyProcessEnvToEnv(env, '_CEE_RUNOPTS', options.env);
728+
copyProcessEnvToEnv(env, '_TAG_REDIR_ERR', options.env);
729+
copyProcessEnvToEnv(env, '_TAG_REDIR_IN', options.env);
730+
copyProcessEnvToEnv(env, '_TAG_REDIR_OUT', options.env);
731+
copyProcessEnvToEnv(env, 'STEPLIB', options.env);
732+
copyProcessEnvToEnv(env, 'LIBPATH', options.env);
733+
copyProcessEnvToEnv(env, '_EDC_SIG_DFLT', options.env);
734+
copyProcessEnvToEnv(env, '_EDC_SUSV3', options.env);
735+
}
720736

721-
if (permission.isEnabled()) {
722-
copyPermissionModelFlagsToEnv(env, 'NODE_OPTIONS', args);
723-
}
737+
if (permission.isEnabled()) {
738+
copyPermissionModelFlagsToEnv(env, 'NODE_OPTIONS', args);
739+
}
724740

725-
let envKeys = [];
726-
// Prototype values are intentionally included.
727-
for (const key in env) {
728-
ArrayPrototypePush(envKeys, key);
729-
}
741+
let envKeys = [];
742+
// Prototype values are intentionally included.
743+
for (const key in env) {
744+
ArrayPrototypePush(envKeys, key);
745+
}
730746

731-
if (process.platform === 'win32') {
732-
// On Windows env keys are case insensitive. Filter out duplicates,
733-
// keeping only the first one (in lexicographic order)
734-
const sawKey = new SafeSet();
735-
envKeys = ArrayPrototypeFilter(
736-
ArrayPrototypeSort(envKeys),
737-
(key) => {
738-
const uppercaseKey = StringPrototypeToUpperCase(key);
739-
if (sawKey.has(uppercaseKey)) {
740-
return false;
741-
}
742-
sawKey.add(uppercaseKey);
743-
return true;
744-
},
745-
);
746-
}
747+
if (process.platform === 'win32') {
748+
// On Windows env keys are case insensitive. Filter out duplicates,
749+
// keeping only the first one (in lexicographic order)
750+
const sawKey = new SafeSet();
751+
envKeys = ArrayPrototypeFilter(
752+
ArrayPrototypeSort(envKeys),
753+
(key) => {
754+
const uppercaseKey = StringPrototypeToUpperCase(key);
755+
if (sawKey.has(uppercaseKey)) {
756+
return false;
757+
}
758+
sawKey.add(uppercaseKey);
759+
return true;
760+
},
761+
);
762+
}
747763

748-
for (const key of envKeys) {
749-
const value = env[key];
750-
if (value !== undefined) {
751-
validateArgumentNullCheck(key, `options.env['${key}']`);
752-
validateArgumentNullCheck(value, `options.env['${key}']`);
753-
ArrayPrototypePush(envPairs, `${key}=${value}`);
764+
for (const key of envKeys) {
765+
const value = env[key];
766+
if (value !== undefined) {
767+
validateArgumentNullCheck(key, `options.env['${key}']`);
768+
validateArgumentNullCheck(value, `options.env['${key}']`);
769+
ArrayPrototypePush(envPairs, `${key}=${value}`);
770+
}
754771
}
755772
}
756773

@@ -768,6 +785,34 @@ function normalizeSpawnArguments(file, args, options) {
768785
};
769786
}
770787

788+
/**
789+
* Windows environment variable names are case-insensitive: keep only the
790+
* first entry for each name in lexicographic order of the names, exactly like
791+
* the key filtering applied to a user-supplied `options.env`.
792+
* @param {string[]} envPairs 'KEY=value' strings
793+
* @returns {string[]}
794+
*/
795+
function dedupeWindowsEnvPairs(envPairs) {
796+
const keyed = [];
797+
for (let i = 0; i < envPairs.length; i++) {
798+
const pair = envPairs[i];
799+
// Names never start with '=' here (hidden variables are not enumerated),
800+
// so the first '=' ends the name.
801+
const eq = StringPrototypeIndexOf(pair, '=');
802+
ArrayPrototypePush(keyed, { key: eq === -1 ? pair : StringPrototypeSlice(pair, 0, eq), pair });
803+
}
804+
ArrayPrototypeSort(keyed, (a, b) => (a.key < b.key ? -1 : a.key > b.key ? 1 : 0));
805+
const sawKey = new SafeSet();
806+
const result = [];
807+
for (let i = 0; i < keyed.length; i++) {
808+
const uppercaseKey = StringPrototypeToUpperCase(keyed[i].key);
809+
if (sawKey.has(uppercaseKey)) continue;
810+
sawKey.add(uppercaseKey);
811+
ArrayPrototypePush(result, keyed[i].pair);
812+
}
813+
return result;
814+
}
815+
771816
function abortChildProcess(child, killSignal, reason) {
772817
if (!child)
773818
return;

‎src/node_env_var.cc‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ using v8::Boolean;
1515
using v8::Context;
1616
using v8::DontDelete;
1717
using v8::DontEnum;
18+
using v8::EscapableHandleScope;
1819
using v8::FunctionTemplate;
1920
using v8::HandleScope;
2021
using v8::IndexedPropertyHandlerConfiguration;
@@ -47,6 +48,7 @@ class RealEnvStore final : public KVStore {
4748
int32_t Query(const char* key) const override;
4849
void Delete(Isolate* isolate, Local<String> key) override;
4950
MaybeLocal<Array> Enumerate(Isolate* isolate) const override;
51+
MaybeLocal<Array> Pairs(Isolate* isolate) const override;
5052
};
5153

5254
class MapKVStore final : public KVStore {
@@ -219,6 +221,62 @@ MaybeLocal<Array> RealEnvStore::Enumerate(Isolate* isolate) const {
219221
return Array::New(isolate, env_v.out(), env_v_index);
220222
}
221223

224+
MaybeLocal<Array> RealEnvStore::Pairs(Isolate* isolate) const {
225+
Mutex::ScopedLock lock(per_process::env_var_mutex);
226+
uv_env_item_t* items;
227+
int count;
228+
229+
auto cleanup = OnScopeLeave([&]() { uv_os_free_environ(items, count); });
230+
CHECK_EQ(uv_os_environ(&items, &count), 0);
231+
232+
MaybeStackBuffer<Local<Value>, 256> pairs_v(count);
233+
int pairs_v_index = 0;
234+
std::string pair;
235+
for (int i = 0; i < count; i++) {
236+
#ifdef _WIN32
237+
// If the key starts with '=' it is a hidden environment variable.
238+
// Enumerate() skips these, so a copy of process.env never had them.
239+
if (items[i].name[0] == '=') continue;
240+
#endif
241+
pair.assign(items[i].name);
242+
pair += '=';
243+
pair += items[i].value;
244+
Local<Value> str;
245+
if (!ToV8Value(isolate->GetCurrentContext(), pair, isolate).ToLocal(&str)) {
246+
return {};
247+
}
248+
pairs_v[pairs_v_index++] = str;
249+
}
250+
251+
return Array::New(isolate, pairs_v.out(), pairs_v_index);
252+
}
253+
254+
MaybeLocal<Array> KVStore::Pairs(Isolate* isolate) const {
255+
EscapableHandleScope scope(isolate);
256+
Local<Context> context = isolate->GetCurrentContext();
257+
Local<Array> keys;
258+
if (!Enumerate(isolate).ToLocal(&keys)) return {};
259+
uint32_t keys_length = keys->Length();
260+
LocalVector<Value> pairs(isolate);
261+
pairs.reserve(keys_length);
262+
for (uint32_t i = 0; i < keys_length; i++) {
263+
Local<Value> key;
264+
Local<String> value;
265+
if (!keys->Get(context, i).ToLocal(&key)) return {};
266+
if (!key->IsString()) continue;
267+
// A key that disappeared between Enumerate() and Get() is skipped, like an
268+
// undefined value is when copying process.env in JS.
269+
if (!Get(isolate, key.As<String>()).ToLocal(&value)) continue;
270+
Local<String> pair = String::Concat(
271+
isolate,
272+
String::Concat(
273+
isolate, key.As<String>(), FIXED_ONE_BYTE_STRING(isolate, "=")),
274+
value);
275+
pairs.push_back(pair);
276+
}
277+
return scope.Escape(Array::New(isolate, pairs.data(), pairs.size()));
278+
}
279+
222280
std::shared_ptr<KVStore> KVStore::Clone(Isolate* isolate) const {
223281
HandleScope handle_scope(isolate);
224282
Local<Context> context = isolate->GetCurrentContext();

‎src/process_wrap.cc‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ class ProcessWrap : public HandleWrap {
7979
SetProtoMethod(isolate, constructor, "kill", Kill);
8080

8181
SetConstructorFunction(context, target, "Process", constructor);
82+
SetMethodNoSideEffect(context, target, "getEnvPairs", GetEnvPairs);
8283

8384
Local<Object> constants = Object::New(isolate);
8485
NODE_DEFINE_CONSTANT(constants, kProcessFlagDetached);
@@ -91,6 +92,7 @@ class ProcessWrap : public HandleWrap {
9192
registry->Register(New);
9293
registry->Register(Spawn);
9394
registry->Register(Kill);
95+
registry->Register(GetEnvPairs);
9496
}
9597

9698
SET_NO_MEMORY_INFO()
@@ -341,6 +343,17 @@ class ProcessWrap : public HandleWrap {
341343
args.GetReturnValue().Set(err);
342344
}
343345

346+
// The current environment as ["KEY=value", ...], i.e. what a spawned
347+
// child inherits by default, produced in one pass over the environment
348+
// block instead of one interceptor round trip per variable.
349+
static void GetEnvPairs(const FunctionCallbackInfo<Value>& args) {
350+
Environment* env = Environment::GetCurrent(args);
351+
Local<Array> pairs;
352+
if (env->env_vars()->Pairs(env->isolate()).ToLocal(&pairs)) {
353+
args.GetReturnValue().Set(pairs);
354+
}
355+
}
356+
344357
static void Kill(const FunctionCallbackInfo<Value>& args) {
345358
Environment* env = Environment::GetCurrent(args);
346359
ProcessWrap* wrap;

‎src/util.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,10 @@ class KVStore {
323323
virtual int32_t Query(const char* key) const = 0;
324324
virtual void Delete(v8::Isolate* isolate, v8::Local<v8::String> key) = 0;
325325
virtual v8::MaybeLocal<v8::Array> Enumerate(v8::Isolate* isolate) const = 0;
326+
// All entries as an array of "KEY=value" strings, in enumeration order —
327+
// the form uv_spawn() consumes. The default implementation is
328+
// Enumerate() + Get(); stores that can produce it in one pass override it.
329+
virtual v8::MaybeLocal<v8::Array> Pairs(v8::Isolate* isolate) const;
326330

327331
virtual std::shared_ptr<KVStore> Clone(v8::Isolate* isolate) const;
328332
virtual v8::Maybe<void> AssignFromObject(v8::Local<v8::Context> context,
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
'use strict';
2+
// When no `env` option is given, a child process must inherit exactly the
3+
// parent's current environment: same variables, same values, reflecting
4+
// runtime additions/deletions made through process.env, and (on POSIX, where
5+
// nothing re-sorts the block) in the same order the parent enumerates it.
6+
const common = require('../common');
7+
const assert = require('assert');
8+
const { spawn, spawnSync, execFileSync } = require('child_process');
9+
10+
// Mutate the environment at runtime in a few ways first.
11+
process.env.TEST_DEFAULT_ENV_ADDED = 'added ünïcödé ✓';
12+
process.env.TEST_DEFAULT_ENV_EMPTY = '';
13+
process.env.TEST_DEFAULT_ENV_EQUALS = 'a=b=c';
14+
process.env.TEST_DEFAULT_ENV_DELETED = 'x';
15+
delete process.env.TEST_DEFAULT_ENV_DELETED;
16+
17+
function expectedEnv() {
18+
// What `{ ...process.env }` yields, minus keys whose value is undefined
19+
// (there are none for the real environment, but keep the definition exact).
20+
const copy = { ...process.env };
21+
for (const key of Object.keys(copy)) {
22+
if (copy[key] === undefined) delete copy[key];
23+
}
24+
return copy;
25+
}
26+
27+
const printEnv = ['-e', 'process.stdout.write(JSON.stringify([Object.keys(process.env), process.env]))'];
28+
29+
function check(output, label, expected = expectedEnv()) {
30+
const [childKeys, childEnv] = JSON.parse(output);
31+
assert.deepStrictEqual(childEnv, expected, `${label}: contents`);
32+
assert.strictEqual(childEnv.TEST_DEFAULT_ENV_ADDED, 'added ünïcödé ✓');
33+
assert.strictEqual(childEnv.TEST_DEFAULT_ENV_EMPTY, '');
34+
assert.strictEqual(childEnv.TEST_DEFAULT_ENV_EQUALS, 'a=b=c');
35+
assert.ok(!('TEST_DEFAULT_ENV_DELETED' in childEnv));
36+
if (!common.isWindows) {
37+
// Integer-like names are hoisted by object key ordering on both sides, so
38+
// compare the order of the remaining names.
39+
const nonIndex = (k) => !/^(?:0|[1-9]\d*)$/.test(k);
40+
assert.deepStrictEqual(childKeys.filter(nonIndex), Object.keys(expected).filter(nonIndex), `${label}: order`);
41+
}
42+
}
43+
44+
// spawnSync, options omitted entirely.
45+
check(spawnSync(process.execPath, printEnv, { encoding: 'utf8' }).stdout, 'spawnSync no options');
46+
// Explicitly undefined / null env behave like the default.
47+
check(spawnSync(process.execPath, printEnv, { encoding: 'utf8', env: undefined }).stdout, 'spawnSync env undefined');
48+
check(spawnSync(process.execPath, printEnv, { encoding: 'utf8', env: null }).stdout, 'spawnSync env null');
49+
// execFileSync goes through the same normalization.
50+
check(execFileSync(process.execPath, printEnv, { encoding: 'utf8' }), 'execFileSync');
51+
// A user-supplied env is still passed through as given (not merged).
52+
{
53+
const env = { ONLY: 'this', PATH: process.env.PATH };
54+
const out = spawnSync(process.execPath, printEnv, { encoding: 'utf8', env }).stdout;
55+
const [, childEnv] = JSON.parse(out);
56+
assert.strictEqual(childEnv.ONLY, 'this');
57+
assert.ok(!('TEST_DEFAULT_ENV_ADDED' in childEnv));
58+
}
59+
// Async spawn (the environment is captured at spawn() time).
60+
{
61+
const expectedAtSpawn = expectedEnv();
62+
const child = spawn(process.execPath, printEnv);
63+
let out = '';
64+
child.stdout.setEncoding('utf8').on('data', (d) => { out += d; });
65+
child.on('close', common.mustCall((code) => {
66+
assert.strictEqual(code, 0);
67+
check(out, 'spawn', expectedAtSpawn);
68+
}));
69+
}
70+
// A variable added after an earlier spawn is seen by a later one (no caching).
71+
process.env.TEST_DEFAULT_ENV_LATE = 'late';
72+
{
73+
const [, childEnv] = JSON.parse(spawnSync(process.execPath, printEnv, { encoding: 'utf8' }).stdout);
74+
assert.strictEqual(childEnv.TEST_DEFAULT_ENV_LATE, 'late');
75+
}

0 commit comments

Comments
 (0)