src: use unique_ptr in platform implementation

Replace raw pointers in task queues with std::unique_ptr. This makes ownership obvious. Backport-PR-URL: #20901 PR-URL: #16970 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>
nodejs · Aug 16, 2018 · 2148b19 · 2148b19
1 parent e932754
commit 2148b19
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 42 deletions.
diff --git a/src/node_platform.cc b/src/node_platform.cc
@@ -18,9 +18,8 @@ using v8::TracingController;
 
 static void BackgroundRunner(void* data) {
   TaskQueue<Task>* background_tasks = static_cast<TaskQueue<Task>*>(data);
-  while (Task* task = background_tasks->BlockingPop()) {
+  while (std::unique_ptr<Task> task = background_tasks->BlockingPop()) {
     task->Run();
-    delete task;
     background_tasks->NotifyOfCompletion();
   }
 }
@@ -39,18 +38,19 @@ void PerIsolatePlatformData::FlushTasks(uv_async_t* handle) {
   platform_data->FlushForegroundTasksInternal();
 }
 
-void PerIsolatePlatformData::CallOnForegroundThread(Task* task) {
-  foreground_tasks_.Push(task);
+void PerIsolatePlatformData::CallOnForegroundThread(
+    std::unique_ptr<Task> task) {
+  foreground_tasks_.Push(std::move(task));
   uv_async_send(flush_tasks_);
 }
 
 void PerIsolatePlatformData::CallDelayedOnForegroundThread(
-    Task* task, double delay_in_seconds) {
-  auto delayed = new DelayedTask();
-  delayed->task = task;
+  std::unique_ptr<Task> task, double delay_in_seconds) {
+  std::unique_ptr<DelayedTask> delayed(new DelayedTask());
+  delayed->task = std::move(task);
   delayed->platform_data = this;
   delayed->timeout = delay_in_seconds;
-  foreground_delayed_tasks_.Push(delayed);
+  foreground_delayed_tasks_.Push(std::move(delayed));
   uv_async_send(flush_tasks_);
 }
 
@@ -125,14 +125,13 @@ size_t NodePlatform::NumberOfAvailableBackgroundThreads() {
   return threads_.size();
 }
 
-void PerIsolatePlatformData::RunForegroundTask(Task* task) {
+void PerIsolatePlatformData::RunForegroundTask(std::unique_ptr<Task> task) {
   Isolate* isolate = Isolate::GetCurrent();
   HandleScope scope(isolate);
   Environment* env = Environment::GetCurrent(isolate);
   InternalCallbackScope cb_scope(env, Local<Object>(), { 0, 0 },
                                  InternalCallbackScope::kAllowEmptyResource);
   task->Run();
-  delete task;
 }
 
 void PerIsolatePlatformData::RunForegroundTask(uv_timer_t* handle) {
@@ -141,7 +140,7 @@ void PerIsolatePlatformData::RunForegroundTask(uv_timer_t* handle) {
   auto it = std::find(tasklist.begin(), tasklist.end(), delayed);
   CHECK_NE(it, tasklist.end());
   tasklist.erase(it);
-  RunForegroundTask(delayed->task);
+  RunForegroundTask(std::move(delayed->task));
   uv_close(reinterpret_cast<uv_handle_t*>(&delayed->timer),
            [](uv_handle_t* handle) {
     delete static_cast<DelayedTask*>(handle->data);
@@ -162,39 +161,40 @@ void NodePlatform::DrainBackgroundTasks(Isolate* isolate) {
   PerIsolatePlatformData* per_isolate = ForIsolate(isolate);
 
   do {
-    // Right now, there is no way to drain only background tasks associated with
-    // a specific isolate, so this sometimes does more work than necessary.
-    // In the long run, that functionality is probably going to be available
-    // anyway, though.
+    // Right now, there is no way to drain only background tasks associated
+    // with a specific isolate, so this sometimes does more work than
+    // necessary. In the long run, that functionality is probably going to
+    // be available anyway, though.
     background_tasks_.BlockingDrain();
   } while (per_isolate->FlushForegroundTasksInternal());
 }
 
 bool PerIsolatePlatformData::FlushForegroundTasksInternal() {
   bool did_work = false;
 
-  while (auto delayed = foreground_delayed_tasks_.Pop()) {
+  while (std::unique_ptr<DelayedTask> delayed =
+      foreground_delayed_tasks_.Pop()) {
     did_work = true;
     uint64_t delay_millis =
         static_cast<uint64_t>(delayed->timeout + 0.5) * 1000;
-    delayed->timer.data = static_cast<void*>(delayed);
+    delayed->timer.data = static_cast<void*>(delayed.get());
     uv_timer_init(loop_, &delayed->timer);
     // Timers may not guarantee queue ordering of events with the same delay if
     // 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);
+    scheduled_delayed_tasks_.push_back(delayed.release());
   }
-  while (Task* task = foreground_tasks_.Pop()) {
+  while (std::unique_ptr<Task> task = foreground_tasks_.Pop()) {
     did_work = true;
-    RunForegroundTask(task);
+    RunForegroundTask(std::move(task));
   }
   return did_work;
 }
 
 void NodePlatform::CallOnBackgroundThread(Task* task,
                                           ExpectedRuntime expected_runtime) {
-  background_tasks_.Push(task);
+  background_tasks_.Push(std::unique_ptr<Task>(task));
 }
 
 PerIsolatePlatformData* NodePlatform::ForIsolate(Isolate* isolate) {
@@ -205,14 +205,14 @@ PerIsolatePlatformData* NodePlatform::ForIsolate(Isolate* isolate) {
 }
 
 void NodePlatform::CallOnForegroundThread(Isolate* isolate, Task* task) {
-  ForIsolate(isolate)->CallOnForegroundThread(task);
+  ForIsolate(isolate)->CallOnForegroundThread(std::unique_ptr<Task>(task));
 }
 
 void NodePlatform::CallDelayedOnForegroundThread(Isolate* isolate,
                                                  Task* task,
                                                  double delay_in_seconds) {
-  ForIsolate(isolate)->CallDelayedOnForegroundThread(task,
-                                                     delay_in_seconds);
+  ForIsolate(isolate)->CallDelayedOnForegroundThread(
+    std::unique_ptr<Task>(task), delay_in_seconds);
 }
 
 void NodePlatform::FlushForegroundTasks(v8::Isolate* isolate) {
@@ -240,34 +240,34 @@ TaskQueue<T>::TaskQueue()
       outstanding_tasks_(0), stopped_(false), task_queue_() { }
 
 template <class T>
-void TaskQueue<T>::Push(T* task) {
+void TaskQueue<T>::Push(std::unique_ptr<T> task) {
   Mutex::ScopedLock scoped_lock(lock_);
   outstanding_tasks_++;
-  task_queue_.push(task);
+  task_queue_.push(std::move(task));
   tasks_available_.Signal(scoped_lock);
 }
 
 template <class T>
-T* TaskQueue<T>::Pop() {
+std::unique_ptr<T> TaskQueue<T>::Pop() {
   Mutex::ScopedLock scoped_lock(lock_);
-  T* result = nullptr;
-  if (!task_queue_.empty()) {
-    result = task_queue_.front();
-    task_queue_.pop();
+  if (task_queue_.empty()) {
+    return std::unique_ptr<T>(nullptr);
   }
+  std::unique_ptr<T> result = std::move(task_queue_.front());
+  task_queue_.pop();
   return result;
 }
 
 template <class T>
-T* TaskQueue<T>::BlockingPop() {
+std::unique_ptr<T> TaskQueue<T>::BlockingPop() {
   Mutex::ScopedLock scoped_lock(lock_);
   while (task_queue_.empty() && !stopped_) {
     tasks_available_.Wait(scoped_lock);
   }
   if (stopped_) {
-    return nullptr;
+    return std::unique_ptr<T>(nullptr);
   }
-  T* result = task_queue_.front();
+  std::unique_ptr<T> result = std::move(task_queue_.front());
   task_queue_.pop();
   return result;
 }

diff --git a/src/node_platform.h b/src/node_platform.h
@@ -22,9 +22,9 @@ class TaskQueue {
   TaskQueue();
   ~TaskQueue() {}
 
-  void Push(T* task);
-  T* Pop();
-  T* BlockingPop();
+  void Push(std::unique_ptr<T> task);
+  std::unique_ptr<T> Pop();
+  std::unique_ptr<T> BlockingPop();
   void NotifyOfCompletion();
   void BlockingDrain();
   void Stop();
@@ -35,11 +35,11 @@ class TaskQueue {
   ConditionVariable tasks_drained_;
   int outstanding_tasks_;
   bool stopped_;
-  std::queue<T*> task_queue_;
+  std::queue<std::unique_ptr<T>> task_queue_;
 };
 
 struct DelayedTask {
-  v8::Task* task;
+  std::unique_ptr<v8::Task> task;
   uv_timer_t timer;
   double timeout;
   PerIsolatePlatformData* platform_data;
@@ -50,8 +50,9 @@ class PerIsolatePlatformData {
   PerIsolatePlatformData(v8::Isolate* isolate, uv_loop_t* loop);
   ~PerIsolatePlatformData();
 
-  void CallOnForegroundThread(v8::Task* task);
-  void CallDelayedOnForegroundThread(v8::Task* task, double delay_in_seconds);
+  void CallOnForegroundThread(std::unique_ptr<v8::Task> task);
+  void CallDelayedOnForegroundThread(std::unique_ptr<v8::Task> task,
+    double delay_in_seconds);
 
   void Shutdown();
 
@@ -64,7 +65,7 @@ class PerIsolatePlatformData {
 
  private:
   static void FlushTasks(uv_async_t* handle);
-  static void RunForegroundTask(v8::Task* task);
+  static void RunForegroundTask(std::unique_ptr<v8::Task> task);
   static void RunForegroundTask(uv_timer_t* timer);
 
   int ref_count_ = 1;