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

Accessing process.stdin hangs on Windows #10836

Closed
segrey opened this issue Jan 16, 2017 · 8 comments
Closed

Accessing process.stdin hangs on Windows #10836

segrey opened this issue Jan 16, 2017 · 8 comments
Labels
child_process Issues and PRs related to the child_process subsystem. windows Issues and PRs related to the Windows platform.

Comments

@segrey
Copy link

segrey commented Jan 16, 2017

Let's consider this example project consisting of 3 files:

// main.js
var spawn = require('child_process').spawn;
var middle = spawn(process.execPath, ['middle.js'], {
    'stdio': 'pipe'
});

middle.stdout.on('data', function(data) {
    process.stdout.write('stdout from middle: ' + data);
});

middle.stderr.on('data', function(data) {
    process.stdout.write('stderr from middle: ' + data);
});

middle.on('close', function(code) {
    process.stdout.write('middle process finished with exit code ' + code);
});
// middle.js
var spawn = require('child_process').spawn;
var stdin = process.stdin;
console.log('middle: isatty(stdin)=' +require('tty').isatty(stdin));
spawn(process.execPath, ['child.js'], {
    'stdio': [process.stdin, process.stdout, process.stderr]
});
// child.js
console.log('child: accessing process.stdin');
console.log('child: done successfully ' + process.stdin);

Running node main.js in cmd.exe should normally output these lines and exit

stdout from middle: middle: isatty(stdin)=false
stdout from middle: child: accessing process.stdin
stdout from middle: child: done successfully [object Object]
middle process finished with exit code 0

However, from time to time it outputs only two first lines and hangs:

stdout from middle: middle: isatty(stdin)=false
stdout from middle: child: accessing process.stdin

The hanging is caused by accessing process.stdin in child.js. Please note it works fine if process.stdin is replaced with 'pipe' here:

// middle.js
...
spawn(process.execPath, ['child.js'], {
    'stdio': ['pipe', process.stdout, process.stderr]
});

Node.js v7.4.0
OS Windows 10

@addaleax addaleax added child_process Issues and PRs related to the child_process subsystem. windows Issues and PRs related to the Windows platform. labels Jan 16, 2017
@addaleax
Copy link
Member

/cc @nodejs/platform-windows

@seishun
Copy link
Contributor

seishun commented Jan 16, 2017

See also #5620

@Fishrock123
Copy link
Contributor

Does changing the spawn option to 'inherit' for stdin make any difference?

@segrey
Copy link
Author

segrey commented Jan 16, 2017

@Fishrock123 If you meant changing middle.js, like

// middle.js
...
spawn(process.execPath, ['child.js'], {
    'stdio': 'inherit'
});

Then, no, it doesn't make any difference for me.
As for changing main.js, it's will change the behavior, yes. However, middle.js is executed with piped standard streams (not tty) in my environment, so it's impossible to make this change for me.

@Fishrock123
Copy link
Contributor

If you meant changing middle.js, like

Correct. Just making sure there wasn't any hidden Windows logic in inherit.

bzoz added a commit to JaneaSystems/libuv that referenced this issue Mar 23, 2017
Add a thread that will interrupt uv_pipe_zero_readdile_thread_proc
every half second. This allows other processes to access the pipe
without deadlocking

Ref: nodejs/node#10836

warning: refname 'HEAD' is ambiguous.
bzoz added a commit to JaneaSystems/libuv that referenced this issue Mar 23, 2017
Add a thread that will interrupt uv_pipe_zero_readdile_thread_proc
every half second. This allows other processes to access the pipe
without deadlocking

Ref: joyent/libuv#1313
Ref: nodejs/node#10836
bzoz added a commit to JaneaSystems/libuv that referenced this issue Apr 26, 2017
Add a thread that will interrupt uv_pipe_zero_readdile_thread_proc every half
second. This allows other processes to access the pipe without deadlocking

Ref: nodejs/node#10836
bzoz added a commit to JaneaSystems/libuv that referenced this issue Apr 27, 2017
Add a thread that will interrupt uv_pipe_zero_readdile_thread_proc every half
second. This allows other processes to access the pipe without deadlocking

Ref: nodejs/node#10836
bzoz added a commit to JaneaSystems/libuv that referenced this issue May 11, 2017
Add a thread that will interrupt uv_pipe_zero_readdile_thread_proc every two
and half second. This allows other processes to access the pipe without
deadlocking

Ref: nodejs/node#10836
@Trott
Copy link
Member

Trott commented Jul 15, 2017

@bzoz Is fixing this likely blocked by until libuv/libuv#1273 or equivalent lands in libuv and the appropriate version of libuv gets upstreamed to our deps directory?

@bzoz
Copy link
Contributor

bzoz commented Jul 17, 2017

Yes

bzoz added a commit to JaneaSystems/libuv that referenced this issue Aug 8, 2017
Add a thread that will interrupt uv_pipe_zero_readdile_thread_proc every two
and half second. This allows other processes to access the pipe without
deadlocking

Ref: nodejs/node#10836
bzoz added a commit to JaneaSystems/libuv that referenced this issue Aug 18, 2017
Add a thread that will interrupt uv_pipe_zero_readdile_thread_proc every two
and half second. This allows other processes to access the pipe without
deadlocking

Ref: nodejs/node#10836
bzoz added a commit to JaneaSystems/libuv that referenced this issue Sep 6, 2017
Add a thread that will interrupt uv_pipe_zero_readdile_thread_proc every two
and half second. This allows other processes to access the pipe without
deadlocking

Ref: nodejs/node#10836
bzoz added a commit to JaneaSystems/libuv that referenced this issue Sep 19, 2017
Add a thread that will interrupt uv_pipe_zero_readdile_thread_proc every two
and half second. This allows other processes to access the pipe without
deadlocking

Ref: nodejs/node#10836
@bzoz
Copy link
Contributor

bzoz commented Feb 7, 2018

This does not reproduce any longer, it would seem it was fixed in v8.3.0.
I've made a test for this in #18614

@bzoz bzoz closed this as completed Feb 7, 2018
bzoz added a commit to JaneaSystems/node that referenced this issue Feb 12, 2018
Add two regression tests for stdio over pipes.

test-stdio-pipe-access tests if accessing stdio pipe that is being read
by another process does not deadlocks Node.js. This was reported in
nodejs#10836 and was fixed in v8.3.0.
The deadlock would happen intermittently, so we run the test 5 times.

test-stdio-pipe-redirect tests if redirecting one child process stdin to
another process stdout does not crash Node as reported in
nodejs#17493. It was fixed in
nodejs#18019.
BridgeAR pushed a commit to BridgeAR/node that referenced this issue Feb 17, 2018
Add two regression tests for stdio over pipes.

test-stdio-pipe-access tests if accessing stdio pipe that is being read
by another process does not deadlocks Node.js. This was reported in
nodejs#10836 and was fixed in v8.3.0.
The deadlock would happen intermittently, so we run the test 5 times.

test-stdio-pipe-redirect tests if redirecting one child process stdin to
another process stdout does not crash Node as reported in
nodejs#17493. It was fixed in
nodejs#18019.

PR-URL: nodejs#18614
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this issue Feb 21, 2018
Add two regression tests for stdio over pipes.

test-stdio-pipe-access tests if accessing stdio pipe that is being read
by another process does not deadlocks Node.js. This was reported in
#10836 and was fixed in v8.3.0.
The deadlock would happen intermittently, so we run the test 5 times.

test-stdio-pipe-redirect tests if redirecting one child process stdin to
another process stdout does not crash Node as reported in
#17493. It was fixed in
#18019.

PR-URL: #18614
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
yhwang pushed a commit to yhwang/node that referenced this issue Feb 22, 2018
Add two regression tests for stdio over pipes.

test-stdio-pipe-access tests if accessing stdio pipe that is being read
by another process does not deadlocks Node.js. This was reported in
nodejs#10836 and was fixed in v8.3.0.
The deadlock would happen intermittently, so we run the test 5 times.

test-stdio-pipe-redirect tests if redirecting one child process stdin to
another process stdout does not crash Node as reported in
nodejs#17493. It was fixed in
nodejs#18019.

PR-URL: nodejs#18614
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
addaleax pushed a commit that referenced this issue Feb 26, 2018
Add two regression tests for stdio over pipes.

test-stdio-pipe-access tests if accessing stdio pipe that is being read
by another process does not deadlocks Node.js. This was reported in
#10836 and was fixed in v8.3.0.
The deadlock would happen intermittently, so we run the test 5 times.

test-stdio-pipe-redirect tests if redirecting one child process stdin to
another process stdout does not crash Node as reported in
#17493. It was fixed in
#18019.

PR-URL: #18614
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
addaleax pushed a commit that referenced this issue Feb 26, 2018
Add two regression tests for stdio over pipes.

test-stdio-pipe-access tests if accessing stdio pipe that is being read
by another process does not deadlocks Node.js. This was reported in
#10836 and was fixed in v8.3.0.
The deadlock would happen intermittently, so we run the test 5 times.

test-stdio-pipe-redirect tests if redirecting one child process stdin to
another process stdout does not crash Node as reported in
#17493. It was fixed in
#18019.

PR-URL: #18614
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this issue Feb 26, 2018
Add two regression tests for stdio over pipes.

test-stdio-pipe-access tests if accessing stdio pipe that is being read
by another process does not deadlocks Node.js. This was reported in
#10836 and was fixed in v8.3.0.
The deadlock would happen intermittently, so we run the test 5 times.

test-stdio-pipe-redirect tests if redirecting one child process stdin to
another process stdout does not crash Node as reported in
#17493. It was fixed in
#18019.

PR-URL: #18614
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MayaLekova pushed a commit to MayaLekova/node that referenced this issue May 8, 2018
Add two regression tests for stdio over pipes.

test-stdio-pipe-access tests if accessing stdio pipe that is being read
by another process does not deadlocks Node.js. This was reported in
nodejs#10836 and was fixed in v8.3.0.
The deadlock would happen intermittently, so we run the test 5 times.

test-stdio-pipe-redirect tests if redirecting one child process stdin to
another process stdout does not crash Node as reported in
nodejs#17493. It was fixed in
nodejs#18019.

PR-URL: nodejs#18614
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this issue Aug 7, 2018
Add two regression tests for stdio over pipes.

test-stdio-pipe-access tests if accessing stdio pipe that is being read
by another process does not deadlocks Node.js. This was reported in
#10836 and was fixed in v8.3.0.
The deadlock would happen intermittently, so we run the test 5 times.

test-stdio-pipe-redirect tests if redirecting one child process stdin to
another process stdout does not crash Node as reported in
#17493. It was fixed in
#18019.

PR-URL: #18614
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this issue Aug 7, 2018
Add two regression tests for stdio over pipes.

test-stdio-pipe-access tests if accessing stdio pipe that is being read
by another process does not deadlocks Node.js. This was reported in
#10836 and was fixed in v8.3.0.
The deadlock would happen intermittently, so we run the test 5 times.

test-stdio-pipe-redirect tests if redirecting one child process stdin to
another process stdout does not crash Node as reported in
#17493. It was fixed in
#18019.

PR-URL: #18614
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this issue Aug 9, 2018
Add two regression tests for stdio over pipes.

test-stdio-pipe-access tests if accessing stdio pipe that is being read
by another process does not deadlocks Node.js. This was reported in
#10836 and was fixed in v8.3.0.
The deadlock would happen intermittently, so we run the test 5 times.

test-stdio-pipe-redirect tests if redirecting one child process stdin to
another process stdout does not crash Node as reported in
#17493. It was fixed in
#18019.

PR-URL: #18614
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this issue Aug 16, 2018
Add two regression tests for stdio over pipes.

test-stdio-pipe-access tests if accessing stdio pipe that is being read
by another process does not deadlocks Node.js. This was reported in
#10836 and was fixed in v8.3.0.
The deadlock would happen intermittently, so we run the test 5 times.

test-stdio-pipe-redirect tests if redirecting one child process stdin to
another process stdout does not crash Node as reported in
#17493. It was fixed in
#18019.

PR-URL: #18614
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
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. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

No branches or pull requests

6 participants