From 787143bf3e3a96bed9da17c453c8f77e016fd1b1 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 3 Mar 2020 10:29:06 +0000 Subject: [PATCH] src: pass resource object along with InternalMakeCallback This was an oversight in 9fdb6e6aaf45b2364bac89a. Fixing this is necessary to make `executionAsyncResource()` work as expected. Refs: https://github.com/nodejs/node/pull/30959 Fixes: https://github.com/nodejs/node/issues/32060 PR-URL: https://github.com/nodejs/node/pull/32063 Reviewed-By: Vladimir de Turckheim Reviewed-By: Stephen Belanger Reviewed-By: Gireesh Punathil Reviewed-By: James M Snell --- src/api/callback.cc | 5 ++- src/async_wrap.cc | 2 +- src/node_internals.h | 1 + .../test-async-exec-resource-http-32060.js | 37 +++++++++++++++++++ 4 files changed, 42 insertions(+), 3 deletions(-) create mode 100644 test/async-hooks/test-async-exec-resource-http-32060.js diff --git a/src/api/callback.cc b/src/api/callback.cc index 1fb85c5883f120..f7e7ddedfae377 100644 --- a/src/api/callback.cc +++ b/src/api/callback.cc @@ -139,6 +139,7 @@ void InternalCallbackScope::Close() { } MaybeLocal InternalMakeCallback(Environment* env, + Local resource, Local recv, const Local callback, int argc, @@ -150,7 +151,7 @@ MaybeLocal InternalMakeCallback(Environment* env, CHECK(!argv[i].IsEmpty()); #endif - InternalCallbackScope scope(env, recv, asyncContext); + InternalCallbackScope scope(env, resource, asyncContext); if (scope.Failed()) { return MaybeLocal(); } @@ -224,7 +225,7 @@ MaybeLocal MakeCallback(Isolate* isolate, CHECK_NOT_NULL(env); Context::Scope context_scope(env->context()); MaybeLocal ret = - InternalMakeCallback(env, recv, callback, argc, argv, asyncContext); + InternalMakeCallback(env, recv, recv, callback, argc, argv, asyncContext); if (ret.IsEmpty() && env->async_callback_scope_depth() == 0) { // This is only for legacy compatibility and we may want to look into // removing/adjusting it. diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 892e33c624bb88..b35cdca08a6f87 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -749,7 +749,7 @@ MaybeLocal AsyncWrap::MakeCallback(const Local cb, ProviderType provider = provider_type(); async_context context { get_async_id(), get_trigger_async_id() }; MaybeLocal ret = InternalMakeCallback( - env(), object(), cb, argc, argv, context); + env(), GetResource(), object(), cb, argc, argv, context); // This is a static call with cached values because the `this` object may // no longer be alive at this point. diff --git a/src/node_internals.h b/src/node_internals.h index 91ba2a58a755ae..7dcbf65f8e698a 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -199,6 +199,7 @@ static v8::MaybeLocal New(Environment* env, v8::MaybeLocal InternalMakeCallback( Environment* env, + v8::Local resource, v8::Local recv, const v8::Local callback, int argc, diff --git a/test/async-hooks/test-async-exec-resource-http-32060.js b/test/async-hooks/test-async-exec-resource-http-32060.js new file mode 100644 index 00000000000000..0ff68aa1070e19 --- /dev/null +++ b/test/async-hooks/test-async-exec-resource-http-32060.js @@ -0,0 +1,37 @@ +'use strict'; +require('../common'); +const assert = require('assert'); +const { + executionAsyncResource, + executionAsyncId, + createHook, +} = require('async_hooks'); +const http = require('http'); + +const hooked = {}; +createHook({ + init(asyncId, type, triggerAsyncId, resource) { + hooked[asyncId] = resource; + } +}).enable(); + +const server = http.createServer((req, res) => { + res.write('hello'); + setTimeout(() => { + res.end(' world!'); + }, 1000); +}); + +server.listen(0, () => { + assert.strictEqual(executionAsyncResource(), hooked[executionAsyncId()]); + http.get({ port: server.address().port }, (res) => { + assert.strictEqual(executionAsyncResource(), hooked[executionAsyncId()]); + res.on('data', () => { + assert.strictEqual(executionAsyncResource(), hooked[executionAsyncId()]); + }); + res.on('end', () => { + assert.strictEqual(executionAsyncResource(), hooked[executionAsyncId()]); + server.close(); + }); + }); +});