From 646e5a471766e27e8317bb54d1fd1d2c72cffb69 Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Tue, 9 Jun 2020 14:50:03 -0700 Subject: [PATCH] domain: remove native domain code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the async_hooks callback trampoline, domains no longer need any native code. With this, domains can exist in pure JavaScript. PR-URL: https://github.com/nodejs/node/pull/33801 Reviewed-By: Anna Henningsen Reviewed-By: Vladimir de Turckheim Reviewed-By: Gus Caplan Reviewed-By: Andrey Pechkurov Reviewed-By: Gerhard Stöbich --- lib/domain.js | 3 ++- lib/internal/async_hooks.js | 12 +++++++++--- node.gyp | 1 - src/api/callback.cc | 19 +++---------------- src/env.h | 1 - src/node_binding.cc | 1 - src/node_domain.cc | 35 ----------------------------------- 7 files changed, 14 insertions(+), 58 deletions(-) delete mode 100644 src/node_domain.cc diff --git a/lib/domain.js b/lib/domain.js index f18823960cde02..febd7620f21500 100644 --- a/lib/domain.js +++ b/lib/domain.js @@ -42,6 +42,7 @@ const { ERR_UNHANDLED_ERROR } = require('internal/errors').codes; const { createHook } = require('async_hooks'); +const { useDomainTrampoline } = require('internal/async_hooks'); // TODO(addaleax): Use a non-internal solution for this. const kWeak = Symbol('kWeak'); @@ -145,7 +146,7 @@ function topLevelDomainCallback(cb, ...args) { // another one. The stack is each entered domain. let stack = []; exports._stack = stack; -internalBinding('domain').enable(topLevelDomainCallback); +useDomainTrampoline(topLevelDomainCallback); function updateExceptionCapture() { if (stack.every((domain) => domain.listenerCount('error') === 0)) { diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index e400f24689fbcc..fc894ebe4d773f 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -106,8 +106,13 @@ const emitDestroyNative = emitHookFactory(destroy_symbol, 'emitDestroyNative'); const emitPromiseResolveNative = emitHookFactory(promise_resolve_symbol, 'emitPromiseResolveNative'); -function callbackTrampoline(asyncId, cb, domain_cb, ...args) { - if (hasHooks(kBefore)) +let domain_cb; +function useDomainTrampoline(fn) { + domain_cb = fn; +} + +function callbackTrampoline(asyncId, cb, ...args) { + if (asyncId && hasHooks(kBefore)) emitBeforeNative(asyncId); let result; @@ -118,7 +123,7 @@ function callbackTrampoline(asyncId, cb, domain_cb, ...args) { result = ReflectApply(cb, this, args); } - if (hasHooks(kAfter)) + if (asyncId && hasHooks(kAfter)) emitAfterNative(asyncId); return result; @@ -564,6 +569,7 @@ module.exports = { emitAfter: emitAfterScript, emitDestroy: emitDestroyScript, registerDestroyHook, + useDomainTrampoline, nativeHooks: { init: emitInitNative, before: emitBeforeNative, diff --git a/node.gyp b/node.gyp index ddf59f4a0273cf..0af72d48c25150 100644 --- a/node.gyp +++ b/node.gyp @@ -590,7 +590,6 @@ 'src/node_contextify.cc', 'src/node_credentials.cc', 'src/node_dir.cc', - 'src/node_domain.cc', 'src/node_env_var.cc', 'src/node_errors.cc', 'src/node_file.cc', diff --git a/src/api/callback.cc b/src/api/callback.cc index 7daddcd977e33f..9f52c25cf0d900 100644 --- a/src/api/callback.cc +++ b/src/api/callback.cc @@ -173,29 +173,16 @@ MaybeLocal InternalMakeCallback(Environment* env, return MaybeLocal(); } - Local domain_cb = env->domain_callback(); MaybeLocal ret; - if (asyncContext.async_id != 0 && hook_count != 0) { - MaybeStackBuffer, 16> args(3 + argc); + if (hook_count != 0) { + MaybeStackBuffer, 16> args(2 + argc); args[0] = v8::Number::New(env->isolate(), asyncContext.async_id); args[1] = callback; - if (domain_cb.IsEmpty()) { - args[2] = Undefined(env->isolate()); - } else { - args[2] = domain_cb; - } for (int i = 0; i < argc; i++) { - args[i + 3] = argv[i]; + args[i + 2] = argv[i]; } ret = hook_cb->Call(env->context(), recv, args.length(), &args[0]); - } else if (asyncContext.async_id == 0 && !domain_cb.IsEmpty()) { - MaybeStackBuffer, 16> args(1 + argc); - args[0] = callback; - for (int i = 0; i < argc; i++) { - args[i + 1] = argv[i]; - } - ret = domain_cb->Call(env->context(), recv, args.length(), &args[0]); } else { ret = callback->Call(env->context(), recv, argc, argv); } diff --git a/src/env.h b/src/env.h index 09c8ff9db45ce4..ba3306a43347b8 100644 --- a/src/env.h +++ b/src/env.h @@ -486,7 +486,6 @@ constexpr size_t kFsStatsBufferLength = V(async_hooks_promise_resolve_function, v8::Function) \ V(buffer_prototype_object, v8::Object) \ V(crypto_key_object_constructor, v8::Function) \ - V(domain_callback, v8::Function) \ V(domexception_function, v8::Function) \ V(enhance_fatal_stack_after_inspector, v8::Function) \ V(enhance_fatal_stack_before_inspector, v8::Function) \ diff --git a/src/node_binding.cc b/src/node_binding.cc index e3014657bbe25b..80ab4e69e110d0 100644 --- a/src/node_binding.cc +++ b/src/node_binding.cc @@ -48,7 +48,6 @@ V(config) \ V(contextify) \ V(credentials) \ - V(domain) \ V(errors) \ V(fs) \ V(fs_dir) \ diff --git a/src/node_domain.cc b/src/node_domain.cc deleted file mode 100644 index 9075845442fd4d..00000000000000 --- a/src/node_domain.cc +++ /dev/null @@ -1,35 +0,0 @@ -#include "env-inl.h" -#include "v8.h" - -namespace node { -namespace domain { - -using v8::Context; -using v8::Function; -using v8::FunctionCallbackInfo; -using v8::Local; -using v8::Object; -using v8::Value; - - -void Enable(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - - CHECK(args[0]->IsFunction()); - - env->set_domain_callback(args[0].As()); -} - -void Initialize(Local target, - Local unused, - Local context, - void* priv) { - Environment* env = Environment::GetCurrent(context); - - env->SetMethod(target, "enable", Enable); -} - -} // namespace domain -} // namespace node - -NODE_MODULE_CONTEXT_AWARE_INTERNAL(domain, node::domain::Initialize)