From bb1bd7639554766a88b8f7aef8891bf8249e613e Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Tue, 20 Oct 2015 12:20:10 -0600 Subject: [PATCH] async_wrap: call callback in destructor 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: https://github.com/nodejs/node/pull/3461 Reviewed-By: Fedor Indutny --- src/async-wrap-inl.h | 19 ++++++++++++++++++- src/async-wrap.cc | 3 +++ src/async-wrap.h | 2 +- src/env.h | 1 + ...st-async-wrap-disabled-propagate-parent.js | 2 +- .../test-async-wrap-propagate-parent.js | 2 +- 6 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/async-wrap-inl.h b/src/async-wrap-inl.h index 9aaf67d3a8c518..1d9ebe27e45bef 100644 --- a/src/async-wrap-inl.h +++ b/src/async-wrap-inl.h @@ -41,11 +41,12 @@ inline AsyncWrap::AsyncWrap(Environment* env, v8::Local 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 ret = init_fn->Call(env->context(), object, ARRAY_SIZE(argv), argv); @@ -57,6 +58,22 @@ inline AsyncWrap::AsyncWrap(Environment* env, } +inline AsyncWrap::~AsyncWrap() { + if (!ran_init_callback()) + return; + + v8::Local fn = env()->async_hooks_destroy_function(); + if (!fn.IsEmpty()) { + v8::HandleScope scope(env()->isolate()); + v8::Local uid = v8::Integer::New(env()->isolate(), get_uid()); + v8::MaybeLocal 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(bits_ & 1); } diff --git a/src/async-wrap.cc b/src/async-wrap.cc index 767e76831c3d04..c9f5caad1e4ea8 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -131,6 +131,8 @@ static void SetupHooks(const FunctionCallbackInfo& args) { env->set_async_hooks_pre_function(args[1].As()); if (args[2]->IsFunction()) env->set_async_hooks_post_function(args[2].As()); + if (args[3]->IsFunction()) + env->set_async_hooks_destroy_function(args[3].As()); } @@ -156,6 +158,7 @@ static void Initialize(Local target, env->set_async_hooks_init_function(Local()); env->set_async_hooks_pre_function(Local()); env->set_async_hooks_post_function(Local()); + env->set_async_hooks_destroy_function(Local()); } diff --git a/src/async-wrap.h b/src/async-wrap.h index 5fbd2309383d87..5db29600bcd180 100644 --- a/src/async-wrap.h +++ b/src/async-wrap.h @@ -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; diff --git a/src/env.h b/src/env.h index cdfd19c9740e31..93f7c47c9919a3 100644 --- a/src/env.h +++ b/src/env.h @@ -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) \ diff --git a/test/parallel/test-async-wrap-disabled-propagate-parent.js b/test/parallel/test-async-wrap-disabled-propagate-parent.js index de36071524c4fe..70d82befe72a63 100644 --- a/test/parallel/test-async-wrap-disabled-propagate-parent.js +++ b/test/parallel/test-async-wrap-disabled-propagate-parent.js @@ -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. diff --git a/test/parallel/test-async-wrap-propagate-parent.js b/test/parallel/test-async-wrap-propagate-parent.js index 8074b0062e0db2..beeb27ba7866a9 100644 --- a/test/parallel/test-async-wrap-propagate-parent.js +++ b/test/parallel/test-async-wrap-propagate-parent.js @@ -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.