nodejs
worker: inherits mutated NODE_OPTIONS
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).
@arcanis It looks like there are CI failures to address.
/cc @nodejs/workers
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, ifper_isolate_optsis 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_OPTIONSinstead, 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)
As far as I can tell my questions are still relevant, and I'd need some guidance on what's the appropriate strategy here.
@aduh95 Dunno if the @nodejs/workers worked, maybe you could tag someone that can help moving this forward?