From 6af3e34b6bfa71f17dba1216a3d22000e9ae1ba2 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Sat, 13 Nov 2021 12:51:42 +0530 Subject: [PATCH] Revert "async_hooks: merge resource_symbol with owner_symbol" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 7ca2f1303996e0c79c354e979a1527da444ca886. PR-URL: https://github.com/nodejs/node/pull/40741 Fixes: https://github.com/nodejs/node/issues/40693 Reviewed-By: Stephen Belanger Reviewed-By: Gerhard Stöbich Reviewed-By: Benjamin Gruenbaum --- lib/_http_agent.js | 2 +- lib/internal/async_hooks.js | 12 ++--- lib/internal/js_stream_socket.js | 50 ++----------------- lib/internal/stream_base_commons.js | 13 +---- lib/net.js | 6 +-- src/async_wrap.cc | 4 +- src/env.h | 1 + .../test-http-agent-handle-reuse-parallel.js | 2 + .../test-http-agent-handle-reuse-serial.js | 2 + 9 files changed, 21 insertions(+), 71 deletions(-) diff --git a/lib/_http_agent.js b/lib/_http_agent.js index 87450993a64716..9c15875762dd47 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -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(); } } diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index 8608d6d1a7bed9..17cdabbd281ad1 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -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 @@ -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; } diff --git a/lib/internal/js_stream_socket.js b/lib/internal/js_stream_socket.js index fd1294ec9764f9..faad988e820ffa 100644 --- a/lib/internal/js_stream_socket.js +++ b/lib/internal/js_stream_socket.js @@ -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 diff --git a/lib/internal/stream_base_commons.js b/lib/internal/stream_base_commons.js index 13b5f541cb88ef..5254fc1553dd77 100644 --- a/lib/internal/stream_base_commons.js +++ b/lib/internal/stream_base_commons.js @@ -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') @@ -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](); diff --git a/lib/net.js b/lib/net.js index 41ff284e1ec027..3bbe96f1e04af0 100644 --- a/lib/net.js +++ b/lib/net.js @@ -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) { diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 8ed8ce11d88b22..d5a62951a7d5c7 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -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())); } } @@ -589,7 +589,7 @@ void AsyncWrap::AsyncReset(Local resource, double execution_async_id, Local 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)); } } diff --git a/src/env.h b/src/env.h index c0712d4881a084..5cf789f9bb7ee9 100644 --- a/src/env.h +++ b/src/env.h @@ -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 diff --git a/test/async-hooks/test-http-agent-handle-reuse-parallel.js b/test/async-hooks/test-http-agent-handle-reuse-parallel.js index a7d76a694b24d3..cd73b3ed2cb61c 100644 --- a/test/async-hooks/test-http-agent-handle-reuse-parallel.js +++ b/test/async-hooks/test-http-agent-handle-reuse-parallel.js @@ -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'); } diff --git a/test/async-hooks/test-http-agent-handle-reuse-serial.js b/test/async-hooks/test-http-agent-handle-reuse-serial.js index 2ee118bb240a36..bbc7183d6e72ca 100644 --- a/test/async-hooks/test-http-agent-handle-reuse-serial.js +++ b/test/async-hooks/test-http-agent-handle-reuse-serial.js @@ -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'); }