From 619c5b6eb3dc04fa791e148110c0f1fcdd543efa Mon Sep 17 00:00:00 2001 From: Anna Henningsen <anna@addaleax.net> Date: Tue, 16 Apr 2019 13:31:44 +0200 Subject: [PATCH] src: do not require JS Context for `~AsyncResoure()` Allow the destructor to be called during GC, which is a common use case. PR-URL: https://github.com/nodejs/node/pull/27255 Fixes: https://github.com/nodejs/node/issues/27218 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> --- src/api/async_resource.cc | 20 ++++++++++++-------- src/node.h | 2 +- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/api/async_resource.cc b/src/api/async_resource.cc index 1d8224114e9511..5141c7c6b35abd 100644 --- a/src/api/async_resource.cc +++ b/src/api/async_resource.cc @@ -1,4 +1,5 @@ #include "node.h" +#include "env-inl.h" namespace node { @@ -15,21 +16,22 @@ AsyncResource::AsyncResource(Isolate* isolate, Local<Object> resource, const char* name, async_id trigger_async_id) - : isolate_(isolate), + : env_(Environment::GetCurrent(isolate)), resource_(isolate, resource) { + CHECK_NOT_NULL(env_); async_context_ = EmitAsyncInit(isolate, resource, name, trigger_async_id); } AsyncResource::~AsyncResource() { - EmitAsyncDestroy(isolate_, async_context_); + EmitAsyncDestroy(env_, async_context_); resource_.Reset(); } MaybeLocal<Value> AsyncResource::MakeCallback(Local<Function> callback, int argc, Local<Value>* argv) { - return node::MakeCallback(isolate_, get_resource(), + return node::MakeCallback(env_->isolate(), get_resource(), callback, argc, argv, async_context_); } @@ -37,7 +39,7 @@ MaybeLocal<Value> AsyncResource::MakeCallback(Local<Function> callback, MaybeLocal<Value> AsyncResource::MakeCallback(const char* method, int argc, Local<Value>* argv) { - return node::MakeCallback(isolate_, get_resource(), + return node::MakeCallback(env_->isolate(), get_resource(), method, argc, argv, async_context_); } @@ -45,13 +47,13 @@ MaybeLocal<Value> AsyncResource::MakeCallback(const char* method, MaybeLocal<Value> AsyncResource::MakeCallback(Local<String> symbol, int argc, Local<Value>* argv) { - return node::MakeCallback(isolate_, get_resource(), + return node::MakeCallback(env_->isolate(), get_resource(), symbol, argc, argv, async_context_); } Local<Object> AsyncResource::get_resource() { - return resource_.Get(isolate_); + return resource_.Get(env_->isolate()); } async_id AsyncResource::get_async_id() const { @@ -62,9 +64,11 @@ 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->isolate_, - res->resource_.Get(res->isolate_), + : node::CallbackScope(res->env_->isolate(), + res->resource_.Get(res->env_->isolate()), res->async_context_) {} } // namespace node diff --git a/src/node.h b/src/node.h index fadfd8b7866b49..5098dc9c7e77df 100644 --- a/src/node.h +++ b/src/node.h @@ -803,7 +803,7 @@ class NODE_EXTERN AsyncResource { }; private: - v8::Isolate* isolate_; + Environment* env_; v8::Persistent<v8::Object> resource_; async_context async_context_; };