worker: inherits mutated NODE_OPTIONS

[arcanis]

Changes made to the NODE_OPTIONS environment variables weren't properly taken into account when spawning workers, unlike how child_process.fork works.

Fixes part of https://github.com/nodejs/node/issues/37410 (I didn't touch the process.execArgv part, I was worried it'd be a breaking change otherwise).

[aduh95]

@arcanis It looks like there are CI failures to address.

/cc @nodejs/workers

[arcanis]

I've investigated, but the issue is a little complicated for me, as I don't know very well the v8 C++ API. It turns out the real cause for the problem this PR tries to fix is that NODE_OPTIONS is only processed when env is explicitly forwarded:

https://github.com/nodejs/node/blob/main/src/node_worker.cc#L488-L527

However, doing this also creates a new per_isolate_opts object (required for options_parser::Parse to work, otherwise it will segfault) - when per_isolate_opts is set, it will eventually make its way into isolate_data_->options(), which I think causes env->options()->cpu_prof to be reset here.

What I'm not sure, and where I could use some help:

• Would it be acceptable to hardcode a list of settings that should be persisted across workers (starting with --cpu-prof)? In that case, how can I get the parent option values? I'm not sure what code specifies that, if per_isolate_opts is NULL, then the options are inherited from the parent isolate.

• That being said, is forwarding the main isolate options (only in some cases!) even intended? Shouldn't users pass the relevant flags to NODE_OPTIONS instead, if they wish to debug nested workers/threads? (Probably a major change, so I don't plan to change this behaviour here; it's just for my curiosity)

[robertsLando]

updates on this?

[arcanis]

As far as I can tell my questions are still relevant, and I'd need some guidance on what's the appropriate strategy here.

[robertsLando]

@aduh95 Dunno if the @nodejs/workers worked, maybe you could tag someone that can help moving this forward?