src: add Cleanable class to Environment

We store a linked list of `Cleanable` objects on the
`node::Environment` and invoke their `Clean()` method during env
teardown. `Cleanable`s can be added/removed using
`Environment::AddCleanable()` resp. `Environment::RemoveCleanable()`.

This eliminates the need for adding many cleanup hooks.

Author: gabrielschulhof

src/env-inl.h

@@ -148,6 +148,31 @@ inline void Environment::PopAsyncCallbackScope() {
   async_callback_scope_depth_--;
 }
 
+inline void Environment::AddCleanable(Cleanable* new_cleanable) {
+  CHECK_NOT_NULL(new_cleanable);
+  new_cleanable->next_ = cleanables_;
+  new_cleanable->prev_ = nullptr;
+  if (cleanables_ != nullptr) {
+    cleanables_->prev_ = new_cleanable;
+  }
+  cleanables_ = new_cleanable;
+}
+
+inline void Environment::RemoveCleanable(Cleanable* old_cleanable) {
+  CHECK_NOT_NULL(old_cleanable);
+  if (old_cleanable->prev_ != nullptr) {
+    old_cleanable->prev_->next_ = old_cleanable->next_;
+  }
+  if (old_cleanable->next_ != nullptr) {
+    old_cleanable->next_->prev_ = old_cleanable->prev_;
+  }
+  if (cleanables_ == old_cleanable) {
+    cleanables_ = old_cleanable->next_;
+  }
+  old_cleanable->next_ = nullptr;
+  old_cleanable->prev_ = nullptr;
+}
+
 inline AliasedUint32Array& ImmediateInfo::fields() {
   return fields_;
 }

src/env.cc

@@ -804,6 +804,7 @@ Environment::Environment(IsolateData* isolate_data,
       timeout_info_(isolate_, 1, MAYBE_FIELD_PTR(env_info, timeout_info)),
       tick_info_(isolate, MAYBE_FIELD_PTR(env_info, tick_info)),
       timer_base_(uv_now(isolate_data->event_loop())),
+      cleanables_(nullptr),
       exec_argv_(exec_args),
       argv_(args),
       exec_path_(Environment::GetExecPath(args)),
@@ -1279,6 +1280,12 @@ void Environment::RunCleanup() {
   // Defer the BaseObject cleanup after handles are cleaned up.
   CleanupHandles();
 
+  while (cleanables_ != nullptr) {
+    Cleanable* cleanable = cleanables_;
+    RemoveCleanable(cleanable);
+    cleanable->Clean();
+  }
+
   while (!cleanup_queue_.empty() || principal_realm_->HasCleanupHooks() ||
          native_immediates_.size() > 0 ||
          native_immediates_threadsafe_.size() > 0 ||

src/env.h

@@ -594,6 +594,17 @@ void DefaultProcessExitHandlerInternal(Environment* env, ExitCode exit_code);
 v8::Maybe<ExitCode> SpinEventLoopInternal(Environment* env);
 v8::Maybe<ExitCode> EmitProcessExitInternal(Environment* env);
 
+class Cleanable {
+  friend class Environment;
+
+ public:
+  virtual void Clean() = 0;
+
+ private:
+  Cleanable* next_ = nullptr;
+  Cleanable* prev_ = nullptr;
+};
+
 /**
  * Environment is a per-isolate data structure that represents an execution
  * environment. Each environment has a principal realm. An environment can
@@ -611,6 +622,9 @@ class Environment final : public MemoryRetainer {
   static std::string GetExecPath(const std::vector<std::string>& argv);
   static std::string GetCwd(const std::string& exec_path);
 
+  void AddCleanable(Cleanable* new_cleanable);
+  void RemoveCleanable(Cleanable* old_cleanable);
+
   inline size_t SelfSize() const override;
   bool IsRootNode() const override { return true; }
   void MemoryInfo(MemoryTracker* tracker) const override;
@@ -1080,6 +1094,7 @@ class Environment final : public MemoryRetainer {
   void TrackContext(v8::Local<v8::Context> context);
   void UntrackContext(v8::Local<v8::Context> context);
 
+  Cleanable* cleanables_;
   std::list<binding::DLib> loaded_addons_;
   v8::Isolate* const isolate_;
   IsolateData* const isolate_data_;

src/node_buffer.cc

@@ -84,7 +84,7 @@ using v8::Value;
 
 namespace {
 
-class CallbackInfo {
+class CallbackInfo : public Cleanable {
  public:
   static inline Local<ArrayBuffer> CreateTrackedArrayBuffer(
       Environment* env,
@@ -95,9 +95,9 @@ class CallbackInfo {
 
   CallbackInfo(const CallbackInfo&) = delete;
   CallbackInfo& operator=(const CallbackInfo&) = delete;
+  void Clean();
 
  private:
-  static void CleanupHook(void* data);
   inline void OnBackingStoreFree();
   inline void CallAndResetCallback();
   inline CallbackInfo(Environment* env,
@@ -112,7 +112,6 @@ class CallbackInfo {
   Environment* const env_;
 };
 
-
 Local<ArrayBuffer> CallbackInfo::CreateTrackedArrayBuffer(
     Environment* env,
     char* data,
@@ -152,25 +151,23 @@ CallbackInfo::CallbackInfo(Environment* env,
       data_(data),
       hint_(hint),
       env_(env) {
-  env->AddCleanupHook(CleanupHook, this);
+  env->AddCleanable(this);
   env->isolate()->AdjustAmountOfExternalAllocatedMemory(sizeof(*this));
 }
 
-void CallbackInfo::CleanupHook(void* data) {
-  CallbackInfo* self = static_cast<CallbackInfo*>(data);
-
+void CallbackInfo::Clean() {
   {
-    HandleScope handle_scope(self->env_->isolate());
-    Local<ArrayBuffer> ab = self->persistent_.Get(self->env_->isolate());
+    HandleScope handle_scope(env_->isolate());
+    Local<ArrayBuffer> ab = persistent_.Get(env_->isolate());
     if (!ab.IsEmpty() && ab->IsDetachable()) {
       ab->Detach(Local<Value>()).Check();
-      self->persistent_.Reset();
+      persistent_.Reset();
     }
   }
 
   // Call the callback in this case, but don't delete `this` yet because the
   // BackingStore deleter callback will do so later.
-  self->CallAndResetCallback();
+  CallAndResetCallback();
 }
 
 void CallbackInfo::CallAndResetCallback() {
@@ -182,7 +179,7 @@ void CallbackInfo::CallAndResetCallback() {
   }
   if (callback != nullptr) {
     // Clean up all Environment-related state and run the callback.
-    env_->RemoveCleanupHook(CleanupHook, this);
+    env_->RemoveCleanable(this);
     int64_t change_in_bytes = -static_cast<int64_t>(sizeof(*this));
     env_->isolate()->AdjustAmountOfExternalAllocatedMemory(change_in_bytes);