[v22.2.0 regression] Some `Worker` use cases are broken

[liuxingbaoyu]

Version

v22.2.0

Platform

all

Subsystem

worker_threads

What steps will reproduce the bug?

import { Worker, isMainThread } from "worker_threads";

if (isMainThread) {
  new Worker(import.meta.filename, { env: process.env });
} else {
  console.log("Hello, world!");
}

node --expose_gc .\test.mjs

How often does it reproduce? Is there a required condition?

always

What is the expected behavior? Why is that the expected behavior?

Hello, world!

What do you see instead?

node:internal/worker:221
      throw new ERR_WORKER_INVALID_EXEC_ARGV(this[kHandle].invalidExecArgv);
            ^

Error [ERR_WORKER_INVALID_EXEC_ARGV]: Initiated Worker with invalid execArgv flags: --expose_gc
    at new Worker (node:internal/worker:221:13)
    at file:///F:/babel/test.mjs:4:3
    at ModuleJob.run (node:internal/modules/esm/module_job:262:25)
    at async ModuleLoader.import (node:internal/modules/esm/loader:475:24)
    at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:109:5) {
  code: 'ERR_WORKER_INVALID_EXEC_ARGV'
}

Additional information

Ref: #52827
It seems that passing execArgv will cause some problems until #41103 is resolved.

I'm not sure this is a regression, but it could be an unintentional breakage and would be a bit difficult to resolve on the user side since there are no known exec argvs supported by Worker.

cc @theanarkh You may be interested!

[nicolo-ribaudo]

Would it be possible to revert #52827 until a better fix is found? (i.e. the worker should just ignore unsupported argv that are passed implicitly rather than explicitly, or argv should be filtered before being passed to the worker)

[bricss]

It seems like this behavior is quite intentional, as per docs:

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 is provided as process.execArgv inside the worker. By default, options are inherited from the parent thread.

And a simple workaround is to add execArgv: [] to the worker's options 🕵️ or filter forbidden cli options from parent thread and pass it into worker options 🤷

[nicolo-ribaudo]

If "execArgv must always be passed otherwise your worker might break" is intentional, then:

• it shouldn't be affected by whether the env: {} option is passed or not
• Node.js should probably warn when not passing execArgv, as a library might pass tests but it cannot control what CLI options the app that it's used into uses

The docs say that "By default, options are inherited from the parent thread". The behavior before #52827 was (when defining env) that process-wide options are inherited and thread-local options are not. The new behavior is that thread-local options are inherited, and process-wide options just crash.

Neither of the two behaviors seem ideal and neither matches the docs (when env is defined), but broken for broken it's better to keep the old broken to avoid breaking existing code.

When not passing env, both thread-local and process-wide options are inherited.

or filter forbidden cli options from parent thread and pass it into worker options 🤷

Node.js doesn't expose a list of options that are forbidden, so it's impossible to do so (see #52827)

[bricss]

How about adding some code to fix this by filtering out unwanted cli options by default? 🤔

[theanarkh]

Maybe we can ignore the invalid args when the execArgv is inherited from parent ? Only throw an error when the user pass execArgv explicitly, like here.