worker_threads and NODE_OPTIONS

[arcanis]

• Version: 12.3.1
• Platform: all
• Subsystem: workers
• Tl;dr: process._preload_modules should be forwarded to the workers

This one is a bit convoluted ... first some context:

• Yarn 2 keeps the vendors within zip archives instead of unpacking them
• To make it work, NODE_OPTIONS contains --require /path/to/.pnp.js
• This .pnp.js extends the fs module to add support for in-zip reading
• Parcel 2, a vendor (and thus stored within a zip), uses worker_threads if possible

It works fine with child_process.fork, because under the regular CLI the --require scripts are executed before the entry point is resolved (except when using --inspect if I remember correctly), so by the time runMain is called the zip archives can be accessed.

However, with worker_threads it seems that the NODE_OPTIONS --require entries are completely ignored and runMain is called directly, causing the script to be executed outside of the PnP context and thus not being able to access its dependencies or even spawn if stored within a zip archive (which is the case by default, although it can be disabled).

[bnoordhuis]

Tangential: it sounds like implementing #27501 (module archive support) would also fix your issue.

[arcanis]

It would certainly help (we talked about it some time ago with @bmeck), but I feel like this issue should be fixed even outside of my own use case - for example esm can't work with worker_threads for the same reason.

[addaleax]

@arcanis Can you share a reproduction? As far as I can tell, --require is respected for Workers, as it should be:

Repro setup, click to expand

test.js:

'use strict';
const { Worker, isMainThread } = require('worker_threads');

if (isMainThread)
  new Worker(__filename);

preload.js:

'use strict';
console.log('preload module', require('worker_threads').threadId);

Run in shell:

$ node --require ./preload.js test.js
preload module 0
preload module 1
$ NODE_OPTIONS='--require ./preload.js' node test.js
preload module 0
preload module 1

Each print twice, as they should, once for each Node.js instance

[arcanis]

Hey @addaleax! Sorry for the (very 🙃) late answer. I've just investigated more and the problem appears if you make the following change to your code (which seems to be what Parcel is doing):

new Worker(__filename, {execArgv: process.execArgv});

In this case, the preload won't be executed anymore as execArgv will be an empty array.

cc @mischnic

[addaleax]

@arcanis Hey! Thanks for the update … however, with only that modification, I still can’t reproduce the issue (neither with Node.js master nor v12.3.1). Is there something else that turns execArgv into an empty array? I wouldn’t know why --require flags would end up being excluded from it, tbh.

[arcanis]

I've put the repro here: https://github.com/arcanis/node-options-execargv

Running script.sh yields the following output:

v12.3.1
preload module 0
[]
[]

Removing the execArgv option from the Worker constructor yields the following one:

v12.3.1
preload module 0
[]
preload module 1
[]

[addaleax]

Ah, I see – sorry, this was kind of my bad. The issue is that NODE_OPTIONS don’t end up affecting process.execArgv.

So I guess the question is whether NODE_OPTIONS should also affect Workers even if execArgv was passed in explicitly as an option?

[arcanis]

Imo it should, because users would have a way to override this behaviour if they wish to by explicitly removing the key from the environment before passing it to the Worker constructor. So if you want a Worker with zero flag inheritance, you could write:

const env = {...process.env};
delete env.NODE_OPTIONS;

new Worker(..., {
  execArgv: [],
  env,
});

[lundibundi]

This should now work with or without the execArgv in Worker options.
See #31711.

Feel free to reopen if I missed anything.