Skip to content

Commit

Permalink
net,child_process: improve naming in internal code
Browse files Browse the repository at this point in the history
All of this code is internal-only, and the changed variables/methods
are not generally useful to userland code.

When backporting this to release branches, it might be appropriate to
add non-enumerable aliases to be 100 % sure.

PR-URL: #14449
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
addaleax committed Jul 26, 2017
1 parent e59987c commit 75a19fb
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 25 deletions.
11 changes: 11 additions & 0 deletions doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,17 @@ Type: Runtime

*Note*: change was made while `async_hooks` was an experimental API.

<a id="DEP00XX"></a>
### DEP00XX: Several internal properties of net.Server

Type: Runtime

Accessing several internal, undocumented properties of `net.Server` instances
with inappropriate names has been deprecated.

*Note*: As the original API was undocumented and not generally useful for
non-internal code, no replacement API is provided.

[`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size
[`Buffer.from(array)`]: buffer.html#buffer_class_method_buffer_from_array
[`Buffer.from(buffer)`]: buffer.html#buffer_class_method_buffer_from_buffer
Expand Down
13 changes: 6 additions & 7 deletions lib/internal/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,16 @@ const handleConversion = {

// if the socket was created by net.Server
if (socket.server) {
// the slave should keep track of the socket
// the worker should keep track of the socket
message.key = socket.server._connectionKey;

var firstTime = !this.channel.sockets.send[message.key];
var socketList = getSocketList('send', this, message.key);

// the server should no longer expose a .connection property
// and when asked to close it should query the socket status from
// the slaves
if (firstTime) socket.server._setupSlave(socketList);
// the workers
if (firstTime) socket.server._setupWorker(socketList);

// Act like socket is detached
if (!options.keepOpen)
Expand Down Expand Up @@ -911,12 +911,12 @@ function _validateStdio(stdio, sync) {
}


function getSocketList(type, slave, key) {
var sockets = slave.channel.sockets[type];
function getSocketList(type, worker, key) {
var sockets = worker.channel.sockets[type];
var socketList = sockets[key];
if (!socketList) {
var Construct = type === 'send' ? SocketListSend : SocketListReceive;
socketList = sockets[key] = new Construct(slave, key);
socketList = sockets[key] = new Construct(worker, key);
}
return socketList;
}
Expand Down Expand Up @@ -958,6 +958,5 @@ module.exports = {
ChildProcess,
setupChannel,
_validateStdio,
getSocketList,
spawnSync
};
64 changes: 46 additions & 18 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -1187,7 +1187,7 @@ function Server(options, connectionListener) {
Object.defineProperty(this, 'connections', {
get: internalUtil.deprecate(() => {

if (this._usingSlaves) {
if (this._usingWorkers) {
return null;
}
return this._connections;
Expand All @@ -1201,8 +1201,8 @@ function Server(options, connectionListener) {

this[async_id_symbol] = -1;
this._handle = null;
this._usingSlaves = false;
this._slaves = [];
this._usingWorkers = false;
this._workers = [];
this._unref = false;

this.allowHalfOpen = options.allowHalfOpen || false;
Expand Down Expand Up @@ -1555,13 +1555,13 @@ Server.prototype.getConnections = function(cb) {
nextTick(asyncId, cb, err, connections);
}

if (!this._usingSlaves) {
if (!this._usingWorkers) {
end(null, this._connections);
return this;
}

// Poll slaves
var left = this._slaves.length;
// Poll workers
var left = this._workers.length;
var total = this._connections;

function oncount(err, count) {
Expand All @@ -1574,8 +1574,8 @@ Server.prototype.getConnections = function(cb) {
if (--left === 0) return end(null, total);
}

for (var n = 0; n < this._slaves.length; n++) {
this._slaves[n].getConnections(oncount);
for (var n = 0; n < this._workers.length; n++) {
this._workers[n].getConnections(oncount);
}

return this;
Expand All @@ -1598,22 +1598,22 @@ Server.prototype.close = function(cb) {
this._handle = null;
}

if (this._usingSlaves) {
var left = this._slaves.length;
const onSlaveClose = () => {
if (this._usingWorkers) {
var left = this._workers.length;
const onWorkerClose = () => {
if (--left !== 0) return;

this._connections = 0;
this._emitCloseIfDrained();
};

// Increment connections to be sure that, even if all sockets will be closed
// during polling of slaves, `close` event will be emitted only once.
// during polling of workers, `close` event will be emitted only once.
this._connections++;

// Poll slaves
for (var n = 0; n < this._slaves.length; n++)
this._slaves[n].close(onSlaveClose);
// Poll workers
for (var n = 0; n < this._workers.length; n++)
this._workers[n].close(onWorkerClose);
} else {
this._emitCloseIfDrained();
}
Expand Down Expand Up @@ -1646,9 +1646,9 @@ Server.prototype.listenFD = internalUtil.deprecate(function(fd, type) {
}, 'Server.listenFD is deprecated. Use Server.listen({fd: <number>}) instead.',
'DEP0021');

Server.prototype._setupSlave = function(socketList) {
this._usingSlaves = true;
this._slaves.push(socketList);
Server.prototype._setupWorker = function(socketList) {
this._usingWorkers = true;
this._workers.push(socketList);
};

Server.prototype.ref = function() {
Expand Down Expand Up @@ -1693,6 +1693,34 @@ if (process.platform === 'win32') {
_setSimultaneousAccepts = function(handle) {};
}

// TODO(addaleax): Remove these after the Node 9.x branch cut.
Object.defineProperty(Server.prototype, '_usingSlaves', {
get: internalUtil.deprecate(function() {
return this._usingWorkers;
}, 'Accessing internal properties of net.Server is deprecated.', 'DEP00XX'),
set: internalUtil.deprecate((val) => {
this._usingWorkers = val;
}, 'Accessing internal properties of net.Server is deprecated.', 'DEP00XX'),
configurable: true, enumerable: false
});

Object.defineProperty(Server.prototype, '_slaves', {
get: internalUtil.deprecate(function() {
return this._workers;
}, 'Accessing internal properties of net.Server is deprecated.', 'DEP00XX'),
set: internalUtil.deprecate((val) => {
this._workers = val;
}, 'Accessing internal properties of net.Server is deprecated.', 'DEP00XX'),
configurable: true, enumerable: false
});

Object.defineProperty(Server.prototype, '_setupSlave', {
value: internalUtil.deprecate(function(socketList) {
return this._setupWorker(socketList);
}, 'Accessing internal properties of net.Server is deprecated.', 'DEP00XX'),
configurable: true, enumerable: false
});

module.exports = {
_createServerHandle: createServerHandle,
_normalizeArgs: normalizeArgs,
Expand Down

0 comments on commit 75a19fb

Please sign in to comment.