diff --git a/configure.py b/configure.py index 1d3e2917081a55..4d81d33e26c2cf 100755 --- a/configure.py +++ b/configure.py @@ -1322,7 +1322,7 @@ def configure_v8(o): o['variables']['v8_optimized_debug'] = 0 if options.v8_non_optimized_debug else 1 o['variables']['dcheck_always_on'] = 1 if options.v8_with_dchecks else 0 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_promise_internal_field_count'] = 2 # Add internal fields to promises for async hooks. o['variables']['v8_use_siphash'] = 0 if options.without_siphash else 1 o['variables']['v8_enable_pointer_compression'] = 1 if options.enable_pointer_compression else 0 o['variables']['v8_enable_31bit_smis_on_64bit_arch'] = 1 if options.enable_pointer_compression else 0 diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index f4d4f1da49c4ca..8603741eb05356 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -6,7 +6,6 @@ const { FunctionPrototypeBind, ObjectPrototypeHasOwnProperty, ObjectDefineProperty, - Promise, ReflectApply, Symbol, } = primordials; @@ -297,34 +296,14 @@ function restoreActiveHooks() { active_hooks.tmp_fields = null; } -function trackPromise(promise, parent, silent) { - const asyncId = getOrSetAsyncId(promise); - - promise[trigger_async_id_symbol] = parent ? getOrSetAsyncId(parent) : - getDefaultTriggerAsyncId(); - - if (!silent && initHooksExist()) { - const triggerId = promise[trigger_async_id_symbol]; +// TODO(puzpuzpuz) remove +function fastPromiseHook(type, promise, asyncId, triggerId) { + if (type === kInit && initHooksExist()) { emitInitScript(asyncId, 'PROMISE', triggerId, promise); } -} - -function fastPromiseHook(type, promise, parent) { - if (type === kInit || !promise[async_id_symbol]) { - const silent = type !== kInit; - if (parent instanceof Promise) { - trackPromise(promise, parent, silent); - } else { - trackPromise(promise, null, silent); - } - - if (!silent) return; - } - const asyncId = promise[async_id_symbol]; switch (type) { case kBefore: - const triggerId = promise[trigger_async_id_symbol]; emitBeforeScript(asyncId, triggerId, promise); break; case kAfter: diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 26451f22b51a34..add29597f3ba81 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -189,6 +189,43 @@ void AsyncWrap::EmitAfter(Environment* env, double async_id) { env->async_hooks_after_function()); } +static double GetOrAssignPromiseAsyncId(Environment* env, + Local promise, + bool read_only) { + if (!read_only && + promise->GetAlignedPointerFromInternalField(0) == nullptr) { + double async_id = env->new_async_id(); + promise->SetInternalField(0, Number::New(env->isolate(), async_id)); + return async_id; + } + + Local obj = promise->GetInternalField(0); + if (obj->IsNumber()) { + return obj.As()->Value(); + } + + return AsyncWrap::kInvalidAsyncId; +} + +static double GetPromiseTriggerAsyncId(Environment* env, + Local promise) { + if (promise->GetAlignedPointerFromInternalField(1) == nullptr) { + return AsyncWrap::kInvalidAsyncId; + } + + Local obj = promise->GetInternalField(1); + return obj->IsNumber() ? obj.As()->Value() + : AsyncWrap::kInvalidAsyncId; + + return AsyncWrap::kInvalidAsyncId; +} + +static void AssignPromiseTriggerAsyncId(Environment* env, + Local promise, + double trigger_async_id) { + promise->SetInternalField(1, Number::New(env->isolate(), trigger_async_id)); +} + class PromiseWrap : public AsyncWrap { public: PromiseWrap(Environment* env, Local object, bool silent) @@ -226,24 +263,21 @@ PromiseWrap* PromiseWrap::New(Environment* env, if (!env->promise_wrap_template()->NewInstance(context).ToLocal(&obj)) return nullptr; - CHECK_NULL(promise->GetAlignedPointerFromInternalField(0)); - promise->SetInternalField(0, obj); - + double async_id = kInvalidAsyncId; + double trigger_async_id = kInvalidAsyncId; // Skip for init events if (silent) { - Local maybe_async_id = promise - ->Get(context, env->async_id_symbol()) - .ToLocalChecked(); - - Local maybe_trigger_async_id = promise - ->Get(context, env->trigger_async_id_symbol()) - .ToLocalChecked(); - - if (maybe_async_id->IsNumber() && maybe_trigger_async_id->IsNumber()) { - double async_id = maybe_async_id.As()->Value(); - double trigger_async_id = maybe_trigger_async_id.As()->Value(); - return new PromiseWrap(env, obj, async_id, trigger_async_id); - } + // Interop with fast hook. + async_id = GetOrAssignPromiseAsyncId(env, promise, true); + trigger_async_id = GetPromiseTriggerAsyncId(env, promise); + } + + promise->SetInternalField(0, obj); + + if (async_id != kInvalidAsyncId && trigger_async_id != kInvalidAsyncId) { + // Clean up internal fields. + promise->SetInternalField(1, Undefined(env->isolate())); + return new PromiseWrap(env, obj, async_id, trigger_async_id); } return new PromiseWrap(env, obj, silent); @@ -312,75 +346,62 @@ static uint16_t ToAsyncHooksType(PromiseHookType type) { UNREACHABLE(); } -// Simplified JavaScript hook fast-path for when there is no destroy hook +// Simplified fast-path hook for when there is no destroy hook static void FastPromiseHook(PromiseHookType type, Local promise, Local parent) { Local context = promise->CreationContext(); Environment* env = Environment::GetCurrent(context); if (env == nullptr) return; - if (type == PromiseHookType::kBefore && - env->async_hooks()->fields()[AsyncHooks::kBefore] == 0) { - Local maybe_async_id; - if (!promise->Get(context, env->async_id_symbol()) - .ToLocal(&maybe_async_id)) { - return; - } - - Local maybe_trigger_async_id; - if (!promise->Get(context, env->trigger_async_id_symbol()) - .ToLocal(&maybe_trigger_async_id)) { - return; - } - - if (maybe_async_id->IsNumber() && maybe_trigger_async_id->IsNumber()) { - double async_id = maybe_async_id.As()->Value(); - double trigger_async_id = maybe_trigger_async_id.As()->Value(); - env->async_hooks()->push_async_context( - async_id, trigger_async_id, promise); - } - - return; + double async_id = GetOrAssignPromiseAsyncId(env, promise, false); + if (async_id == AsyncWrap::kInvalidAsyncId) { + // Interop with slow hook. + PromiseWrap* wrap = extractPromiseWrap(promise); + if (wrap == nullptr) return; + async_id = wrap->get_async_id(); } - if (type == PromiseHookType::kAfter && - env->async_hooks()->fields()[AsyncHooks::kAfter] == 0) { - Local maybe_async_id; - if (!promise->Get(context, env->async_id_symbol()) - .ToLocal(&maybe_async_id)) { - return; - } - - if (maybe_async_id->IsNumber()) { - double async_id = maybe_async_id.As()->Value(); - if (env->execution_async_id() == async_id) { - // This condition might not be true if async_hooks was enabled during - // the promise callback execution. - env->async_hooks()->pop_async_context(async_id); + double trigger_async_id = GetPromiseTriggerAsyncId(env, promise); + if (trigger_async_id == AsyncWrap::kInvalidAsyncId) { + // Interop with slow hook. + PromiseWrap* wrap = extractPromiseWrap(promise); + if (wrap != nullptr) { + trigger_async_id = wrap->get_trigger_async_id(); + } else if (parent->IsPromise()) { + Local parent_promise = parent.As(); + trigger_async_id = GetOrAssignPromiseAsyncId(env, parent_promise, false); + if (trigger_async_id == AsyncWrap::kInvalidAsyncId) { + // Interop with slow hook. + PromiseWrap* wrap = extractPromiseWrap(parent_promise); + if (wrap == nullptr) return; + trigger_async_id = wrap->get_trigger_async_id(); + } else { + AssignPromiseTriggerAsyncId(env, promise, trigger_async_id); } + } else { + trigger_async_id = env->get_default_trigger_async_id(); } - - return; } - if (type == PromiseHookType::kResolve && - env->async_hooks()->fields()[AsyncHooks::kPromiseResolve] == 0) { - return; + if (type == PromiseHookType::kInit) { + AsyncWrap::EmitAsyncInit(env, promise, + env->async_hooks() + ->provider_string(AsyncWrap::PROVIDER_PROMISE), + async_id, trigger_async_id); + } else if (type == PromiseHookType::kBefore) { + env->async_hooks()->push_async_context( + async_id, trigger_async_id, promise); + AsyncWrap::EmitBefore(env, async_id); + } else if (type == PromiseHookType::kAfter) { + AsyncWrap::EmitAfter(env, async_id); + if (env->execution_async_id() == async_id) { + // This condition might not be true if async_hooks was enabled during + // the promise callback execution. + env->async_hooks()->pop_async_context(async_id); + } + } else if (type == PromiseHookType::kResolve) { + AsyncWrap::EmitPromiseResolve(env, async_id); } - - // Getting up to this point means either init type or - // that there are active hooks of another type. - // In both cases fast-path JS hook should be called. - - Local argv[] = { - Integer::New(env->isolate(), ToAsyncHooksType(type)), - promise, - parent - }; - - TryCatchScope try_catch(env, TryCatchScope::CatchMode::kFatal); - Local promise_hook = env->promise_hook_handler(); - USE(promise_hook->Call(context, Undefined(env->isolate()), 3, argv)); } static void FullPromiseHook(PromiseHookType type, Local promise, diff --git a/test/addons/async-hooks-promise/binding.cc b/test/addons/async-hooks-promise/binding.cc index 452cbda8793aa1..3d6a0fd4c86888 100644 --- a/test/addons/async-hooks-promise/binding.cc +++ b/test/addons/async-hooks-promise/binding.cc @@ -6,7 +6,7 @@ namespace { using v8::FunctionCallbackInfo; using v8::Isolate; using v8::Local; -using v8::NewStringType; +using v8::Number; using v8::Object; using v8::Promise; using v8::String; @@ -23,13 +23,20 @@ static void GetPromiseField(const FunctionCallbackInfo& args) { auto isolate = args.GetIsolate(); if (!args[0]->IsPromise()) - return ThrowError(isolate, "arg is not an Promise"); + return ThrowError(isolate, "arg 0 is not an Promise"); + + if (!args[1]->IsNumber()) + return ThrowError(isolate, "arg 1 is not a number"); auto p = args[0].As(); - if (p->InternalFieldCount() < 1) - return ThrowError(isolate, "Promise has no internal field"); + if (p->InternalFieldCount() < 2) + return ThrowError(isolate, "Promise has no internal fields"); + + double fieldIdx = args[1].As()->Value(); + if (fieldIdx != 0 && fieldIdx != 1) + return ThrowError(isolate, "Index has to be 0 or 1"); - auto l = p->GetInternalField(0); + auto l = p->GetInternalField(fieldIdx); args.GetReturnValue().Set(l); } diff --git a/test/addons/async-hooks-promise/test.js b/test/addons/async-hooks-promise/test.js index d38bf9bd978103..f3ee0d44af615c 100644 --- a/test/addons/async-hooks-promise/test.js +++ b/test/addons/async-hooks-promise/test.js @@ -13,16 +13,19 @@ if (process.env.NODE_TEST_WITH_ASYNC_HOOKS) { return; } -// Baseline to make sure the internal field isn't being set. +// Baseline to make sure both internal fields aren't set. assert.strictEqual( - binding.getPromiseField(Promise.resolve(1)), + binding.getPromiseField(Promise.resolve(1), 0), + 0); +assert.strictEqual( + binding.getPromiseField(Promise.resolve(1), 1), 0); const emptyHook = async_hooks.createHook({}).enable(); // Check that no PromiseWrap is created when there are no hook callbacks. assert.strictEqual( - binding.getPromiseField(Promise.resolve(1)), + binding.getPromiseField(Promise.resolve(1), 1), 0); emptyHook.disable(); @@ -41,10 +44,11 @@ const initOnlyHook = async_hooks.createHook({ // Check that no PromiseWrap is created when only using an init hook. { const promise = Promise.resolve(1); - assert.strictEqual(binding.getPromiseField(promise), 0); + assert.strictEqual(typeof binding.getPromiseField(promise, 0), 'number'); + assert.strictEqual(typeof binding.getPromiseField(promise, 1), 'number'); assert.strictEqual(lastResource, promise); - assert.strictEqual(lastAsyncId, promise[async_id_symbol]); - assert.strictEqual(lastTriggerAsyncId, promise[trigger_async_id_symbol]); + assert.strictEqual(lastAsyncId, binding.getPromiseField(promise, 0)); + assert.strictEqual(lastTriggerAsyncId, binding.getPromiseField(promise, 1)); } initOnlyHook.disable(); @@ -65,7 +69,8 @@ const hookWithDestroy = async_hooks.createHook({ // Check that the internal field returns the same PromiseWrap passed to init(). { const promise = Promise.resolve(1); - const promiseWrap = binding.getPromiseField(promise); + const promiseWrap = binding.getPromiseField(promise, 0); + assert.strictEqual(binding.getPromiseField(promise, 1), 0); assert.strictEqual(lastResource, promiseWrap); assert.strictEqual(lastAsyncId, promiseWrap[async_id_symbol]); assert.strictEqual(lastTriggerAsyncId, promiseWrap[trigger_async_id_symbol]); @@ -78,7 +83,10 @@ hookWithDestroy.disable(); // future microtask. setImmediate(() => { assert.strictEqual( - binding.getPromiseField(Promise.resolve(1)), + binding.getPromiseField(Promise.resolve(1), 0), + 0); + assert.strictEqual( + binding.getPromiseField(Promise.resolve(1), 1), 0); const noDestroyHook = async_hooks.createHook({ @@ -95,10 +103,11 @@ setImmediate(() => { // Check that no PromiseWrap is created when there is no destroy hook. const promise = Promise.resolve(1); - assert.strictEqual(binding.getPromiseField(promise), 0); + assert.strictEqual(typeof binding.getPromiseField(promise, 0), 'number'); + assert.strictEqual(typeof binding.getPromiseField(promise, 1), 'number'); assert.strictEqual(lastResource, promise); - assert.strictEqual(lastAsyncId, promise[async_id_symbol]); - assert.strictEqual(lastTriggerAsyncId, promise[trigger_async_id_symbol]); + assert.strictEqual(lastAsyncId, binding.getPromiseField(promise, 0)); + assert.strictEqual(lastTriggerAsyncId, binding.getPromiseField(promise, 1)); noDestroyHook.disable(); });