src: simplify Environment::HandleCleanup

- Make the HandleCleanup a struct, and make the queue a std::list, iterate over it in CleanupHandles() and clear it after that. - Put the handle cleanup registration into a method and document that they will not be called in the one environemt per process setup. PR-URL: #19319 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Jackson Tian <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Mar 15, 2018 · a1a409a · a1a409a
1 parent 855dabd
commit a1a409a
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 51 deletions.
diff --git a/src/env-inl.h b/src/env-inl.h
@@ -341,7 +341,7 @@ inline uv_idle_t* Environment::immediate_idle_handle() {
 inline void Environment::RegisterHandleCleanup(uv_handle_t* handle,
                                                HandleCleanupCb cb,
                                                void *arg) {
-  handle_cleanup_queue_.PushBack(new HandleCleanup(handle, cb, arg));
+  handle_cleanup_queue_.push_back(HandleCleanup{handle, cb, arg});
 }
 
 inline void Environment::FinishHandleCleanup(uv_handle_t* handle) {

diff --git a/src/env.cc b/src/env.cc
@@ -175,7 +175,34 @@ void Environment::Start(int argc,
   uv_unref(reinterpret_cast<uv_handle_t*>(&idle_prepare_handle_));
   uv_unref(reinterpret_cast<uv_handle_t*>(&idle_check_handle_));
 
-  auto close_and_finish = [](Environment* env, uv_handle_t* handle, void* arg) {
+  // Register clean-up cb to be called to clean up the handles
+  // when the environment is freed, note that they are not cleaned in
+  // the one environment per process setup, but will be called in
+  // FreeEnvironment.
+  RegisterHandleCleanups();
+
+  if (start_profiler_idle_notifier) {
+    StartProfilerIdleNotifier();
+  }
+
+  auto process_template = FunctionTemplate::New(isolate());
+  process_template->SetClassName(FIXED_ONE_BYTE_STRING(isolate(), "process"));
+
+  auto process_object =
+      process_template->GetFunction()->NewInstance(context()).ToLocalChecked();
+  set_process_object(process_object);
+
+  SetupProcessObject(this, argc, argv, exec_argc, exec_argv);
+  LoadAsyncWrapperInfo(this);
+
+  static uv_once_t init_once = UV_ONCE_INIT;
+  uv_once(&init_once, InitThreadLocalOnce);
+  uv_key_set(&thread_local_env, this);
+}
+
+void Environment::RegisterHandleCleanups() {
+  HandleCleanupCb close_and_finish = [](Environment* env, uv_handle_t* handle,
+                                        void* arg) {
     handle->data = env;
 
     uv_close(handle, [](uv_handle_t* handle) {
@@ -199,32 +226,14 @@ void Environment::Start(int argc,
       reinterpret_cast<uv_handle_t*>(&idle_check_handle_),
       close_and_finish,
       nullptr);
-
-  if (start_profiler_idle_notifier) {
-    StartProfilerIdleNotifier();
-  }
-
-  auto process_template = FunctionTemplate::New(isolate());
-  process_template->SetClassName(FIXED_ONE_BYTE_STRING(isolate(), "process"));
-
-  auto process_object =
-      process_template->GetFunction()->NewInstance(context()).ToLocalChecked();
-  set_process_object(process_object);
-
-  SetupProcessObject(this, argc, argv, exec_argc, exec_argv);
-  LoadAsyncWrapperInfo(this);
-
-  static uv_once_t init_once = UV_ONCE_INIT;
-  uv_once(&init_once, InitThreadLocalOnce);
-  uv_key_set(&thread_local_env, this);
 }
 
 void Environment::CleanupHandles() {
-  while (HandleCleanup* hc = handle_cleanup_queue_.PopFront()) {
+  for (HandleCleanup& hc : handle_cleanup_queue_) {
     handle_cleanup_waiting_++;
-    hc->cb_(this, hc->handle_, hc->arg_);
-    delete hc;
+    hc.cb_(this, hc.handle_, hc.arg_);
   }
+  handle_cleanup_queue_.clear();
 
   while (handle_cleanup_waiting_ != 0)
     uv_run(event_loop(), UV_RUN_ONCE);

diff --git a/src/env.h b/src/env.h
@@ -540,26 +540,6 @@ class Environment {
     DISALLOW_COPY_AND_ASSIGN(TickInfo);
   };
 
-  typedef void (*HandleCleanupCb)(Environment* env,
-                                  uv_handle_t* handle,
-                                  void* arg);
-
-  class HandleCleanup {
-   private:
-    friend class Environment;
-
-    HandleCleanup(uv_handle_t* handle, HandleCleanupCb cb, void* arg)
-        : handle_(handle),
-          cb_(cb),
-          arg_(arg) {
-    }
-
-    uv_handle_t* handle_;
-    HandleCleanupCb cb_;
-    void* arg_;
-    ListNode<HandleCleanup> handle_cleanup_queue_;
-  };
-
   static inline Environment* GetCurrent(v8::Isolate* isolate);
   static inline Environment* GetCurrent(v8::Local<v8::Context> context);
   static inline Environment* GetCurrent(
@@ -580,7 +560,22 @@ class Environment {
              int exec_argc,
              const char* const* exec_argv,
              bool start_profiler_idle_notifier);
+
+  typedef void (*HandleCleanupCb)(Environment* env,
+                                  uv_handle_t* handle,
+                                  void* arg);
+  struct HandleCleanup {
+    uv_handle_t* handle_;
+    HandleCleanupCb cb_;
+    void* arg_;
+  };
+
+  void RegisterHandleCleanups();
   void CleanupHandles();
+  inline void RegisterHandleCleanup(uv_handle_t* handle,
+                                    HandleCleanupCb cb,
+                                    void *arg);
+  inline void FinishHandleCleanup(uv_handle_t* handle);
 
   inline void AssignToContext(v8::Local<v8::Context> context,
                               const ContextInfo& info);
@@ -596,12 +591,6 @@ class Environment {
   inline uv_check_t* immediate_check_handle();
   inline uv_idle_t* immediate_idle_handle();
 
-  // Register clean-up cb to be called on environment destruction.
-  inline void RegisterHandleCleanup(uv_handle_t* handle,
-                                    HandleCleanupCb cb,
-                                    void *arg);
-  inline void FinishHandleCleanup(uv_handle_t* handle);
-
   inline AsyncHooks* async_hooks();
   inline ImmediateInfo* immediate_info();
   inline TickInfo* tick_info();
@@ -822,8 +811,7 @@ class Environment {
   friend int GenDebugSymbols();
   HandleWrapQueue handle_wrap_queue_;
   ReqWrapQueue req_wrap_queue_;
-  ListHead<HandleCleanup,
-           &HandleCleanup::handle_cleanup_queue_> handle_cleanup_queue_;
+  std::list<HandleCleanup> handle_cleanup_queue_;
   int handle_cleanup_waiting_;
 
   double* heap_statistics_buffer_ = nullptr;