n-api: fix use-after-free with napi_remove_async_cleanup_hook

src/callback_queue-inl.h

@@ -22,8 +22,8 @@ CallbackQueue<R, Args...>::Shift() {
     head_ = ret->get_next();
     if (!head_)
       tail_ = nullptr;  // The queue is now empty.
+    size_--;
   }
-  size_--;
   return ret;
 }
 

src/env.cc

@@ -634,7 +634,10 @@ void Environment::RunCleanup() {
   initial_base_object_count_ = 0;
   CleanupHandles();
 
-  while (!cleanup_hooks_.empty()) {
+  while (!cleanup_hooks_.empty() ||
+         native_immediates_.size() > 0 ||
+         native_immediates_threadsafe_.size() > 0 ||
+         native_immediates_interrupts_.size() > 0) {
     // Copy into a vector, since we can't sort an unordered_set in-place.
     std::vector<CleanupHookCallback> callbacks(
         cleanup_hooks_.begin(), cleanup_hooks_.end());

src/node_api.cc

@@ -533,6 +533,7 @@ napi_status napi_add_async_cleanup_hook(
   auto handle = node::AddEnvironmentCleanupHook(env->isolate, fun, arg);
   if (remove_handle != nullptr) {
     *remove_handle = new napi_async_cleanup_hook_handle__ { std::move(handle) };
+    env->Ref();
   }
 
   return napi_clear_last_error(env);
@@ -547,6 +548,11 @@ napi_status napi_remove_async_cleanup_hook(
   node::RemoveEnvironmentCleanupHook(std::move(remove_handle->handle));
   delete remove_handle;
 
+  // Release the `env` handle asynchronously since it would be surprising if
+  // a call to a N-API function would destroy `env` synchronously.
+  static_cast<node_napi_env>(env)->node_env()
+      ->SetImmediate([env](node::Environment*) { env->Unref(); });
+
   return napi_clear_last_error(env);
 }