jprichardson / node-kexec

Node.js exec function to replace running process; like Ruby's exec.
MIT License
50 stars 25 forks source link

Doesn't work with node v0.8.x #5

Closed dazoe closed 12 years ago

dazoe commented 12 years ago

Tested with version 0.8.8 kexec('top'); just causes the node process to die and doesn't start 'top'

I ran strace on it and it looks like it is loading and starting top but top is dying because it can't get stdio...

...
write(2, "\ttop: failed tty get\n", 21) = -1 EBADF (Bad file descriptor)
close(1)                                = -1 EBADF (Bad file descriptor)
close(2)                                = -1 EBADF (Bad file descriptor)
exit_group(1)                           = ?
+++ exited with 1 +++

works fine with node 0.6.x

jprichardson commented 12 years ago

Ya, I was aware that it didn't work with Node v0.8. But, I had no idea why. I tried asking here without much luck: https://groups.google.com/forum/?fromgroups=#!topic/nodejs-dev/FnLd3QQWs5Y

But, your report is helpful, because now I have some ideas to try to get this to work. Thanks!

mattinsler commented 12 years ago

I'd love for this to work. =-)

This doesn't solve the issue above, but to migrate to node-gyp, just do this...

Add binding.gyp:

{ "targets": [ { "target_name": "kexec", "sources": [ "src/node_kexec.cpp" ] } ] }

Change package.json to:

{ "name" : "kexec", "version" : "0.0.4", "description" : "Replace your Node.js process with another process. Like Ruby exec.", "homepage" : [ "https://github.com/jprichardson/node-kexec" ], "repository" : { "type" : "git", "url" : "https://github.com/jprichardson/node-kexec" }, "keywords" : ["exec","spawn","process"], "author" : "JP Richardson jprichardson@gmail.com", "licenses" : [ {"type" : "MIT" } ],
"main" : "./index", "engines" : { "node" : ">=0.4" }, "scripts": { "install": "node-gyp configure build" } }

jprichardson commented 12 years ago

Guys, I'm kinda at a loss on this one... I've tried a lot of things. Here's a dump of various things:

static Handle<Value> kexec(const Arguments& args) {
    puts("here.");
    //HandleScope scope;
    String::Utf8Value v8str(args[0]);
    //char* argv2[] = { const_cast<char *>("-c"), *v8str,  NULL};
    printf("Rar: %s\n", *v8str);
    puts("hi from mod");
    //char* argv2[] = {const_cast<char *>("-c"), *v8str, NULL};
    //char* argv3[] = { const_cast<char *>(""), const_cast<char *>("-c"), const_cast<char *>("top"), const_cast<char *>(">"), const_cast<char *>("/tmp/poo.txt"), NULL};
    char* argv4[] = {null};

    int err = execvp("/bin/bash", argv4);
    printf("Error: %d", err);      
    //int i = execvp("/usr/bin/top", argv3);
    //execlp("ps", "ps", NULL);
    //printf("Return code: %d", i);
    //return scope.Close(Undefined());//Undefined();
    //return scope.Close(String::New("world"));
    return Undefined();
}

I'm not sure where to go from here. Nothing really seems to work. What confuses me the most is I'm not sure how from @dazoe's report, bash can't get access to stdio.

Any ideas?

Thanks,

JP

ehg commented 12 years ago

After a quick dig, we found that commenting uv_disable_stdio_inheritance() out in node.cc seems to allow the child process access to stdio.

char** Init(int argc, char *argv[]) {
  // Initialize prog_start_time to get relative uptime.
  uv_uptime(&prog_start_time);

  // Make inherited handles noninheritable.
  uv_disable_stdio_inheritance();
aidanhs commented 12 years ago

The comment on uv_disable_stdio_inheritance is sadly quite terse. If you go back further you'll find https://github.com/joyent/node/commit/37d75ba241780c9199f601dd4c70713d190c68e6#L1R1581, which is a little more descriptive.

I guess node severs all ties child processes may have to node's stdin and stdout. Hence invalid file descriptors.

Digging a bit more, it seems that the reason this was added in the first place was to make a test pass - https://github.com/joyent/node/commit/09be360a0fee2c7619bae8c4248f9ed3d79d1b30. Reading that single line test description, it seems to be explicitly testing that it's impossible to do what kexec aims to do (I think?).

jprichardson commented 12 years ago

Hmm... so would it be possible to make a function uv_enable_stdio_inheritance() in kexec.cpp?

Here's the source for uv_disable_stdio_inheritance(): from: https://github.com/joyent/libuv/blob/master/src/unix/core.c

void uv_disable_stdio_inheritance(void) {
  int fd;

  /* Set the CLOEXEC flag on all open descriptors. Unconditionally try the
   * first 16 file descriptors. After that, bail out after the first error.
   */
  for (fd = 0; ; fd++)
    if (uv__cloexec(fd, 1) && fd > 15)
      break;
}

uv__cloexec():

int uv__cloexec(int fd, int set) {
  int r;

  do
    r = ioctl(fd, set ? FIOCLEX : FIONCLEX);
  while (r == -1 && errno == EINTR);

  return r;
}
jprichardson commented 12 years ago

Fixed it! Patch coming soon!

jprichardson commented 12 years ago

OK, patch pushed and published to NPM. Look it over and let me know if it works for you.

aidanhs commented 12 years ago

I think you need to edit https://github.com/jprichardson/node-kexec/commit/950a24fa56797e5dfe1b40caa9bd22d4c1ebe413#L3R12 as well?

ehg commented 12 years ago

Works for me, thanks!

jprichardson commented 12 years ago

@aidanhs I probably should have just deleted the wscript file since it's for node-waf and node-waf isn't in use by Node anymore. That file is only good for Node v0.4 compatibility in which case, the person should just run node install kexec0.0.3.