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

Child Process: fork stdio option doesn't support the String variant that spawn does #10793

Closed
SEUH opened this issue Jan 13, 2017 · 14 comments
Closed
Labels
child_process Issues and PRs related to the child_process subsystem. feature request Issues that request new features to be added to Node.js. good first issue Issues that are suitable for first-time contributors.

Comments

@SEUH
Copy link

SEUH commented Jan 13, 2017

Version: v7.4.0
Platform: Windows 64 bit

The following code forks a script but all it's stdio objects are null

index.js

var child = childProcess.fork('./userapp/test.js', [], {
  stdio: 'pipe'
});
console.log('stdio', child.stdio);

test.js

var count = 0;
setInterval(function () {
  if (count == 3) process.exit(1);
  console.log('test: ' + count);
  count++;
}, 1000, 0);

console output:

stdio [ null, null, null, null ]
test: 0
test: 1
test: 2

Also i've tried to set the childs stdio with stdio: [stream, stream, stream] but this didn't work and the child used the parents stream to output

@mscdex mscdex added the child_process Issues and PRs related to the child_process subsystem. label Jan 13, 2017
@sam-github sam-github added the feature request Issues that request new features to be added to Node.js. label Jan 13, 2017
@sam-github
Copy link
Contributor

Inconsistent and annoying, but documented, for what that's worth:

stdio Supports the array version of child_process.spawn()'s stdio option.

@sam-github sam-github changed the title Child Process: all stdio streams are null Child Process: fork stdio option doesn't support the String variant that spawn does Jan 13, 2017
@SEUH
Copy link
Author

SEUH commented Jan 13, 2017

@sam-github I see what you mean but this doesn't solve the problem.
Even with the array version provided and 'ipc', it doesn't work

var child = childProcess.fork('./userapp/test.js', config.project.args, {
  stdio: [0,1,2,'ipc']
});
console.log('stdio', child.stdio);

still the same output as before. Also replacing 0,1,2 with writeable streams doesn't work

@sam-github
Copy link
Contributor

@SEUH as-expected.

A sparse array of pipes to the child process, corresponding with positions in the stdio option passed to child_process.spawn() that have been set to the value 'pipe'.

@SEUH
Copy link
Author

SEUH commented Jan 13, 2017

@sam-github I still don't get it. Am i missing something? If i set stdio: ['pipe','pipe','pipe','ipc'] stdio's are still null

@sam-github
Copy link
Contributor

sam-github commented Jan 13, 2017

And to be clear: not-a-problem

child.stdio exists so that the parent and read and write data to the child's stdio.

It isn't possible to write child to a child's input, if the child is getting input from a tty, or an explicit fd, or anything other than if the child is reading input from a pipe connected to the parent.

Same for output: the data the child writes to its stdout or stderr can be read by the parent only if the child's stdout or stderr is connected via a pipe to the parent.

So the docs don't just describe an arbitary limitation, bug, or missing feature. They describe a natural behaviour.

Note that your original description attempted to pipe the child's stdio using 'pipe'. That is a missing feature, because in spawn, that string value is a short-hand for ['pipe', 'pipe', 'pipe']. Your example above isn't equivalent, its the equivalent of what spawn would do with stdio: 'inherit' (also a feature missing from fork(), of course), and very different from 'pipe'.

@sam-github
Copy link
Contributor

> child_process.fork('./f.js', {stdio: ['pipe', 'pipe', 'pipe', 'ipc']}).stdio[0]
Socket {
  connecting: false,
.... etc.

Works for me. Provide a standalone runnable example if you don't think it works.

@SEUH
Copy link
Author

SEUH commented Jan 13, 2017

@sam-github Found the problem...i provided empty args

var child = childProcess.fork('./userapp/test.js', config.project.args, {
  stdio: ['pipe','pipe','pipe','ipc']
});
console.log('stdio', child.stdio);

config.project.args was null.

@sam-github
Copy link
Contributor

sam-github commented Jan 13, 2017

Now that is a bug, IMO:

> child_process.fork('./f.js', null, {stdio: ['pipe', 'pipe', 'pipe', 'ipc']}).stdio
[ null, null, null, null ]

Providing null or undefined as the arg array should either type-error, or just mean "no args" - the latter makes more sense to me, but I would have to check the other array args in the child_process API, and make sure that the handling was consistent.

I suspect it is assuming there is no options if the args are null/undefined ATM, but haven't looked at the code.

@SEUH
Copy link
Author

SEUH commented Jan 13, 2017

@sam-github Right...do you open an issue for this?

@sam-github
Copy link
Contributor

If you did I would really appreciate it. Otherwise I will do so later.

@sam-github
Copy link
Contributor

And if you have interest, both the feature and the bug are quite easy to implement, good first contributions.

@sam-github sam-github added the good first issue Issues that are suitable for first-time contributors. label Jan 13, 2017
@trendsetter37
Copy link
Contributor

@sam-github I can take a stab a this

@sam-github
Copy link
Contributor

@trendsetter37 Can you report the bug, so the bug and the feature are tracked as individual issues? Then say which of the two (both?) you are going to take a stab at. Thanks!

@trendsetter37
Copy link
Contributor

Sure, I'll report the bug and take a stab at both.

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. feature request Issues that request new features to be added to Node.js. good first issue Issues that are suitable for first-time contributors.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants