From 4456a333eeb5c3be428f4ee1e0dd460c2b887afa Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sun, 25 Oct 2015 11:48:41 -0700 Subject: [PATCH 1/5] lib,doc: return boolean from child.send() The documentation indicates that child.send() returns a boolean but it has returned undefinined at least since io.js v1. It now returns a boolean per the (slightly updated) documentation. --- doc/api/child_process.markdown | 5 +++-- lib/internal/child_process.js | 6 +++--- test/parallel/test-fork-send-returns-boolean.js | 9 +++++++++ 3 files changed, 15 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-fork-send-returns-boolean.js diff --git a/doc/api/child_process.markdown b/doc/api/child_process.markdown index 13997d65452909..954354cc32a810 100644 --- a/doc/api/child_process.markdown +++ b/doc/api/child_process.markdown @@ -266,9 +266,10 @@ argument: `null` on success, or an `Error` object on failure. `child.send()` emits an `'error'` event if no callback was given and the message cannot be sent, for example because the child process has already exited. -Returns `true` under normal circumstances or `false` when the backlog of +`child.send()` returns `false` if the channel has closed or when the backlog of unsent messages exceeds a threshold that makes it unwise to send more. -Use the callback mechanism to implement flow control. +Otherwise, it returns `true`. Use the callback mechanism to implement flow +control. #### Example: sending server object diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js index 8c1abc5f746fc8..9858245ccdd929 100644 --- a/lib/internal/child_process.js +++ b/lib/internal/child_process.js @@ -504,8 +504,7 @@ function setupChannel(target, channel) { handle = undefined; } if (this.connected) { - this._send(message, handle, false, callback); - return; + return this._send(message, handle, false, callback); } const ex = new Error('channel closed'); if (typeof callback === 'function') { @@ -513,6 +512,7 @@ function setupChannel(target, channel) { } else { this.emit('error', ex); // FIXME(bnoordhuis) Defer to next tick. } + return false; }; target._send = function(message, handle, swallowErrors, callback) { @@ -577,7 +577,7 @@ function setupChannel(target, channel) { handle: null, message: message, }); - return; + return this._handleQueue.length < (65536 * 2); } var req = new WriteWrap(); diff --git a/test/parallel/test-fork-send-returns-boolean.js b/test/parallel/test-fork-send-returns-boolean.js new file mode 100644 index 00000000000000..1a2dbfcedc899b --- /dev/null +++ b/test/parallel/test-fork-send-returns-boolean.js @@ -0,0 +1,9 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const fork = require('child_process').fork; + +const n = fork(common.fixturesDir + '/empty.js'); + +const rv = n.send({ hello: 'world' }); +assert.strictEqual(typeof rv, 'boolean'); From 2779bfbab82fad1aff06831c805be08b941524e0 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sun, 25 Oct 2015 11:54:54 -0700 Subject: [PATCH 2/5] fixup: it is going to be true --- test/parallel/test-fork-send-returns-boolean.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-fork-send-returns-boolean.js b/test/parallel/test-fork-send-returns-boolean.js index 1a2dbfcedc899b..b751846947822c 100644 --- a/test/parallel/test-fork-send-returns-boolean.js +++ b/test/parallel/test-fork-send-returns-boolean.js @@ -6,4 +6,4 @@ const fork = require('child_process').fork; const n = fork(common.fixturesDir + '/empty.js'); const rv = n.send({ hello: 'world' }); -assert.strictEqual(typeof rv, 'boolean'); +assert.strictEqual(rv, true); From 0f09b7c4a091fc81c3ad16aad655df8dc9243db4 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 28 Oct 2015 09:34:55 -0700 Subject: [PATCH 3/5] fixup: 128K -> 1 --- lib/internal/child_process.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js index 9858245ccdd929..b53bef77458677 100644 --- a/lib/internal/child_process.js +++ b/lib/internal/child_process.js @@ -577,7 +577,7 @@ function setupChannel(target, channel) { handle: null, message: message, }); - return this._handleQueue.length < (65536 * 2); + return this._handleQueue.length < 1; } var req = new WriteWrap(); From 08ccde94a9b19a6189ed25e19ff2f9b034f78538 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 28 Oct 2015 09:54:35 -0700 Subject: [PATCH 4/5] fixup: doh! --- lib/internal/child_process.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js index b53bef77458677..da213955a59345 100644 --- a/lib/internal/child_process.js +++ b/lib/internal/child_process.js @@ -577,7 +577,7 @@ function setupChannel(target, channel) { handle: null, message: message, }); - return this._handleQueue.length < 1; + return this._handleQueue.length === 1; } var req = new WriteWrap(); From eba271a368d4aec6b70dbd2496356410000d5a6d Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 28 Oct 2015 10:45:46 -0700 Subject: [PATCH 5/5] fixup: rename test file to conform to other files --- ...urns-boolean.js => test-child-process-send-returns-boolean.js} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/parallel/{test-fork-send-returns-boolean.js => test-child-process-send-returns-boolean.js} (100%) diff --git a/test/parallel/test-fork-send-returns-boolean.js b/test/parallel/test-child-process-send-returns-boolean.js similarity index 100% rename from test/parallel/test-fork-send-returns-boolean.js rename to test/parallel/test-child-process-send-returns-boolean.js