diff --git a/configure b/configure index 7ce5e964e53ee5..01aacb8ed0f131 100755 --- a/configure +++ b/configure @@ -954,6 +954,7 @@ def configure_v8(o): o['variables']['v8_no_strict_aliasing'] = 1 # Work around compiler bugs. o['variables']['v8_optimized_debug'] = 0 # Compile with -O0 in debug builds. o['variables']['v8_random_seed'] = 0 # Use a random seed for hash tables. + o['variables']['v8_promise_internal_field_count'] = 1 # Add internal field to promises for async hooks. o['variables']['v8_use_snapshot'] = 'false' if options.without_snapshot else 'true' o['variables']['node_use_v8_platform'] = b(not options.without_v8_platform) o['variables']['node_use_bundled_v8'] = b(not options.without_bundled_v8) diff --git a/src/async-wrap.cc b/src/async-wrap.cc index 2efe0e23ecb72f..0ea6c64a8be582 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -283,8 +283,8 @@ bool AsyncWrap::EmitAfter(Environment* env, double async_id) { class PromiseWrap : public AsyncWrap { public: - PromiseWrap(Environment* env, Local object) - : AsyncWrap(env, object, PROVIDER_PROMISE) {} + PromiseWrap(Environment* env, Local object, bool silent) + : AsyncWrap(env, object, PROVIDER_PROMISE, silent) {} size_t self_size() const override { return sizeof(*this); } }; @@ -293,33 +293,14 @@ static void PromiseHook(PromiseHookType type, Local promise, Local parent, void* arg) { Local context = promise->CreationContext(); Environment* env = Environment::GetCurrent(context); - if (type == PromiseHookType::kInit) { - // Unfortunately, promises don't have internal fields. Need a surrogate that - // async wrap can wrap. - Local obj = - env->async_hooks_promise_object()->NewInstance(context).ToLocalChecked(); - PromiseWrap* wrap = new PromiseWrap(env, obj); - v8::PropertyAttribute hidden = - static_cast(v8::ReadOnly - | v8::DontDelete - | v8::DontEnum); - promise->DefineOwnProperty(context, - env->promise_wrap(), - v8::External::New(env->isolate(), wrap), - hidden).FromJust(); - // The async tag will be destroyed at the same time as the promise as the - // only reference to it is held by the promise. This allows the promise - // wrap instance to be notified when the promise is destroyed. - promise->DefineOwnProperty(context, - env->promise_async_tag(), - obj, hidden).FromJust(); + PromiseWrap* wrap = Unwrap(promise); + if (type == PromiseHookType::kInit || wrap == nullptr) { + bool silent = type != PromiseHookType::kInit; + wrap = new PromiseWrap(env, promise, silent); + wrap->MakeWeak(wrap); } else if (type == PromiseHookType::kResolve) { // TODO(matthewloring): need to expose this through the async hooks api. } - Local external_wrap = - promise->Get(context, env->promise_wrap()).ToLocalChecked(); - PromiseWrap* wrap = - static_cast(external_wrap.As()->Value()); CHECK_NE(wrap, nullptr); if (type == PromiseHookType::kBefore) { PreCallbackExecution(wrap, false); @@ -415,11 +396,6 @@ void AsyncWrap::Initialize(Local target, env->SetMethod(target, "clearIdStack", ClearIdStack); env->SetMethod(target, "addIdToDestroyList", QueueDestroyId); - Local promise_object_template = - v8::ObjectTemplate::New(env->isolate()); - promise_object_template->SetInternalFieldCount(1); - env->set_async_hooks_promise_object(promise_object_template); - v8::PropertyAttribute ReadOnlyDontDelete = static_cast(v8::ReadOnly | v8::DontDelete); @@ -516,7 +492,8 @@ void LoadAsyncWrapperInfo(Environment* env) { AsyncWrap::AsyncWrap(Environment* env, Local object, - ProviderType provider) + ProviderType provider, + bool silent) : BaseObject(env, object), provider_type_(provider) { CHECK_NE(provider, PROVIDER_NONE); @@ -526,7 +503,7 @@ AsyncWrap::AsyncWrap(Environment* env, persistent().SetWrapperClassId(NODE_ASYNC_ID_OFFSET + provider); // Use AsyncReset() call to execute the init() callbacks. - AsyncReset(); + AsyncReset(silent); } @@ -538,10 +515,12 @@ AsyncWrap::~AsyncWrap() { // Generalized call for both the constructor and for handles that are pooled // and reused over their lifetime. This way a new uid can be assigned when // the resource is pulled out of the pool and put back into use. -void AsyncWrap::AsyncReset() { +void AsyncWrap::AsyncReset(bool silent) { async_id_ = env()->new_async_id(); trigger_id_ = env()->get_init_trigger_id(); + if (silent) return; + EmitAsyncInit(env(), object(), env()->async_hooks()->provider_string(provider_type()), async_id_, trigger_id_); diff --git a/src/async-wrap.h b/src/async-wrap.h index d3676a01c06705..fa37ea13a65993 100644 --- a/src/async-wrap.h +++ b/src/async-wrap.h @@ -86,7 +86,8 @@ class AsyncWrap : public BaseObject { AsyncWrap(Environment* env, v8::Local object, - ProviderType provider); + ProviderType provider, + bool silent = false); virtual ~AsyncWrap(); @@ -116,7 +117,7 @@ class AsyncWrap : public BaseObject { inline double get_trigger_id() const; - void AsyncReset(); + void AsyncReset(bool silent = false); // Only call these within a valid HandleScope. // TODO(trevnorris): These should return a MaybeLocal. diff --git a/src/env.h b/src/env.h index 11c957d2be135b..c8c8232cc07fd4 100644 --- a/src/env.h +++ b/src/env.h @@ -195,8 +195,6 @@ namespace node { V(preference_string, "preference") \ V(priority_string, "priority") \ V(produce_cached_data_string, "produceCachedData") \ - V(promise_wrap, "_promise_async_wrap") \ - V(promise_async_tag, "_promise_async_wrap_tag") \ V(raw_string, "raw") \ V(read_host_object_string, "_readHostObject") \ V(readable_string, "readable") \ @@ -258,7 +256,6 @@ namespace node { V(async_hooks_init_function, v8::Function) \ V(async_hooks_before_function, v8::Function) \ V(async_hooks_after_function, v8::Function) \ - V(async_hooks_promise_object, v8::ObjectTemplate) \ V(binding_cache_object, v8::Object) \ V(buffer_constructor_function, v8::Function) \ V(buffer_prototype_object, v8::Object) \ diff --git a/test/parallel/test-async-wrap-promise-after-enabled.js b/test/parallel/test-async-wrap-promise-after-enabled.js new file mode 100644 index 00000000000000..475411a3dae531 --- /dev/null +++ b/test/parallel/test-async-wrap-promise-after-enabled.js @@ -0,0 +1,34 @@ +'use strict'; + +// Regression test for https://github.com/nodejs/node/issues/13237 + +const common = require('../common'); +const assert = require('assert'); + +const async_hooks = require('async_hooks'); + +const seenEvents = []; + +const p = new Promise((resolve) => resolve(1)); +p.then(() => seenEvents.push('then')); + +const hooks = async_hooks.createHook({ + init: common.mustNotCall(), + + before: common.mustCall((id) => { + assert.ok(id > 1); + seenEvents.push('before'); + }), + + after: common.mustCall((id) => { + assert.ok(id > 1); + seenEvents.push('after'); + hooks.disable(); + }) +}); + +setImmediate(() => { + assert.deepStrictEqual(seenEvents, ['before', 'then', 'after']); +}); + +hooks.enable(); // After `setImmediate` in order to not catch its init event.