nodejs / nodejs/node

Stop throwing for unsupported options in `Worker`'s `execArgv` and `env.NODE_OPTIONS`.

オープン
#41,103 コメント 14 件 リアクション 15 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

worker
主要言語
JavaScript
スター
122k
フォーク
37.3k
平均マージ
4日 2時間
マージ済み PR(30日)
283

説明

Version

v17.2.0

Platform

No response

Subsystem

worker_threads

What steps will reproduce the bug?

Consider this test.js file:

const { isMainThread, Worker } = require("worker_threads");

if (isMainThread) {
  new Worker(__filename, {
    // execArgv: process.execArgv
  });
} else {
  console.log("Hello from the worker!", process.execArgv);
}

NOTE: --experimental-json-modules and --title are just examples

Run these two commands:

node --experimental-json-modules test.js
node --experimental-json-modules --title foo test.js

They will log

Hello from the worker! [ '--experimental-json-modules' ]
Hello from the worker! [ '--experimental-json-modules', '--title', 'foo' ]

Now, uncomment the execArgv: process.execArgv line and run the two commands again. The first command will still log

Hello from the worker! [ '--experimental-json-modules' ]

however, the second one throws:

Error [ERR_WORKER_INVALID_EXEC_ARGV]: Initiated Worker with invalid execArgv flags: --title
    at new NodeError (node:internal/errors:371:5)
    at new Worker (node:internal/worker:191:13)
    at Object.<anonymous> (/home/nicolo/Documenti/dev/babel/babel/test.js:4:3)
    at Module._compile (node:internal/modules/cjs/loader:1097:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1149:10)
    at Module.load (node:internal/modules/cjs/loader:975:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
    at node:internal/main/run_main_module:17:47 {
  code: 'ERR_WORKER_INVALID_EXEC_ARGV'
}

This mans that the default behavior of the Worker execArgv option is not to inherit execArgv from the parent thread (contrary to what the documentation says).

Why would you want to explicitly pass execArgv: process.execArgv anyway?

A few days ago I was trying to implement a require hook that internally uses a Worker, and in order to avoid registering it recursively in an infinite worker-spawn loop I tried to filter out the --require flag:

new Worker(workerFile, {
  execArgv: process.execArgv.filter((val, i, argv) => val === "--requrie" || argv[i-1] === "--requrie"),
});

this seemed to work, until it crashed in a test that was using the --expose_gc option.

Today, I found the exact same problem in jest-worker (a Jest subpackage that is often also used outside of Jest): https://github.com/facebook/jest/pull/12103 was opened because jest-worker didn't work when running Node.js with --max_old_space_size.

Since Node.js doesn't provide the list of flags supported by workers progrmmatically (and I didn't find it in the docs either 😅) there is no way to filter the execArgv used by a worker.

What is the current behavior?

I initially expected the real default behavior to be something like execArgv: process.execArgv.filter(isSupportedByWorker), but it looks like it's not either. Consider for example the --expose_gc (which is disallowed in workers) option with this example:

const { isMainThread, Worker } = require("worker_threads");

if (isMainThread) {
  new Worker(__filename);
} else {
  console.log("Hello from the worker!", process.execArgv, globalThis.gc);
}

If you run node --expose_gc test.js, it will log

Hello from the worker! [ '--expose_gc' ] [Function: gc]

You can see that the gc function is present. This means that the disallowed flags are not ignored (and not even "ignored but still used to populate the process.execArgv array): they are used and affect the runtime behavior.

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

Only for the options marked as "unsupported" in workers.

What is the expected behavior?

I see four possible solutions:

  • Just ignore the unsupported options
  • Never throw, but warn when passing an unsupported option to a worker (potentially even if it's passed by default without an explicit execArgv).
  • Only throw if the unsupported option is not also used as an execArgv of the parent thread.
  • Always throw for unsupported options (so that execArgv: process.execArgv becomes the default behavior), and provide a Worker.filterExecArgv utility that removes disallowed options, and that I would have used like
    Worker.filterExecArgv(process.execArgv).filter((val, i, argv) => val === "--requrie" || argv[i-1] === "--requrie")
    

If the current behavior is expected, then the docs should be updated to mention that the default value cannot be replicated by explicitly passing an execArgv option.

What do you see instead?

No response

Additional information

No response

コントリビューションガイド

コントリビューションガイドを開く

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

調査の方向性

まず、--experimental-json-modules、--title、--expose_gc を使用する test.js の worker_threads 再現から始め、次に Worker execArgv のドキュメントとエントリーポイントを確認します。issue では互換性のない動作の選択肢がいくつか提示されているため、最初にサポートされていない execArgv と env.NODE_OPTIONS の意図された扱いを明確にしてください; 完了条件には、一致する動作とテストカバレッジを含める必要があります。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
javascript, node.js
領域
backend
issue の種類
機能追加
難易度
5/5
見積もり時間
1週間以上
活発さ
静か
明瞭さ
説明が足りない
初心者へのやさしさ
35/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。