Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Add a way to remove O_NONBLOCK from process.stdin #3994

Closed
n1mmy opened this issue Sep 11, 2012 · 7 comments
Closed

Add a way to remove O_NONBLOCK from process.stdin #3994

n1mmy opened this issue Sep 11, 2012 · 7 comments

Comments

@n1mmy
Copy link

n1mmy commented Sep 11, 2012

As soon as process.stdin is evaluated, it sets the file descriptor to non-blocking IO (O_NONBLOCK). This makes it hard to share stdin with child processes, since many third-party utilities do not expect O_NONBLOCK to be set.

For example, this works fine:

require('child_process').spawn('mongo', [], {stdio: 'inherit'});

But this fails catastrophically (mongo shell prints > over and over, and eats all the CPU):

process.stdin;
require('child_process').spawn('mongo', [], {stdio: 'inherit'});

It would be great if node had a way to convert stdin back to blocking IO. Perhaps something like process.stdin.setBlocking(true) (as discussed in IRC, linked below)

See also:

http://piscisaureus.no.de/libuv/2012-06-29#00:40:38.256 Discussion of a similar issue on IRC, proposing adding setBlocking method.

#3584 Different issue, but with a similar potential resolution.

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=2653 Another example of bad behavior caused by setting stdin to non-blocking.

@bnoordhuis
Copy link
Member

Does this patch help?

diff --git a/deps/uv/src/unix/process.c b/deps/uv/src/unix/process.c
index 829f79a..35c949c 100644
--- a/deps/uv/src/unix/process.c
+++ b/deps/uv/src/unix/process.c
@@ -321,6 +321,9 @@ static void uv__process_child_init(uv_process_options_t options,
       dup2(use_fd, i);
       close(use_fd);
     }
+
+    if (i <= 2)
+      uv__nonblock(i, 0);
   }

   if (options.cwd && chdir(options.cwd)) {

(Note to self: rename i to fd)

@glasser
Copy link

glasser commented Sep 11, 2012

This seems to fix the emacs bug (last link above) though not the Mongo bug:

diff --git a/deps/uv/src/unix/tty.c b/deps/uv/src/unix/tty.c
index 7193db8..da817ff 100644
--- a/deps/uv/src/unix/tty.c
+++ b/deps/uv/src/unix/tty.c
@@ -143,4 +143,5 @@ void uv_tty_reset_mode() {
   if (orig_termios_fd >= 0) {
     tcsetattr(orig_termios_fd, TCSANOW, &orig_termios);
   }
+  uv__nonblock(0, 0);
 }

It's definitely a bit of a hack, though.

@n1mmy
Copy link
Author

n1mmy commented Sep 11, 2012

Thanks for the quick response, @bnoordhuis. The patch you posted does seem to fix the mongo issue for me.

@bnoordhuis
Copy link
Member

Thanks for testing, I've landed it in joyent/libuv@1988f5e. It'll be in the next stable release.

@n1mmy
Copy link
Author

n1mmy commented Sep 12, 2012

Great, thanks!

@glasser
Copy link

glasser commented Sep 12, 2012

Note that this does still leave the "run node REPL in emacs shell, exit it, emacs shell exits too" bug intact, which my patch fixes.

@bnoordhuis
Copy link
Member

Note that this does still leave the "run node REPL in emacs shell, exit it, emacs shell exits too" bug intact, which my patch fixes.

Isn't that an emacs shell bug?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants