Skip to content

Commit

Permalink
async_hooks: merge resource_symbol with owner_symbol
Browse files Browse the repository at this point in the history
Signed-off-by: Darshan Sen <darshan.sen@postman.com>

PR-URL: #38468
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
  • Loading branch information
RaisinTen authored and danielleadams committed Aug 16, 2021
1 parent 54dd3df commit a80c989
Show file tree
Hide file tree
Showing 9 changed files with 71 additions and 21 deletions.
2 changes: 1 addition & 1 deletion lib/_http_agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ function asyncResetHandle(socket) {
const handle = socket._handle;
if (handle && typeof handle.asyncReset === 'function') {
// Assign the handle a new asyncId and run any destroy()/init() hooks.
handle.asyncReset(new ReusedHandle(handle.getProviderType(), handle));
handle.asyncReset(new ReusedHandle(handle.getProviderType(), socket));
socket[async_id_symbol] = handle.getAsyncId();
}
}
Expand Down
12 changes: 7 additions & 5 deletions lib/internal/async_hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ const active_hooks = {

const { registerDestroyHook } = async_wrap;
const { enqueueMicrotask } = internalBinding('task_queue');
const { resource_symbol, owner_symbol } = internalBinding('symbols');
const { owner_symbol } = internalBinding('symbols');

// Each constant tracks how many callbacks there are for any given step of
// async execution. These are tracked so if the user didn't include callbacks
Expand Down Expand Up @@ -178,11 +178,13 @@ function fatalError(e) {

function lookupPublicResource(resource) {
if (typeof resource !== 'object' || resource === null) return resource;
// TODO(addaleax): Merge this with owner_symbol and use it across all
// AsyncWrap instances.
const publicResource = resource[resource_symbol];
if (publicResource !== undefined)

const publicResource = resource[owner_symbol];

if (publicResource != null) {
return publicResource;
}

return resource;
}

Expand Down
50 changes: 45 additions & 5 deletions lib/internal/js_stream_socket.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,55 @@ const kCurrentWriteRequest = Symbol('kCurrentWriteRequest');
const kCurrentShutdownRequest = Symbol('kCurrentShutdownRequest');
const kPendingShutdownRequest = Symbol('kPendingShutdownRequest');

function isClosing() { return this[owner_symbol].isClosing(); }
function isClosing() {
let socket = this[owner_symbol];

function onreadstart() { return this[owner_symbol].readStart(); }
if (socket.constructor.name === 'ReusedHandle') {
socket = socket.handle;
}

return socket.isClosing();
}

function onreadstart() {
let socket = this[owner_symbol];

if (socket.constructor.name === 'ReusedHandle') {
socket = socket.handle;
}

return socket.readStart();
}

function onreadstop() {
let socket = this[owner_symbol];

if (socket.constructor.name === 'ReusedHandle') {
socket = socket.handle;
}

return socket.readStop();
}

function onshutdown(req) {
let socket = this[owner_symbol];

if (socket.constructor.name === 'ReusedHandle') {
socket = socket.handle;
}

function onreadstop() { return this[owner_symbol].readStop(); }
return socket.doShutdown(req);
}

function onshutdown(req) { return this[owner_symbol].doShutdown(req); }
function onwrite(req, bufs) {
let socket = this[owner_symbol];

function onwrite(req, bufs) { return this[owner_symbol].doWrite(req, bufs); }
if (socket.constructor.name === 'ReusedHandle') {
socket = socket.handle;
}

return socket.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 Down
13 changes: 11 additions & 2 deletions lib/internal/stream_base_commons.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,11 @@ function handleWriteReq(req, data, encoding) {
function onWriteComplete(status) {
debug('onWriteComplete', status, this.error);

const stream = this.handle[owner_symbol];
let stream = this.handle[owner_symbol];

if (stream.constructor.name === 'ReusedHandle') {
stream = stream.handle;
}

if (stream.destroyed) {
if (typeof this.callback === 'function')
Expand Down Expand Up @@ -168,7 +172,12 @@ function onStreamRead(arrayBuffer) {
const nread = streamBaseState[kReadBytesOrError];

const handle = this;
const stream = this[owner_symbol];

let stream = this[owner_symbol];

if (stream.constructor.name === 'ReusedHandle') {
stream = stream.handle;
}

stream[kUpdateTimer]();

Expand Down
6 changes: 5 additions & 1 deletion lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -1102,7 +1102,11 @@ Socket.prototype.unref = function() {


function afterConnect(status, handle, req, readable, writable) {
const self = handle[owner_symbol];
let self = handle[owner_symbol];

if (self.constructor.name === 'ReusedHandle') {
self = self.handle;
}

// Callback may come after call to destroy
if (self.destroyed) {
Expand Down
4 changes: 2 additions & 2 deletions src/async_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ void AsyncWrap::EmitDestroy(bool from_gc) {

if (!persistent().IsEmpty() && !from_gc) {
HandleScope handle_scope(env()->isolate());
USE(object()->Set(env()->context(), env()->resource_symbol(), object()));
USE(object()->Set(env()->context(), env()->owner_symbol(), object()));
}
}

Expand Down Expand Up @@ -586,7 +586,7 @@ void AsyncWrap::AsyncReset(Local<Object> resource, double execution_async_id,
Local<Object> obj = object();
CHECK(!obj.IsEmpty());
if (resource != obj) {
USE(obj->Set(env()->context(), env()->resource_symbol(), resource));
USE(obj->Set(env()->context(), env()->owner_symbol(), resource));
}
}

Expand Down
1 change: 0 additions & 1 deletion src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ constexpr size_t kFsStatsBufferLength =
V(oninit_symbol, "oninit") \
V(owner_symbol, "owner_symbol") \
V(onpskexchange_symbol, "onpskexchange") \
V(resource_symbol, "resource_symbol") \
V(trigger_async_id_symbol, "trigger_async_id_symbol") \

// Strings are per-isolate primitives but Environment proxies them
Expand Down
2 changes: 0 additions & 2 deletions test/async-hooks/test-http-agent-handle-reuse-parallel.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,4 @@ function onExit() {
// Verify reuse handle has been wrapped
assert.strictEqual(first.type, second.type);
assert.ok(first.handle !== second.handle, 'Resource reused');
assert.ok(first.handle === second.handle.handle,
'Resource not wrapped correctly');
}
2 changes: 0 additions & 2 deletions test/async-hooks/test-http-agent-handle-reuse-serial.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,4 @@ function onExit() {
// Verify reuse handle has been wrapped
assert.strictEqual(first.type, second.type);
assert.ok(first.handle !== second.handle, 'Resource reused');
assert.ok(first.handle === second.handle.handle,
'Resource not wrapped correctly');
}

0 comments on commit a80c989

Please sign in to comment.