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

spawn's stdio can result in stderr being closed in child #862

Closed
sam-github opened this issue Feb 17, 2015 · 8 comments · May be fixed by solebox/node#25, UbuntuEvangelist/node#17, saeedahassan/node#35, erdun/node#41 or enterstudio/node#24
Labels
confirmed-bug Issues with confirmed bugs.

Comments

@sam-github
Copy link
Contributor

var cp = require('child_process');
var fs = require('fs');

if (process.env.CHILD) {
  console.error('ERROR');
  return;
}
// open child stdout and stderr on our stderr
var options = {
  stdio: [0, 2, 2],
  env: { CHILD: true },
};
var c = cp.spawn(process.execPath, [__filename], options);

Result: 'ERROR' is not printed.

Excerpt from strace:

[pid 11780] write(2, "ERROR\n", 6)      = -1 EBADF (Bad file descriptor)
[pid 11780] write(2, "ERROR\n", 6)      = -1 EBADF (Bad file descriptor)
[pid 11780] epoll_ctl(4, EPOLL_CTL_DEL, 2, {EPOLLWRNORM|EPOLLHUP|EPOLLRDHUP|EPOLLET|0x242e0000, {u32=32767, u64=47321597480042495}}) = -1 ENOENT (No such file or directory)
[pid 11780] write(2, "events.js:141\n      throw er; //"..., 244) = -1 EBADF (Bad file descriptor)
[pid 11780] exit_group(1)               = ?
[pid 11785] +++ exited with 1 +++

This likely a bug in libuv, but reporting it here because that's where I saw it.

Affects node v0.10 and io.js.

/cc @saghul @bnoordhuis I think we tried to fix a variant of this a year or so ago.

@saghul
Copy link
Member

saghul commented Feb 17, 2015

I believe that's still not fixed. This is the closest open issue I could find: joyent/libuv#923

@bnoordhuis
Copy link
Member

I think this asks for a two-pronged approach: libuv probably has a bug or two that need to be addressed but io.js could reopen fds 0-2 as /dev/null if they aren't valid file descriptors.

@vkurchatkin
Copy link
Contributor

@bnoordhuis can you also comment on #831 ?

@sam-github
Copy link
Contributor Author

Opening fd 2 on /dev/null would avoid the EBADF, and probably is a good idea, so fd 2 doesn't become some random open file, but wouldn't avoid the underlying problem, that console.error() never prints, because stderr is gone.

I dug up the last time I saw this:

@sam-github sam-github added the confirmed-bug Issues with confirmed bugs. label Feb 18, 2015
sam-github added a commit to strongloop/strong-supervisor that referenced this issue Feb 18, 2015
@sam-github
Copy link
Contributor Author

@saghul I think I've fixed this, as well as some related bugs. I'm putting together some node tests, then I'll PR the fix to libuv.

@saghul
Copy link
Member

saghul commented Feb 24, 2015

Great! Thanks Sam.
On Feb 24, 2015 6:57 PM, "Sam Roberts" notifications@github.com wrote:

@saghul https://github.com/saghul I think I've fixed this, as well as
some related bugs. I'm putting together some node tests, then I'll PR the
fix to libuv.


Reply to this email directly or view it on GitHub
#862 (comment).

saghul pushed a commit to saghul/libuv that referenced this issue Mar 20, 2015
If a descriptor was being copied to an fd that *was not its current
value*, it should get closed after being copied. But the existing code
would close the fd, even when it no longer referred to the original
descriptor. Don't do this, only close fds that are not in the desired
range for the child process.

See nodejs/node#862
saghul pushed a commit to saghul/libuv that referenced this issue Mar 20, 2015
If a descriptor was being copied to an fd that *was not its current
value*, it should get closed after being copied. But the existing code
would close the fd, even when it no longer referred to the original
descriptor. Don't do this, only close fds that are not in the desired
range for the child process.

See nodejs/node#862
saghul pushed a commit to saghul/libuv that referenced this issue Apr 7, 2015
If a descriptor was being copied to an fd that *was not its current
value*, it should get closed after being copied. But the existing code
would close the fd, even when it no longer referred to the original
descriptor. Don't do this, only close fds that are not in the desired
range for the child process.

See nodejs/node#862
saghul pushed a commit to saghul/libuv that referenced this issue Apr 10, 2015
If a descriptor was being copied to an fd that *was not its current
value*, it should get closed after being copied. But the existing code
would close the fd, even when it no longer referred to the original
descriptor. Don't do this, only close fds that are not in the desired
range for the child process.

See nodejs/node#862

PR-URL: libuv#309
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
bnoordhuis pushed a commit that referenced this issue May 6, 2015
Fixes: #1397
Fixes: #1512
Fixes: #1621
Fixes: #862
PR-URL: #1646
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@bnoordhuis
Copy link
Member

Should be fixed by 04cc03b. Closing, holler if I should reopen it.

Fishrock123 pushed a commit to Fishrock123/node that referenced this issue May 19, 2015
Fixes: nodejs#1397
Fixes: nodejs#1512
Fixes: nodejs#1621
Fixes: nodejs#862
PR-URL: nodejs#1646
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@leth
Copy link

leth commented Sep 7, 2016

I'm seeing a failure like this frequently from npm install under node 6.4.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment