From beb45acb3927a74413ba717d49e84a8c5bc0905d Mon Sep 17 00:00:00 2001
From: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Date: Sun, 12 Nov 2017 18:25:18 +0100
Subject: [PATCH] src: use unique_ptr for scheduled delayed tasks

Use std::unique_ptr for delayed tasks in the scheduled
delayed tasks vector. This makes it clear that the vector
has ownership of the delayed tasks and is responsible for
deleting them.

Use a custom deleter for the pointers because libuv
needs to close the handle and then delete the data. Provide
the handle when creating the pointer instead of invoking the
special delete action everytime an element is removed from the vector.

Backport-PR-URL: https://github.com/nodejs/node/pull/20901
PR-URL: https://github.com/nodejs/node/pull/17083
Reviewed-By: Anna Henningsen <anna@addaleax.net>
---
 src/node_platform.cc | 34 +++++++++++++++++++---------------
 src/node_platform.h  |  9 ++++++++-
 2 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/src/node_platform.cc b/src/node_platform.cc
index 51927cf7f9f792..0c50e8468d0f44 100644
--- a/src/node_platform.cc
+++ b/src/node_platform.cc
@@ -134,26 +134,23 @@ void PerIsolatePlatformData::RunForegroundTask(std::unique_ptr<Task> task) {
   task->Run();
 }
 
+void PerIsolatePlatformData::DeleteFromScheduledTasks(DelayedTask* task) {
+  auto it = std::find_if(scheduled_delayed_tasks_.begin(),
+                         scheduled_delayed_tasks_.end(),
+                         [task](const DelayedTaskPointer& delayed) -> bool {
+          return delayed.get() == task;
+      });
+  CHECK_NE(it, scheduled_delayed_tasks_.end());
+  scheduled_delayed_tasks_.erase(it);
+}
+
 void PerIsolatePlatformData::RunForegroundTask(uv_timer_t* handle) {
   DelayedTask* delayed = static_cast<DelayedTask*>(handle->data);
-  auto& tasklist = delayed->platform_data->scheduled_delayed_tasks_;
-  auto it = std::find(tasklist.begin(), tasklist.end(), delayed);
-  CHECK_NE(it, tasklist.end());
-  tasklist.erase(it);
   RunForegroundTask(std::move(delayed->task));
-  uv_close(reinterpret_cast<uv_handle_t*>(&delayed->timer),
-           [](uv_handle_t* handle) {
-    delete static_cast<DelayedTask*>(handle->data);
-  });
+  delayed->platform_data->DeleteFromScheduledTasks(delayed);
 }
 
 void PerIsolatePlatformData::CancelPendingDelayedTasks() {
-  for (auto delayed : scheduled_delayed_tasks_) {
-    uv_close(reinterpret_cast<uv_handle_t*>(&delayed->timer),
-             [](uv_handle_t* handle) {
-      delete static_cast<DelayedTask*>(handle->data);
-    });
-  }
   scheduled_delayed_tasks_.clear();
 }
 
@@ -183,7 +180,14 @@ bool PerIsolatePlatformData::FlushForegroundTasksInternal() {
     // the delay is non-zero. This should not be a problem in practice.
     uv_timer_start(&delayed->timer, RunForegroundTask, delay_millis, 0);
     uv_unref(reinterpret_cast<uv_handle_t*>(&delayed->timer));
-    scheduled_delayed_tasks_.push_back(delayed.release());
+
+    scheduled_delayed_tasks_.emplace_back(delayed.release(),
+                                          [](DelayedTask* delayed) {
+      uv_close(reinterpret_cast<uv_handle_t*>(&delayed->timer),
+               [](uv_handle_t* handle) {
+        delete static_cast<DelayedTask*>(handle->data);
+      });
+    });
   }
   while (std::unique_ptr<Task> task = foreground_tasks_.Pop()) {
     did_work = true;
diff --git a/src/node_platform.h b/src/node_platform.h
index 584506ae1f114a..48301a05a11d8c 100644
--- a/src/node_platform.h
+++ b/src/node_platform.h
@@ -4,6 +4,7 @@
 #include <queue>
 #include <unordered_map>
 #include <vector>
+#include <functional>
 
 #include "libplatform/libplatform.h"
 #include "node.h"
@@ -64,6 +65,8 @@ class PerIsolatePlatformData {
   void CancelPendingDelayedTasks();
 
  private:
+  void DeleteFromScheduledTasks(DelayedTask* task);
+
   static void FlushTasks(uv_async_t* handle);
   static void RunForegroundTask(std::unique_ptr<v8::Task> task);
   static void RunForegroundTask(uv_timer_t* timer);
@@ -74,7 +77,11 @@ class PerIsolatePlatformData {
   uv_async_t* flush_tasks_ = nullptr;
   TaskQueue<v8::Task> foreground_tasks_;
   TaskQueue<DelayedTask> foreground_delayed_tasks_;
-  std::vector<DelayedTask*> scheduled_delayed_tasks_;
+
+  // Use a custom deleter because libuv needs to close the handle first.
+  typedef std::unique_ptr<DelayedTask, std::function<void(DelayedTask*)>>
+      DelayedTaskPointer;
+  std::vector<DelayedTaskPointer> scheduled_delayed_tasks_;
 };
 
 class NodePlatform : public MultiIsolatePlatform {