From 5c6cf30143f3191b043ba0b4e814768efa1069f7 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 9 Sep 2017 22:28:02 +0200 Subject: src: add environment cleanup hooks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds pairs of methods to the `Environment` class and to public APIs which can add and remove cleanup handlers. Unlike `AtExit`, this API targets addon developers rather than embedders, giving them (and Node’s internals) the ability to register per-`Environment` cleanup work. We may want to replace `AtExit` with this API at some point. Many thanks for Stephen Belanger for reviewing the original version of this commit in the Ayo.js project. Refs: https://github.com/ayojs/ayo/pull/82 PR-URL: https://github.com/nodejs/node/pull/19377 Reviewed-By: Ben Noordhuis Reviewed-By: James M Snell --- src/env-inl.h | 23 +++++++++++++++++++++++ src/env.cc | 29 +++++++++++++++++++++++++++++ src/env.h | 31 +++++++++++++++++++++++++++++++ src/node.cc | 20 +++++++++++++++++++- src/node.h | 13 +++++++++++++ src/node_api.cc | 22 ++++++++++++++++++++++ src/node_api.h | 7 +++++++ 7 files changed, 144 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/env-inl.h b/src/env-inl.h index 6202e50548..d3c0c211d9 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -629,6 +629,29 @@ inline void Environment::SetTemplateMethod(v8::Local that, t->SetClassName(name_string); // NODE_SET_METHOD() compatibility. } +void Environment::AddCleanupHook(void (*fn)(void*), void* arg) { + auto insertion_info = cleanup_hooks_.emplace(CleanupHookCallback { + fn, arg, cleanup_hook_counter_++ + }); + // Make sure there was no existing element with these values. + CHECK_EQ(insertion_info.second, true); +} + +void Environment::RemoveCleanupHook(void (*fn)(void*), void* arg) { + CleanupHookCallback search { fn, arg, 0 }; + cleanup_hooks_.erase(search); +} + +size_t Environment::CleanupHookCallback::Hash::operator()( + const CleanupHookCallback& cb) const { + return std::hash()(cb.arg_); +} + +bool Environment::CleanupHookCallback::Equal::operator()( + const CleanupHookCallback& a, const CleanupHookCallback& b) const { + return a.fn_ == b.fn_ && a.arg_ == b.arg_; +} + #define VP(PropertyName, StringValue) V(v8::Private, PropertyName) #define VS(PropertyName, StringValue) V(v8::String, PropertyName) #define V(TypeName, PropertyName) \ diff --git a/src/env.cc b/src/env.cc index 08d719a510..aadb81092e 100644 --- a/src/env.cc +++ b/src/env.cc @@ -305,6 +305,35 @@ void Environment::PrintSyncTrace() const { fflush(stderr); } +void Environment::RunCleanup() { + while (!cleanup_hooks_.empty()) { + // Copy into a vector, since we can't sort an unordered_set in-place. + std::vector callbacks( + cleanup_hooks_.begin(), cleanup_hooks_.end()); + // We can't erase the copied elements from `cleanup_hooks_` yet, because we + // need to be able to check whether they were un-scheduled by another hook. + + std::sort(callbacks.begin(), callbacks.end(), + [](const CleanupHookCallback& a, const CleanupHookCallback& b) { + // Sort in descending order so that the most recently inserted callbacks + // are run first. + return a.insertion_order_counter_ > b.insertion_order_counter_; + }); + + for (const CleanupHookCallback& cb : callbacks) { + if (cleanup_hooks_.count(cb) == 0) { + // This hook was removed from the `cleanup_hooks_` set during another + // hook that was run earlier. Nothing to do here. + continue; + } + + cb.fn_(cb.arg_); + cleanup_hooks_.erase(cb); + CleanupHandles(); + } + } +} + void Environment::RunBeforeExitCallbacks() { for (ExitCallback before_exit : before_exit_functions_) { before_exit.cb_(before_exit.arg_); diff --git a/src/env.h b/src/env.h index c0d79883d0..3acb27c954 100644 --- a/src/env.h +++ b/src/env.h @@ -42,6 +42,7 @@ #include #include #include +#include struct nghttp2_rcbuf; @@ -775,6 +776,10 @@ class Environment { v8::Local GetNow(); + inline void AddCleanupHook(void (*fn)(void*), void* arg); + inline void RemoveCleanupHook(void (*fn)(void*), void* arg); + void RunCleanup(); + private: inline void CreateImmediate(native_immediate_callback cb, void* data, @@ -863,6 +868,32 @@ class Environment { void RunAndClearNativeImmediates(); static void CheckImmediate(uv_check_t* handle); + struct CleanupHookCallback { + void (*fn_)(void*); + void* arg_; + + // We keep track of the insertion order for these objects, so that we can + // call the callbacks in reverse order when we are cleaning up. + uint64_t insertion_order_counter_; + + // Only hashes `arg_`, since that is usually enough to identify the hook. + struct Hash { + inline size_t operator()(const CleanupHookCallback& cb) const; + }; + + // Compares by `fn_` and `arg_` being equal. + struct Equal { + inline bool operator()(const CleanupHookCallback& a, + const CleanupHookCallback& b) const; + }; + }; + + // Use an unordered_set, so that we have efficient insertion and removal. + std::unordered_set cleanup_hooks_; + uint64_t cleanup_hook_counter_ = 0; + static void EnvPromiseHook(v8::PromiseHookType type, v8::Local promise, v8::Local parent); diff --git a/src/node.cc b/src/node.cc index 5ddbf99150..c95e084f0c 100644 --- a/src/node.cc +++ b/src/node.cc @@ -904,6 +904,22 @@ void AddPromiseHook(v8::Isolate* isolate, promise_hook_func fn, void* arg) { env->AddPromiseHook(fn, arg); } +void AddEnvironmentCleanupHook(v8::Isolate* isolate, + void (*fun)(void* arg), + void* arg) { + Environment* env = Environment::GetCurrent(isolate); + env->AddCleanupHook(fun, arg); +} + + +void RemoveEnvironmentCleanupHook(v8::Isolate* isolate, + void (*fun)(void* arg), + void* arg) { + Environment* env = Environment::GetCurrent(isolate); + env->RemoveCleanupHook(fun, arg); +} + + CallbackScope::CallbackScope(Isolate* isolate, Local object, async_context asyncContext) @@ -4435,7 +4451,7 @@ Environment* CreateEnvironment(IsolateData* isolate_data, void FreeEnvironment(Environment* env) { - env->CleanupHandles(); + env->RunCleanup(); delete env; } @@ -4533,6 +4549,8 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data, env.set_trace_sync_io(false); const int exit_code = EmitExit(&env); + + env.RunCleanup(); RunAtExit(&env); v8_platform.DrainVMTasks(isolate); diff --git a/src/node.h b/src/node.h index 5a491c1abf..23e2e9995c 100644 --- a/src/node.h +++ b/src/node.h @@ -583,6 +583,19 @@ NODE_EXTERN void AddPromiseHook(v8::Isolate* isolate, promise_hook_func fn, void* arg); +/* This is a lot like node::AtExit, except that the hooks added via this + * function are run before the AtExit ones and will always be registered + * for the current Environment instance. + * These functions are safe to use in an addon supporting multiple + * threads/isolates. */ +NODE_EXTERN void AddEnvironmentCleanupHook(v8::Isolate* isolate, + void (*fun)(void* arg), + void* arg); + +NODE_EXTERN void RemoveEnvironmentCleanupHook(v8::Isolate* isolate, + void (*fun)(void* arg), + void* arg); + /* Returns the id of the current execution context. If the return value is * zero then no execution has been set. This will happen if the user handles * I/O from native code. */ diff --git a/src/node_api.cc b/src/node_api.cc index 4878cf241e..d5437d70d9 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -902,6 +902,28 @@ void napi_module_register(napi_module* mod) { node::node_module_register(nm); } +napi_status napi_add_env_cleanup_hook(napi_env env, + void (*fun)(void* arg), + void* arg) { + CHECK_ENV(env); + CHECK_ARG(env, fun); + + node::AddEnvironmentCleanupHook(env->isolate, fun, arg); + + return napi_ok; +} + +napi_status napi_remove_env_cleanup_hook(napi_env env, + void (*fun)(void* arg), + void* arg) { + CHECK_ENV(env); + CHECK_ARG(env, fun); + + node::RemoveEnvironmentCleanupHook(env->isolate, fun, arg); + + return napi_ok; +} + // Warning: Keep in-sync with napi_status enum static const char* error_messages[] = {nullptr, diff --git a/src/node_api.h b/src/node_api.h index b010d32db7..91c2775a03 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -118,6 +118,13 @@ EXTERN_C_START NAPI_EXTERN void napi_module_register(napi_module* mod); +NAPI_EXTERN napi_status napi_add_env_cleanup_hook(napi_env env, + void (*fun)(void* arg), + void* arg); +NAPI_EXTERN napi_status napi_remove_env_cleanup_hook(napi_env env, + void (*fun)(void* arg), + void* arg); + NAPI_EXTERN napi_status napi_get_last_error_info(napi_env env, const napi_extended_error_info** result); -- cgit v1.2.3