Skip to content

Commit 7ca2f13

Browse files
RaisinTenjasnell
authored andcommitted
async_hooks: merge resource_symbol with owner_symbol
Signed-off-by: Darshan Sen <[email protected]> PR-URL: #38468 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
1 parent fce5303 commit 7ca2f13

9 files changed

+71
-21
lines changed

lib/_http_agent.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -525,7 +525,7 @@ function asyncResetHandle(socket) {
525525
const handle = socket._handle;
526526
if (handle && typeof handle.asyncReset === 'function') {
527527
// Assign the handle a new asyncId and run any destroy()/init() hooks.
528-
handle.asyncReset(new ReusedHandle(handle.getProviderType(), handle));
528+
handle.asyncReset(new ReusedHandle(handle.getProviderType(), socket));
529529
socket[async_id_symbol] = handle.getAsyncId();
530530
}
531531
}

lib/internal/async_hooks.js

+7-5
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ const active_hooks = {
8181

8282
const { registerDestroyHook } = async_wrap;
8383
const { enqueueMicrotask } = internalBinding('task_queue');
84-
const { resource_symbol, owner_symbol } = internalBinding('symbols');
84+
const { owner_symbol } = internalBinding('symbols');
8585

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

179179
function lookupPublicResource(resource) {
180180
if (typeof resource !== 'object' || resource === null) return resource;
181-
// TODO(addaleax): Merge this with owner_symbol and use it across all
182-
// AsyncWrap instances.
183-
const publicResource = resource[resource_symbol];
184-
if (publicResource !== undefined)
181+
182+
const publicResource = resource[owner_symbol];
183+
184+
if (publicResource != null) {
185185
return publicResource;
186+
}
187+
186188
return resource;
187189
}
188190

lib/internal/js_stream_socket.js

+45-5
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,55 @@ const kCurrentWriteRequest = Symbol('kCurrentWriteRequest');
2222
const kCurrentShutdownRequest = Symbol('kCurrentShutdownRequest');
2323
const kPendingShutdownRequest = Symbol('kPendingShutdownRequest');
2424

25-
function isClosing() { return this[owner_symbol].isClosing(); }
25+
function isClosing() {
26+
let socket = this[owner_symbol];
2627

27-
function onreadstart() { return this[owner_symbol].readStart(); }
28+
if (socket.constructor.name === 'ReusedHandle') {
29+
socket = socket.handle;
30+
}
31+
32+
return socket.isClosing();
33+
}
34+
35+
function onreadstart() {
36+
let socket = this[owner_symbol];
37+
38+
if (socket.constructor.name === 'ReusedHandle') {
39+
socket = socket.handle;
40+
}
41+
42+
return socket.readStart();
43+
}
44+
45+
function onreadstop() {
46+
let socket = this[owner_symbol];
47+
48+
if (socket.constructor.name === 'ReusedHandle') {
49+
socket = socket.handle;
50+
}
51+
52+
return socket.readStop();
53+
}
54+
55+
function onshutdown(req) {
56+
let socket = this[owner_symbol];
57+
58+
if (socket.constructor.name === 'ReusedHandle') {
59+
socket = socket.handle;
60+
}
2861

29-
function onreadstop() { return this[owner_symbol].readStop(); }
62+
return socket.doShutdown(req);
63+
}
3064

31-
function onshutdown(req) { return this[owner_symbol].doShutdown(req); }
65+
function onwrite(req, bufs) {
66+
let socket = this[owner_symbol];
3267

33-
function onwrite(req, bufs) { return this[owner_symbol].doWrite(req, bufs); }
68+
if (socket.constructor.name === 'ReusedHandle') {
69+
socket = socket.handle;
70+
}
71+
72+
return socket.doWrite(req, bufs);
73+
}
3474

3575
/* This class serves as a wrapper for when the C++ side of Node wants access
3676
* to a standard JS stream. For example, TLS or HTTP do not operate on network

lib/internal/stream_base_commons.js

+11-2
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,11 @@ function handleWriteReq(req, data, encoding) {
8080
function onWriteComplete(status) {
8181
debug('onWriteComplete', status, this.error);
8282

83-
const stream = this.handle[owner_symbol];
83+
let stream = this.handle[owner_symbol];
84+
85+
if (stream.constructor.name === 'ReusedHandle') {
86+
stream = stream.handle;
87+
}
8488

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

170174
const handle = this;
171-
const stream = this[owner_symbol];
175+
176+
let stream = this[owner_symbol];
177+
178+
if (stream.constructor.name === 'ReusedHandle') {
179+
stream = stream.handle;
180+
}
172181

173182
stream[kUpdateTimer]();
174183

lib/net.js

+5-1
Original file line numberDiff line numberDiff line change
@@ -1102,7 +1102,11 @@ Socket.prototype.unref = function() {
11021102

11031103

11041104
function afterConnect(status, handle, req, readable, writable) {
1105-
const self = handle[owner_symbol];
1105+
let self = handle[owner_symbol];
1106+
1107+
if (self.constructor.name === 'ReusedHandle') {
1108+
self = self.handle;
1109+
}
11061110

11071111
// Callback may come after call to destroy
11081112
if (self.destroyed) {

src/async_wrap.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ void AsyncWrap::EmitDestroy(bool from_gc) {
313313

314314
if (!persistent().IsEmpty() && !from_gc) {
315315
HandleScope handle_scope(env()->isolate());
316-
USE(object()->Set(env()->context(), env()->resource_symbol(), object()));
316+
USE(object()->Set(env()->context(), env()->owner_symbol(), object()));
317317
}
318318
}
319319

@@ -586,7 +586,7 @@ void AsyncWrap::AsyncReset(Local<Object> resource, double execution_async_id,
586586
Local<Object> obj = object();
587587
CHECK(!obj.IsEmpty());
588588
if (resource != obj) {
589-
USE(obj->Set(env()->context(), env()->resource_symbol(), resource));
589+
USE(obj->Set(env()->context(), env()->owner_symbol(), resource));
590590
}
591591
}
592592

src/env.h

-1
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,6 @@ constexpr size_t kFsStatsBufferLength =
170170
V(oninit_symbol, "oninit") \
171171
V(owner_symbol, "owner_symbol") \
172172
V(onpskexchange_symbol, "onpskexchange") \
173-
V(resource_symbol, "resource_symbol") \
174173
V(trigger_async_id_symbol, "trigger_async_id_symbol") \
175174

176175
// Strings are per-isolate primitives but Environment proxies them

test/async-hooks/test-http-agent-handle-reuse-parallel.js

-2
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,4 @@ function onExit() {
8787
// Verify reuse handle has been wrapped
8888
assert.strictEqual(first.type, second.type);
8989
assert.ok(first.handle !== second.handle, 'Resource reused');
90-
assert.ok(first.handle === second.handle.handle,
91-
'Resource not wrapped correctly');
9290
}

test/async-hooks/test-http-agent-handle-reuse-serial.js

-2
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,4 @@ function onExit() {
105105
// Verify reuse handle has been wrapped
106106
assert.strictEqual(first.type, second.type);
107107
assert.ok(first.handle !== second.handle, 'Resource reused');
108-
assert.ok(first.handle === second.handle.handle,
109-
'Resource not wrapped correctly');
110108
}

0 commit comments

Comments
 (0)