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

Using node with process.stdin inside spawned shell #18446

Closed
qfox opened this issue Jan 30, 2018 · 6 comments
Closed

Using node with process.stdin inside spawned shell #18446

qfox opened this issue Jan 30, 2018 · 6 comments
Labels
child_process Issues and PRs related to the child_process subsystem.

Comments

@qfox
Copy link

qfox commented Jan 30, 2018

  • Version: v8.4.0
  • Platform: Darwin duplo 15.6.0 Darwin Kernel Version 15.6.0: Tue Apr 11 16:00:51 PDT 2017; root:xnu-3248.60.11.5.3~1/RELEASE_X86_64 x86_64
  • Subsystem: net, stream or child_process (?)

Tested with:

  • GNU bash, version 3.2.57(1)-release (x86_64-apple-darwin15)
  • GNU bash, version 4.2.25(1)-release (x86_64-pc-linux-gnu)

Not sure is this the same or another bug but in my environment it throws:

Error: write EPIPE
    at _errnoException (util.js:1041:11)
    at WriteWrap.afterWrite [as oncomplete] (net.js:858:14)

But when we played with it a little:

Error: This socket has been ended by the other party
    at Socket.writeAfterFIN [as write] (net.js:352:12)
    at __dirname (/Users/qfox/repos/trendbox-ci/omg.js:11:11)
    at <anonymous>

How to repro:

const { spawn } = require('child_process');
const pause = t => new Promise(resolve => setTimeout(resolve, t));

const { stdin } = spawn('bash', [/*'-i'*/], { stdio: ['pipe', 'inherit'] });

(async () => {
    stdin.write('echo All is fine here\n');
    await pause(200);
    stdin.write('node -e process.stdin && echo After process.stdin\n');
    await pause(2000); // It's important: the bigger pause, the more frequently occurs
    stdin.write('echo After all\n');
})().catch(e => console.error(e.stack));

If we remove usage of process.stdin or force shell to be interactive (pass in ['-i'] flag) or just call node -e process.stdin with </dev/stdin — then the problem disappears.

Would be nice to have a good error message if it's unfixable ;'-(

upd:
See also: #947, #2339, #13278

@gireeshpunathil
Copy link
Member

consistently recreatable in Linux, with this code:

var child = require('child_process').spawn('bash')
child.stdin.write('node -e process.stdin\n')
setTimeout(() => {
  child.stdin.write('echo hello world!\n')
},1000)

strace shows that bash exited with
read(0, 0xecbf60, 1) = -1 EAGAIN (Resource temporarily unavailable)

while trying to read the second command, after the sleep.

EAGAIN for read manual says:
The file descriptor fd refers to a socket and has been marked non-blocking (O_NONBLOCK), and the read would block.

So looks like bash is handing over its fd 0 (a socket in this case) it got from node as it is to the child node.

[pid 5529] fstat(0, {st_mode=S_IFSOCK|0777, st_size=0, ...}) = 0

so Node makes it non-blocking to fit its use case

[pid 5529] ioctl(0, FIONBIO, [1]) = 0

back in bash, it waits to read from fd 0, but the read returns with soft error (EAGAIN). This means no data now, please wait.

Unfortunately bash seems to be not prepared for this scenario. So it exits with error.

With this understanding, it is easy to recreate without Node in the picture:

#cat 18446.cc

#include <sys/ioctl.h>
int main() {
  int set = 1;
  ioctl(0, FIONBIO, &set);
  return 0;
}

#g++ 18446.cc
#bash -c './a.out && read'
bash: line 0: read: read error: 0: Resource temporarily unavailable

Should we restore the standard streams on exit? I thought I have seen a PR on this line.

/cc @bnoordhuis

@gireeshpunathil
Copy link
Member

linking with #14752 and #17737

@gireeshpunathil
Copy link
Member

This issue has been resolved via #20592 and I have verified with both the test programs.

#node 18446.js

All is fine here
After process.stdin
After all

Closing this, fix will mostly be available in the next current release.

@qfox
Copy link
Author

qfox commented Jun 4, 2018

Thank you very much!

@gireeshpunathil
Copy link
Member

unfortunately #20592 was reverted in master, so re-opening this.

@bnoordhuis
Copy link
Member

This should have been fixed by #24260 so I'll go ahead and close it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
child_process Issues and PRs related to the child_process subsystem.
Projects
None yet
Development

No branches or pull requests

4 participants