Skip to content

Commit

Permalink
Revert "async_hooks: merge resource_symbol with owner_symbol"
Browse files Browse the repository at this point in the history
This reverts commit 7ca2f13.

PR-URL: #40741
Fixes: #40693
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
  • Loading branch information
RaisinTen authored and danielleadams committed Jan 30, 2022
1 parent e2af370 commit 460e5d1
Show file tree
Hide file tree
Showing 9 changed files with 21 additions and 71 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(), socket));
handle.asyncReset(new ReusedHandle(handle.getProviderType(), handle));
socket[async_id_symbol] = handle.getAsyncId();
}
}
Expand Down
12 changes: 5 additions & 7 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 { owner_symbol } = internalBinding('symbols');
const { resource_symbol, 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 @@ -176,13 +176,11 @@ function fatalError(e) {

function lookupPublicResource(resource) {
if (typeof resource !== 'object' || resource === null) return resource;

const publicResource = resource[owner_symbol];

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

return resource;
}

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

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

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 onreadstart() { return this[owner_symbol].readStart(); }

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

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

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

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

let stream = this.handle[owner_symbol];

if (stream.constructor.name === 'ReusedHandle') {
stream = stream.handle;
}
const stream = this.handle[owner_symbol];

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

const handle = this;

let stream = this[owner_symbol];

if (stream.constructor.name === 'ReusedHandle') {
stream = stream.handle;
}
const stream = this[owner_symbol];

stream[kUpdateTimer]();

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


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

if (self.constructor.name === 'ReusedHandle') {
self = self.handle;
}
const self = handle[owner_symbol];

// 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()->owner_symbol(), object()));
USE(object()->Set(env()->context(), env()->resource_symbol(), object()));
}
}

Expand Down Expand Up @@ -589,7 +589,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()->owner_symbol(), resource));
USE(obj->Set(env()->context(), env()->resource_symbol(), resource));
}
}

Expand Down
1 change: 1 addition & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ 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: 2 additions & 0 deletions test/async-hooks/test-http-agent-handle-reuse-parallel.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,6 @@ 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: 2 additions & 0 deletions test/async-hooks/test-http-agent-handle-reuse-serial.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,4 +105,6 @@ 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 460e5d1

Please sign in to comment.