From db492c3178aee7edab23d9ade20155fe11c818d9 Mon Sep 17 00:00:00 2001 From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com> Date: Mon, 14 Sep 2020 10:24:40 -0600 Subject: [PATCH] fix: provide asynchronous cleanup hooks in n-api (#25281) Co-authored-by: Shelley Vohr --- patches/node/.patches | 1 + ...c_provide_asynchronous_cleanup_hooks.patch | 250 ++++++++++++++++++ 2 files changed, 251 insertions(+) create mode 100644 patches/node/n-api_src_provide_asynchronous_cleanup_hooks.patch diff --git a/patches/node/.patches b/patches/node/.patches index 222ed19a38f4d..bad678759d52c 100644 --- a/patches/node/.patches +++ b/patches/node/.patches @@ -43,3 +43,4 @@ fix_allow_preventing_setpromiserejectcallback.patch lib_use_non-symbols_in_isurlinstance_check.patch feat_add_implementation_of_v8_platform_postjob.patch fix_enable_tls_renegotiation.patch +n-api_src_provide_asynchronous_cleanup_hooks.patch diff --git a/patches/node/n-api_src_provide_asynchronous_cleanup_hooks.patch b/patches/node/n-api_src_provide_asynchronous_cleanup_hooks.patch new file mode 100644 index 0000000000000..bcc9fe38f1a42 --- /dev/null +++ b/patches/node/n-api_src_provide_asynchronous_cleanup_hooks.patch @@ -0,0 +1,250 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Shelley Vohr +Date: Tue, 25 Aug 2020 19:34:12 -0700 +Subject: n-api,src: provide asynchronous cleanup hooks + +Backports https://github.com/nodejs/node/pull/34572 and +https://github.com/nodejs/node/pull/34819. + +Sometimes addons need to perform cleanup actions, for example +closing libuv handles or waiting for requests to finish, that +cannot be performed synchronously. + +Add C++ API and N-API functions that allow providing such +asynchronous cleanup hooks. + +This patch can be removed when Electron upgrades to a version of Node.js +which contains the above referenced commit(s). + +diff --git a/src/api/hooks.cc b/src/api/hooks.cc +index 037bdda6f41c825dd112b0cc9fca0ebde47c6163..3b16c0350d8a8494202144407664af41d338fe04 100644 +--- a/src/api/hooks.cc ++++ b/src/api/hooks.cc +@@ -73,8 +73,35 @@ int EmitExit(Environment* env) { + .ToChecked(); + } + ++typedef void (*CleanupHook)(void* arg); ++typedef void (*AsyncCleanupHook)(void* arg, void(*)(void*), void*); ++ ++struct AsyncCleanupHookInfo final { ++ Environment* env; ++ AsyncCleanupHook fun; ++ void* arg; ++ bool started = false; ++ // Use a self-reference to make sure the storage is kept alive while the ++ // cleanup hook is registered but not yet finished. ++ std::shared_ptr self; ++}; ++ ++// Opaque type that is basically an alias for `shared_ptr` ++// (but not publicly so for easier ABI/API changes). In particular, ++// std::shared_ptr does not generally maintain a consistent ABI even on a ++// specific platform. ++struct ACHHandle final { ++ std::shared_ptr info; ++}; ++// This is implemented as an operator on a struct because otherwise you can't ++// default-initialize AsyncCleanupHookHandle, because in C++ for a ++// std::unique_ptr to be default-initializable the deleter type also needs ++// to be default-initializable; in particular, function types don't satisfy ++// this. ++void DeleteACHHandle::operator ()(ACHHandle* handle) const { delete handle; } ++ + void AddEnvironmentCleanupHook(Isolate* isolate, +- void (*fun)(void* arg), ++ CleanupHook fun, + void* arg) { + Environment* env = Environment::GetCurrent(isolate); + CHECK_NOT_NULL(env); +@@ -82,13 +109,50 @@ void AddEnvironmentCleanupHook(Isolate* isolate, + } + + void RemoveEnvironmentCleanupHook(Isolate* isolate, +- void (*fun)(void* arg), ++ CleanupHook fun, + void* arg) { + Environment* env = Environment::GetCurrent(isolate); + CHECK_NOT_NULL(env); + env->RemoveCleanupHook(fun, arg); + } + ++static void FinishAsyncCleanupHook(void* arg) { ++ AsyncCleanupHookInfo* info = static_cast(arg); ++ std::shared_ptr keep_alive = info->self; ++ ++ info->env->DecreaseWaitingRequestCounter(); ++ info->self.reset(); ++} ++ ++static void RunAsyncCleanupHook(void* arg) { ++ AsyncCleanupHookInfo* info = static_cast(arg); ++ info->env->IncreaseWaitingRequestCounter(); ++ info->started = true; ++ info->fun(info->arg, FinishAsyncCleanupHook, info); ++} ++ ++AsyncCleanupHookHandle AddEnvironmentCleanupHook( ++ Isolate* isolate, ++ AsyncCleanupHook fun, ++ void* arg) { ++ Environment* env = Environment::GetCurrent(isolate); ++ CHECK_NOT_NULL(env); ++ auto info = std::make_shared(); ++ info->env = env; ++ info->fun = fun; ++ info->arg = arg; ++ info->self = info; ++ env->AddCleanupHook(RunAsyncCleanupHook, info.get()); ++ return AsyncCleanupHookHandle(new ACHHandle { info }); ++} ++ ++void RemoveEnvironmentCleanupHook( ++ AsyncCleanupHookHandle handle) { ++ if (handle->info->started) return; ++ handle->info->self.reset(); ++ handle->info->env->RemoveCleanupHook(RunAsyncCleanupHook, handle->info.get()); ++} ++ + async_id AsyncHooksGetExecutionAsyncId(Isolate* isolate) { + Environment* env = Environment::GetCurrent(isolate); + if (env == nullptr) return -1; +diff --git a/src/node.h b/src/node.h +index 60ecc3bd3499c23b297bc11e7f052aede20520c2..4c4e55e338d7b42c36818a45f6b41170c495adde 100644 +--- a/src/node.h ++++ b/src/node.h +@@ -739,6 +739,20 @@ NODE_EXTERN void RemoveEnvironmentCleanupHook(v8::Isolate* isolate, + void (*fun)(void* arg), + void* arg); + ++/* These are async equivalents of the above. After the cleanup hook is invoked, ++ * `cb(cbarg)` *must* be called, and attempting to remove the cleanup hook will ++ * have no effect. */ ++struct ACHHandle; ++struct NODE_EXTERN DeleteACHHandle { void operator()(ACHHandle*) const; }; ++typedef std::unique_ptr AsyncCleanupHookHandle; ++ ++NODE_EXTERN AsyncCleanupHookHandle AddEnvironmentCleanupHook( ++ v8::Isolate* isolate, ++ void (*fun)(void* arg, void (*cb)(void*), void* cbarg), ++ void* arg); ++ ++NODE_EXTERN void RemoveEnvironmentCleanupHook(AsyncCleanupHookHandle holder); ++ + /* 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 fe24eca1b8e2d81fbafd0a1e2da38d957fbaa1c1..66168bd2c28ce6481516e63734616f487e3ec3e1 100644 +--- a/src/node_api.cc ++++ b/src/node_api.cc +@@ -507,6 +507,70 @@ napi_status napi_remove_env_cleanup_hook(napi_env env, + return napi_ok; + } + ++struct napi_async_cleanup_hook_handle__ { ++ napi_async_cleanup_hook_handle__(napi_env env, ++ napi_async_cleanup_hook user_hook, ++ void* user_data): ++ env_(env), ++ user_hook_(user_hook), ++ user_data_(user_data) { ++ handle_ = node::AddEnvironmentCleanupHook(env->isolate, Hook, this); ++ env->Ref(); ++ } ++ ++ ~napi_async_cleanup_hook_handle__() { ++ node::RemoveEnvironmentCleanupHook(std::move(handle_)); ++ if (done_cb_ != nullptr) ++ done_cb_(done_data_); ++ ++ // Release the `env` handle asynchronously since it would be surprising if ++ // a call to a N-API function would destroy `env` synchronously. ++ static_cast(env_)->node_env() ++ ->SetImmediate([env = env_](node::Environment*) { env->Unref(); }); ++ } ++ ++ static void Hook(void* data, void (*done_cb)(void*), void* done_data) { ++ auto handle = static_cast(data); ++ handle->done_cb_ = done_cb; ++ handle->done_data_ = done_data; ++ handle->user_hook_(handle, handle->user_data_); ++ } ++ ++ node::AsyncCleanupHookHandle handle_; ++ napi_env env_ = nullptr; ++ napi_async_cleanup_hook user_hook_ = nullptr; ++ void* user_data_ = nullptr; ++ void (*done_cb_)(void*) = nullptr; ++ void* done_data_ = nullptr; ++}; ++ ++napi_status napi_add_async_cleanup_hook( ++ napi_env env, ++ napi_async_cleanup_hook hook, ++ void* arg, ++ napi_async_cleanup_hook_handle* remove_handle) { ++ CHECK_ENV(env); ++ CHECK_ARG(env, hook); ++ ++ napi_async_cleanup_hook_handle__* handle = ++ new napi_async_cleanup_hook_handle__(env, hook, arg); ++ ++ if (remove_handle != nullptr) ++ *remove_handle = handle; ++ ++ return napi_clear_last_error(env); ++} ++ ++napi_status napi_remove_async_cleanup_hook( ++ napi_async_cleanup_hook_handle remove_handle) { ++ ++ if (remove_handle == nullptr) ++ return napi_invalid_arg; ++ ++ delete remove_handle; ++ return napi_ok; ++} ++ + napi_status napi_fatal_exception(napi_env env, napi_value err) { + NAPI_PREAMBLE(env); + CHECK_ARG(env, err); +diff --git a/src/node_api.h b/src/node_api.h +index 2f1b45572d8130f27492eb6188c1aa611f2e01a3..577a1dcd94987202819e7a36a2d9674f13d13614 100644 +--- a/src/node_api.h ++++ b/src/node_api.h +@@ -250,6 +250,19 @@ napi_ref_threadsafe_function(napi_env env, napi_threadsafe_function func); + + #endif // NAPI_VERSION >= 4 + ++#ifdef NAPI_EXPERIMENTAL ++ ++NAPI_EXTERN napi_status napi_add_async_cleanup_hook( ++ napi_env env, ++ napi_async_cleanup_hook hook, ++ void* arg, ++ napi_async_cleanup_hook_handle* remove_handle); ++ ++NAPI_EXTERN napi_status napi_remove_async_cleanup_hook( ++ napi_async_cleanup_hook_handle remove_handle); ++ ++#endif // NAPI_EXPERIMENTAL ++ + EXTERN_C_END + + #endif // SRC_NODE_API_H_ +diff --git a/src/node_api_types.h b/src/node_api_types.h +index 1c9a2b8aa21889c0d29fb02b234ae9698d122c2c..0e400e9676df5ba09d350fe7a2a70a1dc9e4d3d6 100644 +--- a/src/node_api_types.h ++++ b/src/node_api_types.h +@@ -41,4 +41,10 @@ typedef struct { + const char* release; + } napi_node_version; + ++#ifdef NAPI_EXPERIMENTAL ++typedef struct napi_async_cleanup_hook_handle__* napi_async_cleanup_hook_handle; ++typedef void (*napi_async_cleanup_hook)(napi_async_cleanup_hook_handle handle, ++ void* data); ++#endif // NAPI_EXPERIMENTAL ++ + #endif // SRC_NODE_API_TYPES_H_