-
Notifications
You must be signed in to change notification settings - Fork 30.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
worker: inherits mutated NODE_OPTIONS #44732
base: main
Are you sure you want to change the base?
Conversation
@arcanis It looks like there are CI failures to address. /cc @nodejs/workers |
05cb903
to
e369572
Compare
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 https://github.com/nodejs/node/blob/main/src/node_worker.cc#L488-L527 However, doing this also creates a new What I'm not sure, and where I could use some help:
|
updates on this? |
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? |
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 #37410 (I didn't touch the
process.execArgv
part, I was worried it'd be a breaking change otherwise).