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

NodeJS cannot spawn a process that reads from STDIN #1774

Closed
soda0289 opened this issue Mar 13, 2017 · 26 comments
Closed

NodeJS cannot spawn a process that reads from STDIN #1774

soda0289 opened this issue Mar 13, 2017 · 26 comments
Assignees

Comments

@soda0289
Copy link

  • A brief description
    Using node.js v4.7.3 it is not possible to spawn a process that reads from stdin. An exception occurs that is only reproducible on Windows. On linux it works as expected.

  • Expected results
    Should spawn process that can read from stdin

  • Actual results (with terminal output if applicable)
    An exception occurs

node.js:724
          throw new Error('Implement me. Unknown stdin file type!');
          ^

Error: Implement me. Unknown stdin file type!
    at process.stdin (node.js:724:17)
    at Object.<anonymous> (/mnt/c/Users/reyad.attiyat/Workspace/node-test/readStdin.js:2:8)
    at Module._compile (module.js:409:26)
    at Object.Module._extensions..js (module.js:416:10)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)
    at Function.Module.runMain (module.js:441:10)
    at startup (node.js:139:18)
    at node.js:990:3

  • Your Windows build number
    15055
  • Steps / All commands required to reproduce the error from a brand new installation
    Create file spawn.js
const spawn = require('child_process').spawn;

const node = spawn('node', ['./readStdin.js'], {stdio: ['pipe', 'pipe', 'pipe'], shell: true});

node.stdout.on('data', (output) => {
    console.log(output.toString());
});

node.stderr.on('data', (output) => {
    console.log(output.toString());
});

node.stdin.setEncoding('utf8');
node.stdin.write('HELLO WORLD');
node.stdin.end();

Create file readStdin.js

process.stdin.resume();
process.stdin.setEncoding('utf8');
process.stdin.on('data', (input) => {
    console.log(input.toString());
});

Run command: node spawn.js

Here is the related node.js issue for the same problem on windows:
nodejs/node#11656

I would expect it to behave the same on bash for windows as it does in linux

@soda0289
Copy link
Author

I think this might be a bug with Node.JS because the following example works as expected:

var child_process = require('child_process');

/**
 * Create the child process, with output piped to the script's stdout
 */
var wc = child_process.spawn('wc');

/**
 * Write some data to stdin, and then use stream.end() to signal that we're
 * done writing data.
 */
wc.stdin.write('test test');
wc.stdin.end();
wc.stdout.on('data', (output) => {
    console.log(output.toString());
});

@gireeshpunathil
Copy link

I don't have a bash on ubuntu on windows environment. Is this an intermittent issue, or a consistent behavior?

@gireeshpunathil
Copy link

If I take the strace as a bona-fide replica of what happens in a Linux OS, here are the relevant portions pertinent to the piping descriptors and actions on them:

// pipe meant to be for piping child's stdin
socketpair(PF_LOCAL, SOCK_STREAM|SOCK_CLOEXEC, 0, [10, 11]) = 0

// pipe meant to be for piping child's stdout
socketpair(PF_LOCAL, SOCK_STREAM|SOCK_CLOEXEC, 0, [12, 13]) = 0

// pipe meant to be for piping child's stderr
socketpair(PF_LOCAL, SOCK_STREAM|SOCK_CLOEXEC, 0, [14, 15]) = 0


// child clone
clone(child_stack=0, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7f8cb0010a10) = 4612

// Closing parent side of the forked pipe
close(11)                               = 0

// Closing parent side of the forked pipe
close(13)                               = 0

// Closing parent side of the forked pipe
close(15)                               = 0

// writing into child's stdin
write(10, "HELLO WORLD", 11)            = 11

// close parent side pipe of child's stdin
close(10)

Under race condition scenarios, it is possible that the child's stdin (peer of 10) is closed before the child could even be spawned, and hence the error.

Can you add a delay in parent and check? for example:

node.stdin.setEncoding('utf8');
node.stdin.write('HELLO WORLD');
setTimeout(function() {
  node.stdin.end();
}, 1000);

@soda0289
Copy link
Author

@gireeshpunathil This bug happens every time on Bash on Windows.

I tested your suggestion and it works. The timeout on the node.stdin.end() allows for the code to work as expected. Thanks

Why do you think this problem occurs on Bash On Windows, Windows 7, and Windows Server 2012? I have tested the code Windows 10 and Debian Linux it works on those systems. Is it just because those operating systems hang on to the stdin fd longer. Do you think it is a bug with node.js?

@gireeshpunathil
Copy link

In terms of the code logic: the stdin which we are talking about is the parent end of a pipe which was created bewteen the two processes. If you want the child to read from parent and write back, the 3 pipes needs to be kept opened. If you think the stdin.end() marks the end of writing to the child, which later the child can resume, you should use pause(). end() closes the parent side socket and breaks the pipe, causing exception in the child when it processes its side. The working test case which you provided has a non-node child, which does not make use of stdin so no issues.

In terms of why this happens only in few systems: the question is when the parent clones a child, which process gets the CPU in the immediate scheduling interval which follows, and who gets the chance to execute first. If the child runs, it initializes its side of stdin and succeeds in reading. If the parent gets the CPU, it writes to child and closes the pipe. So the reason is probably deeply rooted in the way the scheduling works in different systems, in addition to the load in the system, adding to the race condition.

In terms of whether it is an issue with the node: yes and no. Yes in terms of not handling an invalid fd at the startup - probably it could detect this and attach the null device to it. No in terms of the parent being responsible for maintaining the child streams healthy. On an HTTP stream, this is synonymous to doing response.end() followed by response.write(), which causes a similar fault. However, I recommend to raise it with node.js project.

@soda0289
Copy link
Author

@gireeshpunathil Thanks for the response. Your logic makes sense. The reason I wanted to call end() was to let the child process know when all the data has been sent. I can send a null byte instead which is a better solution.

This is not a bug in Bash on Windows so I will close this issue.

@mindplay-dk
Copy link

@soda0289

This is not a bug in Bash on Windows so I will close this issue.

The node folks may disagree (?)

If this is not a bug in WSL, where's the bug?

It's really crucial we pointpoint the cause and figure out how to fix this - node is an important part of the Linux eco-system, and one of the key reasons people use WSL in the first place.

@benhillis
Copy link
Member

I happen to disagree with the node folks, they are being too strict about what types of file handles they allow as standard handles. But that's just like, my opinion, man...

Regardless, this is fixed in recent skip-ahead insider builds.

@mindplay-dk
Copy link

Regardless, this is fixed in recent skip-ahead insider builds.

@benhillis to get this now, I just sign up for Windows 10 Insider Preview and install the Preview build? (anything else or is that it?)

@k00k
Copy link

k00k commented Jun 22, 2018

Regardless, this is fixed in recent skip-ahead insider builds.

@benhillis which build number is it fixed in?

@benhillis
Copy link
Member

@k00k - 17618

@k00k
Copy link

k00k commented Jun 23, 2018

@benhillis I'm running 17686.1003 and I still see the problem. Error [ERR_UNKNOWN_STDIN_TYPE]: Unknown stdin file type - at process.getStdin [as stdin] (internal/process/stdio.js:90:15)

@benhillis
Copy link
Member

@k00k - interesting. Are you doing the same thing as the original post? If not can you please open a new issue?

@k00k
Copy link

k00k commented Jun 23, 2018

@benhillis Originally, I was doing something a bit different, I was/am using Apex Up to start my app which basically just proxies over to run my app. My app runs fine if I don't start with up start, but I get the error when I do.

In any case, to test I did the spawn.js and readStdin.js test that @soda0289 outlined in his original post and I do indeed still get the error using node 8.10 on Win10x64. Pro Insider preview Build 17686.rs_prerelease.180603-1447. For WSL, I'm using Ubuntu.

@zanona
Copy link

zanona commented Jun 24, 2018

I am also experiencing this issue as mentioned under #3307 (which was forwarded to this issue).
My windows build is 17686.1003

@benhillis
Copy link
Member

benhillis commented Jun 27, 2018

When I originally read over this issue I misinterpreted and thought you were using the Windows version of node. This is a different issue and I'm taking a look now.

@benhillis benhillis reopened this Jun 27, 2018
@benhillis
Copy link
Member

I tracked this down to an issue with getsockaddr for unix sockets. Fix will be coming in an upcoming Insiders build.

@benhillis benhillis self-assigned this Jun 27, 2018
@gireeshpunathil
Copy link

awesome news, thansk @benhillis ! mind sharing a summary of the issue?

@Brian-Perkins
Copy link

This should be fixed in Insider Build 17713

@noinkling
Copy link

noinkling commented Aug 19, 2018

I have an issue that seems like it could be related:

In child.js:

let input = '';

process.stdin.on('readable', () => {
  let chunk;
  while (chunk = process.stdin.read()) input += chunk;
});

process.stdin.on('end', () => {
  console.log(input);
});

Run:

node -e "child_process.execSync('node child.js', {stdio: ['pipe','inherit','inherit'], input:'Hello'})"

The child_process.*Sync() functions are supposed to close the stdin stream. In WSL the process hangs (and doesn't print anything). On other platforms (including Windows) it prints "Hello" and exits like it should.

Windows version: 10.0.17134.228
Node version: 10.9.0

I didn't want to create a new issue in case this was just symptom of the same thing and/or was already fixed in an insider build (like the one mentioned just above). Let me know if this is still a problem, and if so, if I should open a new issue for it.

@therealkenc
Copy link
Collaborator

Let me know if this is still a problem

Seems okay (prints "Hello") on 18219.

@noinkling
Copy link

Thanks @therealkenc.

I actually now have reason to believe that it was working at some point on my machine. For my peace of mind can anyone run that snippet against stable (build 17134) and confirm that it does in fact hang for you too?

@zanona
Copy link

zanona commented Aug 20, 2018

@noinkling it just hangs for me? build 17134

@noinkling
Copy link

@zanona Thanks for checking. I was concerned it might be just me.

@therealkenc
Copy link
Collaborator

Yes this would have manifested from when interop was introduced in 14951, until 17713. Note the OP for this issue was 15055.

@noinkling
Copy link

Just confirming that my specific case is indeed fixed as of the October update (build 17763).

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

No branches or pull requests

9 participants