-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
Incorrect ps output for node process on AIX. #10607
Comments
Node modifies the argv array in src/node.cc stripping off all the options leaving only the script file and parameters. |
After a quick google and a test it looks like caching isn’t quite what’s happening. Assuming Node doesn’t intend to hide v8 arguments from ps then this bug only affects AIX. (If Node did intend to change the ps output then I suspect that fixing that now might break a surprising number of things that read the ps output - monitoring scripts and the like.) Either way it should probably just be fixed for AIX so that AIX is consistent with the behaviour on other platforms and doesn’t display junk. If I can sort my AIX build environment out I’ll have a look at copying argv to a new array the startup code can safely modify but it should be a fairly trivial fix. |
AIX passes the same argv array that ps and other utilities see to main(). Node removes elements from argv but as it can't pass back updates to argc which is passed by value to main() it effectively corrupts that array for other users. Fixes: nodejs#10607
I've prototyped the change to libuv on a branch in my node repository, now I need to clone libuv, test the change there on AIX and raise the pull request against libuv. If anyone is interested the change is here: |
The pull request for libuv libuv/libuv#1187 has been approved. I will close the PR for duplicating argv on AIX (#10633). I think I need to raise another PR to enable the test for setting process.title (test/simple/test-setproctitle.js) on AIX once that change has been committed into libuv and Node has taken that level of libuv. It's a very small change but is there a way to tag that so it doesn't get lost? |
Node.js currently rewrites the process command line on AIX. Refs: nodejs/node#10607
Node.js currently rewrites the process command line on AIX. Refs: nodejs/node#10607 PR-URL: #41 Reviewed-By: Richard Chamberlain <richard_chamberlain@uk.ibm.com> Reviewed-By: Howard Hellyer <hhellyer@uk.ibm.com>
@hhellyer Time to raise this PR? |
@gibfahn - Yes, I'll change it to match the libuv test which now only skips the test for SunOS. The libuv update also enabled setting the process title on MVS and now the libuv runs for everything except SunOS. (I just need to try the test out in those two environments first.) |
Fixes: nodejs#10165 Fixes: nodejs#9856 Fixes: nodejs#10607 Fixes: nodejs#11104 PR-URL: nodejs#11094 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Change for enabling the test in Node.js raised as PR #11416 |
Fixes: nodejs#10165 Fixes: nodejs#9856 Fixes: nodejs#10607 Fixes: nodejs#11104 PR-URL: nodejs#11094 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Fixes: nodejs/node#10165 Fixes: nodejs/node#9856 Fixes: nodejs/node#10607 Fixes: nodejs/node#11104 PR-URL: nodejs/node#11094 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
The command line arguments to the node process seem to be becoming corrupted on AIX when displayed by the ps command. For example if I run a simple script and use a command line option:
> node --abort_on_uncaught_exception --experimental_extras --max_old_space_size=300 --no-warnings simplehttp.js opt1 opt2 opt3
The output of running the ps command is:
It looks like the script name and arguments have moved up but not cleared from their original position. It doesn’t seem to be specific to using v8 arguments beginning with --, using -r to require a module causes the same problem.
If I run the same script with the same options on Linux or Mac the output from ps is correct. e.g from Linux:
which is exactly what was on the command line. On AIX if I run a script that prints process.argv and process.execArgv with the same options they are in in the correct positions, it's only the output from ps that looks wrong. It may be something specific in how the options are handled on AIX or it may be node always corrupts argv but that AIX is the only platform that gives the process the real argv array that’s also used by the ps command.
@nodejs/platform-aix - Probably worth the AIX team looking at this in the first instance however it may be a more general problem.
The text was updated successfully, but these errors were encountered: