Skip to content
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

HapiJs + Good under a child process fails with a libuv error #236

Closed
chriswiggins opened this issue Nov 3, 2014 · 11 comments
Closed

HapiJs + Good under a child process fails with a libuv error #236

chriswiggins opened this issue Nov 3, 2014 · 11 comments
Labels
bug
Milestone

Comments

@chriswiggins
Copy link

@chriswiggins chriswiggins commented Nov 3, 2014

Hi all,

As being discussed over at Unitech/pm2#659, we're having issues running Hapi+Good under a child process when IPC is specified as the first stdio descriptor (FD0). I can't quite figure out how to fix it, and unfortunately changing from IPC to something else is something we can't do as it's required functionality.

I've made a test app over at http://github.com/chriswiggins/hapi-good-spawn-ipc to illustrate the problem. Run node index.js and you'll instantly see the child process crash (which is a VERY simple Hapi+Good app). This crash doesn't happen if you run the child without the good logging.

Any ideas as to why this is? I'm happy to fix it if someone can point me in the right direction - its a fairly weird issue!

@arb

This comment has been minimized.

Copy link
Contributor

@arb arb commented Nov 3, 2014

Thanks for this, just a nice reproducible example is very helpful. The error looks like it comes from "way down deep" at the C/C++ layer.

stderr: Assertion failed: (loop->watchers[w->fd] == w), function uv__io_stop, file ../deps/uv/src/unix/core.c, line 701.
@chriswiggins

This comment has been minimized.

Copy link
Author

@chriswiggins chriswiggins commented Nov 3, 2014

@arb, yep exactly. What I'm unsure of is where Good uses libuv or interacts with FD0, causing this error. If the FD0 is changed to 'ignore' from 'ipc', then this also causes the error to stop occurring. Weird!

@arb arb mentioned this issue Nov 3, 2014
@arb

This comment has been minimized.

Copy link
Contributor

@arb arb commented Nov 3, 2014

Ok, we think we found it. See related issue.

@cjihrig

This comment has been minimized.

Copy link
Contributor

@cjihrig cjihrig commented Nov 3, 2014

The problem occurs in Utils.inheritAsync(). Accessing process.stdin when stdin is configured as ipc is causing the crash. I'm still not entirely sure why this is. I believe you would see similar problems if stdout or stderr were setup as ipc and you tried to access them.

@arb arb added the bug label Nov 3, 2014
@arb arb added this to the 4.0.0 milestone Nov 3, 2014
@cjihrig

This comment has been minimized.

Copy link
Contributor

@cjihrig cjihrig commented Nov 4, 2014

@chriswiggins is there a reason you need to pass 'ipc' for stdin? This appears to be a limitation with node itself, as the following code also crashes:

if (process.argv[2] === 'child') {
  process.stdin;
} else {
  var cp = require('child_process');
  var child = cp.spawn('node', [__filename, 'child'], {
    stdio: ['ipc', 'inherit', 'inherit']
  });
}

I've had success with other configuration options, as well as calling fork() instead of spawn(). If you absolutely have to have the IPC channel (node only supports one channel per child), you can call fork(), or spawn() with this stdio configuration ['pipe', 'pipe', 'pipe', 'ipc'], which is how node sets up fork(). Does that work for you?

@cjihrig

This comment has been minimized.

Copy link
Contributor

@cjihrig cjihrig commented Nov 4, 2014

I have also opened an issue on node core - nodejs/node-v0.x-archive#8669

@chriswiggins

This comment has been minimized.

Copy link
Author

@chriswiggins chriswiggins commented Nov 4, 2014

@cjihrig actually no we don't need stdin to be ipc. Just needed an IPC somewhere and having it fourth fixes the problem! Thanks a bunch. I guess I'll close this issue, but good to know we found an actual bug in node core / libuv. Power of the community!
:-)

@chriswiggins

This comment has been minimized.

Copy link
Author

@chriswiggins chriswiggins commented Nov 4, 2014

Actually, I may have spoken too soon but I'll look into this further on the PM2 side

chriswiggins added a commit to chriswiggins/PM2 that referenced this issue Nov 5, 2014
@chriswiggins

This comment has been minimized.

Copy link
Author

@chriswiggins chriswiggins commented Nov 5, 2014

@arb, @cjihrig : Just want to thank you for your help here! We've fixed the issue on our side with FD4. It does work, we just had a slight issue with our test cases :-)

@cjihrig

This comment has been minimized.

Copy link
Contributor

@cjihrig cjihrig commented Nov 5, 2014

Glad you were able to get something working!

@arb

This comment has been minimized.

Copy link
Contributor

@arb arb commented Nov 5, 2014

Good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.