From 85073c523dc2e7ec86306d28e71b31ba950935e1 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Sun, 15 Aug 2021 11:00:35 +0530 Subject: [PATCH 1/2] src: add a constructor overload for CallbackScope This overload accepts the current Environment* as an argument, unlike the other constructor, which accepts an Isolate*. This is useful because we can pass the current Environment* directly instead of recomputing it from the Isolate* inside the constructor. Signed-off-by: Darshan Sen --- src/api/async_resource.cc | 4 +--- src/api/callback.cc | 10 ++++++++++ src/node.h | 3 +++ src/node_api.cc | 2 +- 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/api/async_resource.cc b/src/api/async_resource.cc index 0a2437fe6eda5c..3c4fbdadbc462c 100644 --- a/src/api/async_resource.cc +++ b/src/api/async_resource.cc @@ -62,10 +62,8 @@ async_id AsyncResource::get_trigger_async_id() const { return async_context_.trigger_async_id; } -// TODO(addaleax): We shouldn’t need to use env_->isolate() if we’re just going -// to end up using the Isolate* to figure out the Environment* again. AsyncResource::CallbackScope::CallbackScope(AsyncResource* res) - : node::CallbackScope(res->env_->isolate(), + : node::CallbackScope(res->env_, res->resource_.Get(res->env_->isolate()), res->async_context_) {} diff --git a/src/api/callback.cc b/src/api/callback.cc index 911b1160eba342..8f9d617d1eaf3f 100644 --- a/src/api/callback.cc +++ b/src/api/callback.cc @@ -26,6 +26,16 @@ CallbackScope::CallbackScope(Isolate* isolate, try_catch_.SetVerbose(true); } +CallbackScope::CallbackScope(Environment* env, + Local object, + async_context asyncContext) + : private_(new InternalCallbackScope(env, + object, + asyncContext)), + try_catch_(env->isolate()) { + try_catch_.SetVerbose(true); +} + CallbackScope::~CallbackScope() { if (try_catch_.HasCaught()) private_->MarkAsFailed(); diff --git a/src/node.h b/src/node.h index 1628a2147ea203..dda7f1b517d059 100644 --- a/src/node.h +++ b/src/node.h @@ -1005,6 +1005,9 @@ class NODE_EXTERN CallbackScope { CallbackScope(v8::Isolate* isolate, v8::Local resource, async_context asyncContext); + CallbackScope(Environment* env, + v8::Local resource, + async_context asyncContext); ~CallbackScope(); void operator=(const CallbackScope&) = delete; diff --git a/src/node_api.cc b/src/node_api.cc index 8304ccb7e86a08..b1b85073508672 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -539,7 +539,7 @@ class AsyncContext { class CallbackScope : public node::CallbackScope { public: explicit CallbackScope(AsyncContext* async_context) - : node::CallbackScope(async_context->node_env()->isolate(), + : node::CallbackScope(async_context->node_env(), async_context->resource_.Get( async_context->node_env()->isolate()), async_context->async_context()) {} From 795069ad13551e59b61c1312c5cf228a58791745 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Tue, 17 Aug 2021 20:32:38 +0530 Subject: [PATCH 2/2] src: call overload ctor from the original ctor Call the new constructor overload from the original constructor to reduce code duplication. Signed-off-by: Darshan Sen --- src/api/callback.cc | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/api/callback.cc b/src/api/callback.cc index 8f9d617d1eaf3f..fb0e5586eb400a 100644 --- a/src/api/callback.cc +++ b/src/api/callback.cc @@ -18,13 +18,8 @@ using v8::Value; CallbackScope::CallbackScope(Isolate* isolate, Local object, - async_context asyncContext) - : private_(new InternalCallbackScope(Environment::GetCurrent(isolate), - object, - asyncContext)), - try_catch_(isolate) { - try_catch_.SetVerbose(true); -} + async_context async_context) + : CallbackScope(Environment::GetCurrent(isolate), object, async_context) {} CallbackScope::CallbackScope(Environment* env, Local object,