Skip to content

Commit

Permalink
lib,src: standardize owner_symbol for handles
Browse files Browse the repository at this point in the history
Instead of somtimes using an `owner` string to link from a
native handle object to the corresponding JS object, standardize
on a single symbol that fulfills this role.

PR-URL: #22002
Backport-PR-URL: #22507
Reviewed-By: Michaël Zasso <targos@protonmail.com>
  • Loading branch information
addaleax authored and targos committed Sep 6, 2018
1 parent 727187d commit cae21ea
Show file tree
Hide file tree
Showing 14 changed files with 91 additions and 48 deletions.
23 changes: 12 additions & 11 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const debug = util.debuglog('tls');
const tls_wrap = process.binding('tls_wrap');
const { TCP, constants: TCPConstants } = process.binding('tcp_wrap');
const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap');
const { owner_symbol } = require('internal/async_hooks').symbols;
const {
SecureContext: NativeSecureContext
} = process.binding('crypto');
Expand Down Expand Up @@ -77,7 +78,7 @@ function onhandshakestart(now) {
else
this.handshakes++;

const { owner } = this;
const owner = this[owner_symbol];
if (this.handshakes > tls.CLIENT_RENEG_LIMIT) {
owner._emitTLSError(new ERR_TLS_SESSION_ATTACK());
return;
Expand All @@ -90,7 +91,7 @@ function onhandshakestart(now) {
function onhandshakedone() {
debug('onhandshakedone');

const owner = this.owner;
const owner = this[owner_symbol];

// `newSession` callback wasn't called yet
if (owner._newSessionPending) {
Expand All @@ -103,7 +104,7 @@ function onhandshakedone() {


function loadSession(hello) {
const owner = this.owner;
const owner = this[owner_symbol];

var once = false;
function onSession(err, session) {
Expand Down Expand Up @@ -131,7 +132,7 @@ function loadSession(hello) {


function loadSNI(info) {
const owner = this.owner;
const owner = this[owner_symbol];
const servername = info.servername;
if (!servername || !owner._SNICallback)
return requestOCSP(owner, info);
Expand Down Expand Up @@ -213,7 +214,7 @@ function requestOCSPDone(socket) {


function onnewsession(key, session) {
const owner = this.owner;
const owner = this[owner_symbol];

if (!owner.server)
return;
Expand Down Expand Up @@ -242,11 +243,11 @@ function onnewsession(key, session) {


function onocspresponse(resp) {
this.owner.emit('OCSPResponse', resp);
this[owner_symbol].emit('OCSPResponse', resp);
}

function onerror(err) {
const owner = this.owner;
const owner = this[owner_symbol];

if (owner._writableState.errorEmitted)
return;
Expand Down Expand Up @@ -365,9 +366,9 @@ for (var n = 0; n < proxiedMethods.length; n++) {

tls_wrap.TLSWrap.prototype.close = function close(cb) {
let ssl;
if (this.owner) {
ssl = this.owner.ssl;
this.owner.ssl = null;
if (this[owner_symbol]) {
ssl = this[owner_symbol].ssl;
this[owner_symbol].ssl = null;
}

// Invoke `destroySSL` on close to clean up possibly pending write requests
Expand Down Expand Up @@ -406,7 +407,7 @@ TLSSocket.prototype._wrapHandle = function(wrap) {
handle = options.pipe ?
new Pipe(PipeConstants.SOCKET) :
new TCP(TCPConstants.SOCKET);
handle.owner = this;
handle[owner_symbol] = this;
}

// Wrap socket's handle
Expand Down
16 changes: 12 additions & 4 deletions lib/dgram.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ const { isUint8Array } = require('internal/util/types');
const EventEmitter = require('events');
const {
defaultTriggerAsyncIdScope,
symbols: { async_id_symbol }
symbols: { async_id_symbol, owner_symbol }
} = require('internal/async_hooks');
const { UV_UDP_REUSEADDR } = process.binding('constants').os;

Expand Down Expand Up @@ -82,7 +82,7 @@ function Socket(type, listener) {
}

var handle = newHandle(type, lookup);
handle.owner = this;
handle[owner_symbol] = this;

this[async_id_symbol] = handle.getAsyncId();
this.type = type;
Expand Down Expand Up @@ -136,7 +136,7 @@ function replaceHandle(self, newHandle) {
newHandle.lookup = oldHandle.lookup;
newHandle.bind = oldHandle.bind;
newHandle.send = oldHandle.send;
newHandle.owner = self;
newHandle[owner_symbol] = self;

// Replace the existing handle by the handle we got from master.
oldHandle.close();
Expand Down Expand Up @@ -620,7 +620,7 @@ function stopReceiving(socket) {


function onMessage(nread, handle, buf, rinfo) {
var self = handle.owner;
var self = handle[owner_symbol];
if (nread < 0) {
return self.emit('error', errnoException(nread, 'recvmsg'));
}
Expand Down Expand Up @@ -730,6 +730,14 @@ Socket.prototype._stopReceiving = function() {
};


// Legacy alias on the C++ wrapper object. This is not public API, so we may
// want to runtime-deprecate it at some point. There's no hurry, though.
Object.defineProperty(UDP.prototype, 'owner', {
get() { return this[owner_symbol]; },
set(v) { return this[owner_symbol] = v; }
});


module.exports = {
_createSocketHandle,
createSocket,
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/async_hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const async_wrap = process.binding('async_wrap');
* It has a fixed size, so if that is exceeded, calls to the native
* side are used instead in pushAsyncIds() and popAsyncIds().
*/
const { async_hook_fields, async_id_fields } = async_wrap;
const { async_hook_fields, async_id_fields, owner_symbol } = async_wrap;
// Store the pair executionAsyncId and triggerAsyncId in a std::stack on
// Environment::AsyncHooks::async_ids_stack_ tracks the resource responsible for
// the current execution stack. This is unwound as each resource exits. In the
Expand Down Expand Up @@ -434,7 +434,7 @@ module.exports = {
symbols: {
async_id_symbol, trigger_async_id_symbol,
init_symbol, before_symbol, after_symbol, destroy_symbol,
promise_resolve_symbol
promise_resolve_symbol, owner_symbol
},
constants: {
kInit, kBefore, kAfter, kDestroy, kTotals, kPromiseResolve
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const { TTY } = process.binding('tty_wrap');
const { TCP } = process.binding('tcp_wrap');
const { UDP } = process.binding('udp_wrap');
const SocketList = require('internal/socket_list');
const { owner_symbol } = require('internal/async_hooks').symbols;
const { convertToValidSignal } = require('internal/util');
const { isUint8Array } = require('internal/util/types');
const spawn_sync = process.binding('spawn_sync');
Expand Down Expand Up @@ -210,7 +211,7 @@ function ChildProcess() {
this.spawnfile = null;

this._handle = new Process();
this._handle.owner = this;
this._handle[owner_symbol] = this;

this._handle.onexit = (exitCode, signalCode) => {
if (signalCode) {
Expand Down
5 changes: 3 additions & 2 deletions lib/internal/cluster/child.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const assert = require('assert');
const util = require('util');
const path = require('path');
const EventEmitter = require('events');
const { owner_symbol } = require('internal/async_hooks').symbols;
const Worker = require('internal/cluster/worker');
const { internal, sendHelper } = require('internal/cluster/utils');
const cluster = new EventEmitter();
Expand Down Expand Up @@ -207,8 +208,8 @@ function _disconnect(masterInitiated) {
delete handles[key];
waitingCount++;

if (handle.owner)
handle.owner.close(checkWaitingCount);
if (handle[owner_symbol])
handle[owner_symbol].close(checkWaitingCount);
else
handle.close(checkWaitingCount);
}
Expand Down
20 changes: 15 additions & 5 deletions lib/internal/fs/watchers.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ const {
getStatsFromBinding,
validatePath
} = require('internal/fs/utils');
const { defaultTriggerAsyncIdScope } = require('internal/async_hooks');
const {
defaultTriggerAsyncIdScope,
symbols: { owner_symbol }
} = require('internal/async_hooks');
const { toNamespacedPath } = require('path');
const { validateUint32 } = require('internal/validators');
const { getPathFromURL } = require('internal/url');
Expand All @@ -20,7 +23,6 @@ const assert = require('assert');

const kOldStatus = Symbol('kOldStatus');
const kUseBigint = Symbol('kUseBigint');
const kOwner = Symbol('kOwner');

function emitStop(self) {
self.emit('stop');
Expand All @@ -36,7 +38,7 @@ function StatWatcher(bigint) {
util.inherits(StatWatcher, EventEmitter);

function onchange(newStatus, stats) {
const self = this[kOwner];
const self = this[owner_symbol];
if (self[kOldStatus] === -1 &&
newStatus === -1 &&
stats[2/* new nlink */] === stats[16/* old nlink */]) {
Expand All @@ -59,7 +61,7 @@ StatWatcher.prototype.start = function(filename, persistent, interval) {
return;

this._handle = new _StatWatcher(this[kUseBigint]);
this._handle[kOwner] = this;
this._handle[owner_symbol] = this;
this._handle.onchange = onchange;
if (!persistent)
this._handle.unref();
Expand Down Expand Up @@ -104,7 +106,7 @@ function FSWatcher() {
EventEmitter.call(this);

this._handle = new FSEvent();
this._handle.owner = this;
this._handle[owner_symbol] = this;

this._handle.onchange = (status, eventType, filename) => {
// TODO(joyeecheung): we may check self._handle.initialized here
Expand All @@ -131,6 +133,7 @@ function FSWatcher() {
}
util.inherits(FSWatcher, EventEmitter);


// FIXME(joyeecheung): this method is not documented.
// At the moment if filename is undefined, we
// 1. Throw an Error if it's the first time .start() is called
Expand Down Expand Up @@ -187,6 +190,13 @@ function emitCloseNT(self) {
self.emit('close');
}

// Legacy alias on the C++ wrapper object. This is not public API, so we may
// want to runtime-deprecate it at some point. There's no hurry, though.
Object.defineProperty(FSEvent.prototype, 'owner', {
get() { return this[owner_symbol]; },
set(v) { return this[owner_symbol] = v; }
});

module.exports = {
FSWatcher,
StatWatcher
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const {
defaultTriggerAsyncIdScope,
symbols: {
async_id_symbol,
owner_symbol,
},
} = require('internal/async_hooks');
const { internalBinding } = require('internal/bootstrap/loaders');
Expand Down Expand Up @@ -144,7 +145,7 @@ const kInfoHeaders = Symbol('sent-info-headers');
const kMaybeDestroy = Symbol('maybe-destroy');
const kLocalSettings = Symbol('local-settings');
const kOptions = Symbol('options');
const kOwner = Symbol('owner');
const kOwner = owner_symbol;
const kProceed = Symbol('proceed');
const kProtocol = Symbol('protocol');
const kProxySocket = Symbol('proxy-socket');
Expand Down
13 changes: 7 additions & 6 deletions lib/internal/wrap_js_stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,17 @@ const { Socket } = require('net');
const { JSStream } = process.binding('js_stream');
const uv = process.binding('uv');
const debug = util.debuglog('stream_wrap');
const { owner_symbol } = require('internal/async_hooks').symbols;
const { ERR_STREAM_WRAP } = require('internal/errors').codes;

const kCurrentWriteRequest = Symbol('kCurrentWriteRequest');
const kCurrentShutdownRequest = Symbol('kCurrentShutdownRequest');

function isClosing() { return this.owner.isClosing(); }
function onreadstart() { return this.owner.readStart(); }
function onreadstop() { return this.owner.readStop(); }
function onshutdown(req) { return this.owner.doShutdown(req); }
function onwrite(req, bufs) { return this.owner.doWrite(req, bufs); }
function isClosing() { return this[owner_symbol].isClosing(); }
function onreadstart() { return this[owner_symbol].readStart(); }
function onreadstop() { return this[owner_symbol].readStop(); }
function onshutdown(req) { return this[owner_symbol].doShutdown(req); }
function onwrite(req, bufs) { return this[owner_symbol].doWrite(req, bufs); }

/* This class serves as a wrapper for when the C++ side of Node wants access
* to a standard JS stream. For example, TLS or HTTP do not operate on network
Expand All @@ -37,7 +38,7 @@ class JSStreamWrap extends Socket {
this.doClose(cb);
};
// Inside of the following functions, `this` refers to the handle
// and `this.owner` refers to this JSStreamWrap instance.
// and `this[owner_symbol]` refers to this JSStreamWrap instance.
handle.isClosing = isClosing;
handle.onreadstart = onreadstart;
handle.onreadstop = onreadstop;
Expand Down
24 changes: 16 additions & 8 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ const {
const {
newAsyncId,
defaultTriggerAsyncIdScope,
symbols: { async_id_symbol }
symbols: { async_id_symbol, owner_symbol }
} = require('internal/async_hooks');
const {
createWriteWrap,
Expand Down Expand Up @@ -207,7 +207,7 @@ function initSocketHandle(self) {

// Handle creation may be deferred to bind() or connect() time.
if (self._handle) {
self._handle.owner = self;
self._handle[owner_symbol] = self;
self._handle.onread = onread;
self[async_id_symbol] = getNewAsyncId(self._handle);
}
Expand Down Expand Up @@ -371,7 +371,7 @@ Socket.prototype._final = function(cb) {


function afterShutdown(status, handle) {
var self = handle.owner;
var self = handle[owner_symbol];

debug('afterShutdown destroyed=%j', self.destroyed,
self._readableState);
Expand Down Expand Up @@ -620,7 +620,7 @@ Socket.prototype._destroy = function(exception, cb) {
// buffer, or when there's an error reading.
function onread(nread, buffer) {
var handle = this;
var self = handle.owner;
var self = handle[owner_symbol];
assert(handle === self._handle, 'handle != self._handle');

self._unrefTimer();
Expand Down Expand Up @@ -819,7 +819,7 @@ protoGetter('bytesWritten', function bytesWritten() {


function afterWrite(status, handle, err) {
var self = handle.owner;
var self = handle[owner_symbol];
if (self !== process.stderr && self !== process.stdout)
debug('afterWrite', status);

Expand Down Expand Up @@ -1121,7 +1121,7 @@ Socket.prototype.unref = function() {


function afterConnect(status, handle, req, readable, writable) {
var self = handle.owner;
var self = handle[owner_symbol];

// callback may come after call to destroy
if (self.destroyed) {
Expand Down Expand Up @@ -1323,7 +1323,7 @@ function setupListenHandle(address, port, addressType, backlog, fd) {

this[async_id_symbol] = getNewAsyncId(this._handle);
this._handle.onconnection = onconnection;
this._handle.owner = this;
this._handle[owner_symbol] = this;

// Use a backlog of 512 entries. We pass 511 to the listen() call because
// the kernel does: backlogsize = roundup_pow_of_two(backlogsize + 1);
Expand Down Expand Up @@ -1536,7 +1536,7 @@ Server.prototype.address = function() {

function onconnection(err, clientHandle) {
var handle = this;
var self = handle.owner;
var self = handle[owner_symbol];

debug('onconnection');

Expand Down Expand Up @@ -1667,6 +1667,14 @@ function emitCloseNT(self) {
}


// Legacy alias on the C++ wrapper object. This is not public API, so we may
// want to runtime-deprecate it at some point. There's no hurry, though.
Object.defineProperty(TCP.prototype, 'owner', {
get() { return this[owner_symbol]; },
set(v) { return this[owner_symbol] = v; }
});


Server.prototype.listenFD = internalUtil.deprecate(function(fd, type) {
return this.listen({ fd: fd });
}, 'Server.listenFD is deprecated. Use Server.listen({fd: <number>}) instead.',
Expand Down

0 comments on commit cae21ea

Please sign in to comment.