process.send() is not synchronous

[maloks]

According to the documentation for the child_process module, process.send() should block. It seems that this is no longer the case in v1.1.0.

//parent.js

var fork = require('child_process').fork;
var child = fork('./child.js');
child.on('message', function (m) {
    console.log('got message from child: ' + m);
});

//child.js

process.send(new Buffer(2048));
process.exit(0);

In this example, if the child process attempts to send a sufficiently large object to the parent, the child process exits but does not send all of the data. However, smaller messages (perhaps a Buffer of 1024 bytes) are successfully sent.

I think this change in functionality was introduced in 07bd05b when uv__nonblock() was added to uv_pipe_open() in deps/uv/src/unix/pipe.c. Removing the call to uv__nonblock() restores the original behavior of process.send().

I ran into this issue on OSX 10.10.2 LLVM 6.0 (clang-600.0.56), if it helps.

[brendanashworth]

Running also on OSX, I can confirm that this indeed happens - for me, 2048 sends but 4096 does not. This is noted in the documentation:

Please note that the send() method on both the parent and child are synchronous - sending large chunks of data is not advised (pipes can be used instead, see child_process.spawn).

Running on node v0.11.14, all the data up to 16384 was sent on my machine.

[maloks]

Thanks for confirming. When I read the documentation, I figured that it was an advisory about the blocking nature of send() and the resulting performance impact, but not that there is a restriction on how much data can be sent in a single synchronous call. With node v0.12.0 I'm able to send back megabytes of data to the parent process.

If I remove the call to process.exit() in my example, all of the data is transmitted. This leads me to believe that during the initial call to process.send(), a portion of the data is sent to the parent process, but the rest is placed into the event queue to be sent later. process.exit() then terminates the child process without allowing the rest of the queued data to be sent.

[brendanashworth]

Looks like as per libuv/libuv#393c1c5:

the file descriptor should already be in non-blocking mode before passing it to libuv

Which, I think to set it into non-blocking mode (or blocking in this case), you have to call pipe.setBlocking(), which would have to be done here.

Of course, when I change that, it doesn't fix the issue for me, but I'll leave my work here in case anyone wants to build off that.

diff --git a/lib/child_process.js b/lib/child_process.js
index 5fcd260..fe15032 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -567,6 +567,7 @@ exports._forkChild = function(fd) {
   var p = createPipe(true);
   p.open(fd);
   p.unref();
+  p.setBlocking(true);
   setupChannel(process, p);

   var refs = 0;

/cc @bnoordhuis

[saghul]

FWIW, setBlocking probably calls uv_stream_set_blocking, which is only implemented on Windows and only for named pipes.

[Fishrock123]

Current status: (#774 (comment))

I think I'd rather take a different approach and make the asynchronous nature of process.send() official.

We'd need a docs pr then I think, but this may be rediscussed as per #34

[bpasero]

Can someone summarize how process.send behaves in io.js today? If it uses non blocking pipes, it should be documented as such and if it waits until the process receives the data, too. Also, I wonder if there are now OS differences in the way this works.

[rvagg]

Moving discussion from nodejs/node-convergence-archive#34 and added to 4.0.0 milestone.

@bnoordhuis @piscisaureus what are the chances of getting a change in that would unify asynchronous behaviour across platforms within the next week or so? Are we going to have to punt this till 5.0.0 and if so, how painful will that be for users of v4 as it heads in to LTS?

[rvagg]

ref nodejs/node-v0.x-archive@2411bea where it was made sync in 0.12

[rvagg]

@bnoordhuis can I get you to take responsibility for doing something about sync/async send() this week please? Your latest comment in nodejs/node-convergence-archive#34 was that you thought making sure it's async on all platforms was the way to go and therefore documentation should note this. I'd lgtm any PR that takes this off our known issues list, no matter which way it falls.

[rvagg]

ping @bnoordhuis do we have docs for this yet?

[Fishrock123]

New functionality for being async landed in 56d9584 with docs.

[rvagg]

woo! thanks @bnoordhuis!