Skip to content

Commit

Permalink
async_wrap: call callback in destructor
Browse files Browse the repository at this point in the history
Call a user's callback to notify that the handle has been destroyed.
Only pass the id of the AsyncWrap instance since the object no longer
exists.

The object that's being destructed should never be inspected within the
callback or any time afterward.

This commit make a breaking change. The init callback will now be passed
arguments in the order of provider, id, parent.

PR-URL: #3461
Reviewed-By: Fedor Indutny <fedor@indutny.com>
  • Loading branch information
trevnorris authored and rvagg committed Nov 7, 2015
1 parent eccbec9 commit b663d2b
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 4 deletions.
19 changes: 18 additions & 1 deletion src/async-wrap-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,12 @@ inline AsyncWrap::AsyncWrap(Environment* env,

v8::Local<v8::Value> argv[] = {
v8::Int32::New(env->isolate(), provider),
v8::Integer::New(env->isolate(), get_uid()),
Null(env->isolate())
};

if (parent != nullptr)
argv[1] = parent->object();
argv[2] = parent->object();

v8::MaybeLocal<v8::Value> ret =
init_fn->Call(env->context(), object, ARRAY_SIZE(argv), argv);
Expand All @@ -57,6 +58,22 @@ inline AsyncWrap::AsyncWrap(Environment* env,
}


inline AsyncWrap::~AsyncWrap() {
if (!ran_init_callback())
return;

v8::Local<v8::Function> fn = env()->async_hooks_destroy_function();
if (!fn.IsEmpty()) {
v8::HandleScope scope(env()->isolate());
v8::Local<v8::Value> uid = v8::Integer::New(env()->isolate(), get_uid());
v8::MaybeLocal<v8::Value> ret =
fn->Call(env()->context(), v8::Null(env()->isolate()), 1, &uid);
if (ret.IsEmpty())
FatalError("node::AsyncWrap::~AsyncWrap", "destroy hook threw");
}
}


inline bool AsyncWrap::ran_init_callback() const {
return static_cast<bool>(bits_ & 1);
}
Expand Down
3 changes: 3 additions & 0 deletions src/async-wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ static void SetupHooks(const FunctionCallbackInfo<Value>& args) {
env->set_async_hooks_pre_function(args[1].As<Function>());
if (args[2]->IsFunction())
env->set_async_hooks_post_function(args[2].As<Function>());
if (args[3]->IsFunction())
env->set_async_hooks_destroy_function(args[3].As<Function>());
}


Expand All @@ -156,6 +158,7 @@ static void Initialize(Local<Object> target,
env->set_async_hooks_init_function(Local<Function>());
env->set_async_hooks_pre_function(Local<Function>());
env->set_async_hooks_post_function(Local<Function>());
env->set_async_hooks_destroy_function(Local<Function>());
}


Expand Down
2 changes: 1 addition & 1 deletion src/async-wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class AsyncWrap : public BaseObject {
ProviderType provider,
AsyncWrap* parent = nullptr);

inline virtual ~AsyncWrap() override = default;
inline virtual ~AsyncWrap();

inline ProviderType provider_type() const;

Expand Down
1 change: 1 addition & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ namespace node {
V(async_hooks_init_function, v8::Function) \
V(async_hooks_pre_function, v8::Function) \
V(async_hooks_post_function, v8::Function) \
V(async_hooks_destroy_function, v8::Function) \
V(binding_cache_object, v8::Object) \
V(buffer_constructor_function, v8::Function) \
V(buffer_prototype_object, v8::Object) \
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-async-wrap-disabled-propagate-parent.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ let cntr = 0;
let server;
let client;

function init(type, parent) {
function init(type, id, parent) {
if (parent) {
cntr++;
// Cannot assert in init callback or will abort.
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-async-wrap-propagate-parent.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ let cntr = 0;
let server;
let client;

function init(type, parent) {
function init(type, id, parent) {
if (parent) {
cntr++;
// Cannot assert in init callback or will abort.
Expand Down

0 comments on commit b663d2b

Please sign in to comment.