From a80c989306c152e76fc03b59634303a11183e0c5 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Thu, 29 Apr 2021 20:47:09 +0530 Subject: [PATCH] async_hooks: merge resource_symbol with owner_symbol Signed-off-by: Darshan Sen PR-URL: https://github.com/nodejs/node/pull/38468 Reviewed-By: James M Snell Reviewed-By: Anna Henningsen Reviewed-By: Rich Trott 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, 71 insertions(+), 21 deletions(-) diff --git a/lib/_http_agent.js b/lib/_http_agent.js index 1e372f417ee5b2..a42c0e83992b1f 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(), handle)); + handle.asyncReset(new ReusedHandle(handle.getProviderType(), socket)); socket[async_id_symbol] = handle.getAsyncId(); } } diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index c5895bce1c036c..9aacf4b3bab7c4 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 { 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 @@ -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; } diff --git a/lib/internal/js_stream_socket.js b/lib/internal/js_stream_socket.js index faad988e820ffa..fd1294ec9764f9 100644 --- a/lib/internal/js_stream_socket.js +++ b/lib/internal/js_stream_socket.js @@ -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 diff --git a/lib/internal/stream_base_commons.js b/lib/internal/stream_base_commons.js index 5254fc1553dd77..13b5f541cb88ef 100644 --- a/lib/internal/stream_base_commons.js +++ b/lib/internal/stream_base_commons.js @@ -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') @@ -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](); diff --git a/lib/net.js b/lib/net.js index 20601007fab695..ded48732c2e11e 100644 --- a/lib/net.js +++ b/lib/net.js @@ -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) { diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 3fb2f8c309825c..59b842b232517b 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()->resource_symbol(), object())); + USE(object()->Set(env()->context(), env()->owner_symbol(), object())); } } @@ -586,7 +586,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()->resource_symbol(), resource)); + USE(obj->Set(env()->context(), env()->owner_symbol(), resource)); } } diff --git a/src/env.h b/src/env.h index e1b89261fcb1e9..24b4a48b5f9657 100644 --- a/src/env.h +++ b/src/env.h @@ -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 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 cd73b3ed2cb61c..a7d76a694b24d3 100644 --- a/test/async-hooks/test-http-agent-handle-reuse-parallel.js +++ b/test/async-hooks/test-http-agent-handle-reuse-parallel.js @@ -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'); } 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 bbc7183d6e72ca..2ee118bb240a36 100644 --- a/test/async-hooks/test-http-agent-handle-reuse-serial.js +++ b/test/async-hooks/test-http-agent-handle-reuse-serial.js @@ -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'); }