src: use unique_ptr in platform implementation

Replace raw pointers in task queues with std::unique_ptr. This makes ownership obvious. 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]>

Author: fhinkel

src/node_platform.cc

@@ -18,9 +18,8 @@ using v8::TracingController;
 
 staticvoidBackgroundRunner(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();
 }
 
-voidPerIsolatePlatformData::CallOnForegroundThread(Task* task){
- foreground_tasks_.Push(task);
+voidPerIsolatePlatformData::CallOnForegroundThread(
+ std::unique_ptr<Task> task){
+ foreground_tasks_.Push(std::move(task));
 uv_async_send(flush_tasks_);
 }
 
 voidPerIsolatePlatformData::CallDelayedOnForegroundThread(
-Task* task, double delay_in_seconds){
-auto delayed = newDelayedTask();
- delayed->task = task;
+std::unique_ptr<Task> task, double delay_in_seconds){
+std::unique_ptr<DelayedTask>delayed(newDelayedTask());
+ 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();
 }
 
-voidPerIsolatePlatformData::RunForegroundTask(Task* task){
+voidPerIsolatePlatformData::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;
 }
 
 voidPerIsolatePlatformData::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){
 deletestatic_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());
 }
 
 boolPerIsolatePlatformData::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;
 }
 
 voidNodePlatform::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){
 }
 
 voidNodePlatform::CallOnForegroundThread(Isolate* isolate, Task* task){
-ForIsolate(isolate)->CallOnForegroundThread(task);
+ForIsolate(isolate)->CallOnForegroundThread(std::unique_ptr<Task>(task));
 }
 
 voidNodePlatform::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);
 }
 
 voidNodePlatform::FlushForegroundTasks(v8::Isolate* isolate){
@@ -240,34 +240,34 @@ TaskQueue<T>::TaskQueue()
 outstanding_tasks_(0), stopped_(false), task_queue_(){}
 
 template <classT>
-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 <classT>
-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 <classT>
-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_){
-returnnullptr;
+returnstd::unique_ptr<T>(nullptr);
  }
-T* result = task_queue_.front();
+std::unique_ptr<T> result = std::move(task_queue_.front());
  task_queue_.pop();
 return result;
 }

src/node_platform.h

@@ -22,9 +22,9 @@ class TaskQueue{
 TaskQueue();
 ~TaskQueue(){}
 
-voidPush(T* task);
-T*Pop();
-T*BlockingPop();
+voidPush(std::unique_ptr<T> task);
+std::unique_ptr<T>Pop();
+std::unique_ptr<T>BlockingPop();
 voidNotifyOfCompletion();
 voidBlockingDrain();
 voidStop();
@@ -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_;
 };
 
 structDelayedTask{
- 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();
 
-voidCallOnForegroundThread(v8::Task* task);
-voidCallDelayedOnForegroundThread(v8::Task* task, double delay_in_seconds);
+voidCallOnForegroundThread(std::unique_ptr<v8::Task> task);
+voidCallDelayedOnForegroundThread(std::unique_ptr<v8::Task> task,
+double delay_in_seconds);
 
 voidShutdown();
 
@@ -64,7 +65,7 @@ class PerIsolatePlatformData{
 
 private:
 staticvoidFlushTasks(uv_async_t* handle);
-staticvoidRunForegroundTask(v8::Task* task);
+staticvoidRunForegroundTask(std::unique_ptr<v8::Task> task);
 staticvoidRunForegroundTask(uv_timer_t* timer);
 
 int ref_count_ = 1;