Skip to content

Commit

Permalink
src: do not require JS Context for ~AsyncResoure()
Browse files Browse the repository at this point in the history
Allow the destructor to be called during GC,
which is a common use case.

PR-URL: #27255
Fixes: #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>
  • Loading branch information
addaleax authored and targos committed Apr 27, 2019
1 parent 8b5d738 commit 619c5b6
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 9 deletions.
20 changes: 12 additions & 8 deletions src/api/async_resource.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "node.h"
#include "env-inl.h"

namespace node {

Expand All @@ -15,43 +16,44 @@ 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_);
}

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_);
}

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 {
Expand All @@ -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
2 changes: 1 addition & 1 deletion src/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -803,7 +803,7 @@ class NODE_EXTERN AsyncResource {
};

private:
v8::Isolate* isolate_;
Environment* env_;
v8::Persistent<v8::Object> resource_;
async_context async_context_;
};
Expand Down

0 comments on commit 619c5b6

Please sign in to comment.