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

Proposal: child_process: Send socket but keep it open in parent #4271

Closed
adrienverge opened this issue Dec 14, 2015 · 1 comment
Closed

Proposal: child_process: Send socket but keep it open in parent #4271

adrienverge opened this issue Dec 14, 2015 · 1 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.

Comments

@adrienverge
Copy link

The current behavior when sending a net.Socket object to a child process using child_process.ChildProcess.send() is to close the socket in the parent. This way, only the socket receiver can read and write to it.

Problem: in some use-cases, a user may want to have BOTH parent and child to write to the TCP socket. For instance, I'm developing an app where a process writes data to the socket, whereas another one writes metadata. Sharing the socket in C, not in Node.js (currently).

The enhancement I propose is to add an optional keepOpen parameter to specify whether the parent should keep the socket open or not (it would default to false to keep existing behavior).

const child = require('child_process').fork('child.js');
const server = require('net').createServer();

server.listen(1234).on('connection', function (socket) {
    child.send('socket', socket, { keepOpen: true });

    // Currently this crashes with: "Error: This socket is closed."
    socket.write('-- hello from parent --');
});

What I'm asking here is whether this is an acceptable enhancement or not. If it is, I'll start working on a clean pull request.

@ChALkeR ChALkeR added child_process Issues and PRs related to the child_process subsystem. feature request Issues that request new features to be added to Node.js. labels Dec 14, 2015
@bnoordhuis
Copy link
Member

Sounds reasonable to me. I'll review a pull request.

cjihrig added a commit to cjihrig/node that referenced this issue Feb 22, 2016
This commit adds an options object to process.send(). The same
object is propagated to process._send(), the _handleQueue, and the
send() and postSend() functions of the handle converter.

Fixes: nodejs#4271
PR-URL: nodejs#5283
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
cjihrig added a commit to cjihrig/node that referenced this issue Feb 22, 2016
This option allows an instance of net.Socket to be kept open in
the sending process.

Fixes: nodejs#4271
PR-URL: nodejs#5283
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Fishrock123 pushed a commit that referenced this issue Mar 8, 2016
This commit adds an options object to process.send(). The same
object is propagated to process._send(), the _handleQueue, and the
send() and postSend() functions of the handle converter.

Fixes: #4271
PR-URL: #5283
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Fishrock123 pushed a commit that referenced this issue Mar 8, 2016
This option allows an instance of net.Socket to be kept open in
the sending process.

Fixes: #4271
PR-URL: #5283
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Fishrock123 pushed a commit that referenced this issue Mar 8, 2016
This commit adds an options object to process.send(). The same
object is propagated to process._send(), the _handleQueue, and the
send() and postSend() functions of the handle converter.

Fixes: #4271
PR-URL: #5283
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Fishrock123 pushed a commit that referenced this issue Mar 8, 2016
This option allows an instance of net.Socket to be kept open in
the sending process.

Fixes: #4271
PR-URL: #5283
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
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.
Projects
None yet
Development

No branches or pull requests

3 participants