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

detached child_process doesn't inherit stdio #3596

Closed
yepitschunked opened this issue Oct 29, 2015 · 19 comments
Closed

detached child_process doesn't inherit stdio #3596

yepitschunked opened this issue Oct 29, 2015 · 19 comments
Labels
child_process Issues and PRs related to the child_process subsystem. macos Issues and PRs related to the macOS platform / OSX.

Comments

@yepitschunked
Copy link

The following snippet of code prints the child process' output if detached is true, and doesn't if it's false:

  cmd = '...'
  child_process.spawn(cmd, [], {
    stdio: 'inherit',
    detached: true
  });

This part of the docs seems relevant:

If the parent's stdio is inherited, the child will remain attached to the controlling terminal.

Which seems to imply that inheriting these streams is supported. Not sure if this is just me misreading the docs or...

@mscdex mscdex added the child_process Issues and PRs related to the child_process subsystem. label Oct 30, 2015
@bnoordhuis
Copy link
Member

{ detached: true } is sugar for setsid(2), meaning it disconnects from the controlling tty. Does that answer you question? If not, can you post a complete example with expected and actual output?

@cjihrig
Copy link
Contributor

cjihrig commented Oct 30, 2015

@bnoordhuis is this expected behavior? With Node 5 on OS X, and the following code:

process.stdout;
require('child_process').spawn('ls', [], {
  stdio: 'inherit',
  detached: true
});

Nothing is printed. However, if I comment out the process.stdout on the first line, then ls output is displayed.

@bnoordhuis
Copy link
Member

/cc @indutny - I'm guessing it's related to OS X's broken kqueue support for special files and libuv's workaround. I can reproduce on OS X but it works fine on Linux, FWIW.

@indutny
Copy link
Member

indutny commented Oct 31, 2015

The only explanation that I have is that reopening stdout as /dev/tty somehow interferes with it.

@indutny
Copy link
Member

indutny commented Dec 21, 2015

Gosh, it is twice as odd, considering that piping the output like this: node test.js | less works just fine in both cases. I suspect some OS X oddness atm...

@indutny
Copy link
Member

indutny commented Dec 21, 2015

And of course it is related to the reopening of /dev/tty in libuv: https://github.com/libuv/libuv/blob/25506bb33156f15e27a837979773264dfe8d06be/src/unix/tty.c#L64-L87 . Removing that branch fixes the problem.

@indutny
Copy link
Member

indutny commented Dec 22, 2015

I think there is some fishy stuff with the current session and the way /dev/tty attaches to it on OS X, but I am not yet quite sure what exactly is happening there.

@indutny
Copy link
Member

indutny commented Dec 22, 2015

It looks like for a child process write returns EIO:

write(0x1, "\0", 0x1)        = -1 Err#5

@indutny
Copy link
Member

indutny commented Dec 22, 2015

@wonnage may I ask you to give a try to this patch?

diff --git a/deps/uv/src/unix/process.c b/deps/uv/src/unix/process.c
index 571f8cd..c02c750 100644
--- a/deps/uv/src/unix/process.c
+++ b/deps/uv/src/unix/process.c
@@ -283,8 +283,10 @@ static void uv__process_child_init(const uv_process_options_t* options,
   int use_fd;
   int fd;

-  if (options->flags & UV_PROCESS_DETACHED)
+  if (options->flags & UV_PROCESS_DETACHED) {
+    setpgid(getpid(), getpid());
     setsid();
+  }

   /* First duplicate low numbered fds, since it's not safe to duplicate them,
    * they could get replaced. Example: swapping stdout and stderr; without

I suspect that this is an OS X kernel bug and that for some reason pg->pg_jobc is 0 if setsid() is called alone.

@indutny
Copy link
Member

indutny commented Dec 23, 2015

Gosh, I have figured it out. The actual device behind /dev/tty lives in bsd/kern/tty_tty.c. The thing about this device is that it always looks up the tty using current proc's session. So when you create a new session, it doesn't have any tty assigned to it, and all syscalls on it will result in EIO error.

I'll need some time to think if there is any workaround for this.

@indutny
Copy link
Member

indutny commented Dec 23, 2015

I failed at it miserably :( Submitted Apple bugreport # 23994737

@yepitschunked
Copy link
Author

Hey! sorry i've been inactive on this thread - switched jobs and no longer working on the code that produced this error in the first place :)

@yepitschunked
Copy link
Author

handwaving through my ignorance of unix internals a bit, but is this a correct summary of the issue:

we start with a parent process hooked up to a tty
setsid makes the new process lose controlling terminal (/dev/tty isn't backed up by anything)

afterwards, piping node test.js | less works because stdout is going to stdin of less; less is in the same session as parent and writes to controlling tty just fine, and we get output.

however, if stdout is a tty (node test.js), the uv change you linked attempts to reopen /dev/tty, but at this point /dev/tty doesn't point anywhere in the new process (due to OSX). is there some way to save a reference to correct tty and pass that to the child process in uv_tty_init when calling uv__open_cloexec?

@indutny
Copy link
Member

indutny commented Jan 22, 2016

@wonnage I believe piping works because it is not using tty at all, it is using pipes.

@cjihrig cjihrig added the macos Issues and PRs related to the macOS platform / OSX. label Mar 4, 2016
@aoberoi
Copy link
Contributor

aoberoi commented Mar 25, 2016

@indutny would you mind sharing the apple bug report on open radar?

@indutny
Copy link
Member

indutny commented Mar 25, 2016

@jasnell
Copy link
Member

jasnell commented Jun 7, 2016

Curious if this is fixed with libuv 1.9.x

@jasnell
Copy link
Member

jasnell commented Jun 7, 2016

Answering my own question... running @cjihrig's test case on v6+ works just fine.

Closing as resolved.

@craigsdennis
Copy link

Seeing something very similar with stdin on Windows and Node v6.7.1 anyone know the root of this and if there is another bug not linked here that might contain what the fix was?

cc/ @chalkers

petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Apr 16, 2020
The change in e041ac6
to support sending unlocker process output to the main ngcc
console output prevents messages require that the main process
relinquishes the event-loop to allow the `stdout.on()` handler to
run.  This results in none of the messages being written when ngcc
is run in `--no-async` mode, and some messages failing to be
written if the main process is killed (e.g. ctrl-C).

It appears that the problem with Windows and detached processes
is known - see nodejs/node#3596 (comment).
But in the meantime, this commit is a workaround, where non-Windows
`inherit` the main process `stdout` while on Windows it reverts
to the async handler approach, which is better than nothing.
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Apr 16, 2020
The change in e041ac6
to support sending unlocker process output to the main ngcc
console output prevents messages require that the main process
relinquishes the event-loop to allow the `stdout.on()` handler to
run.  This results in none of the messages being written when ngcc
is run in `--no-async` mode, and some messages failing to be
written if the main process is killed (e.g. ctrl-C).

It appears that the problem with Windows and detached processes
is known - see nodejs/node#3596 (comment).
But in the meantime, this commit is a workaround, where non-Windows
`inherit` the main process `stdout` while on Windows it reverts
to the async handler approach, which is better than nothing.
matsko pushed a commit to angular/angular that referenced this issue Apr 16, 2020
The change in e041ac6
to support sending unlocker process output to the main ngcc
console output prevents messages require that the main process
relinquishes the event-loop to allow the `stdout.on()` handler to
run.  This results in none of the messages being written when ngcc
is run in `--no-async` mode, and some messages failing to be
written if the main process is killed (e.g. ctrl-C).

It appears that the problem with Windows and detached processes
is known - see nodejs/node#3596 (comment).
But in the meantime, this commit is a workaround, where non-Windows
`inherit` the main process `stdout` while on Windows it reverts
to the async handler approach, which is better than nothing.

PR Close #36637
matsko pushed a commit to angular/angular that referenced this issue Apr 16, 2020
The change in e041ac6
to support sending unlocker process output to the main ngcc
console output prevents messages require that the main process
relinquishes the event-loop to allow the `stdout.on()` handler to
run.  This results in none of the messages being written when ngcc
is run in `--no-async` mode, and some messages failing to be
written if the main process is killed (e.g. ctrl-C).

It appears that the problem with Windows and detached processes
is known - see nodejs/node#3596 (comment).
But in the meantime, this commit is a workaround, where non-Windows
`inherit` the main process `stdout` while on Windows it reverts
to the async handler approach, which is better than nothing.

PR Close #36637
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. macos Issues and PRs related to the macOS platform / OSX.
Projects
None yet
Development

No branches or pull requests

8 participants