Skip to content

Commit 2cf2635

Browse files
fhinkelrvagg
authored andcommitted
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: #20901 PR-URL: #17083 Reviewed-By: Anna Henningsen <[email protected]>
1 parent 2148b19 commit 2cf2635

File tree

2 files changed

+27
-16
lines changed

2 files changed

+27
-16
lines changed

‎src/node_platform.cc‎

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -134,26 +134,23 @@ void PerIsolatePlatformData::RunForegroundTask(std::unique_ptr<Task> task){
134134
task->Run();
135135
}
136136

137+
voidPerIsolatePlatformData::DeleteFromScheduledTasks(DelayedTask* task){
138+
auto it = std::find_if(scheduled_delayed_tasks_.begin(),
139+
scheduled_delayed_tasks_.end(),
140+
[task](const DelayedTaskPointer& delayed) -> bool{
141+
return delayed.get() == task;
142+
});
143+
CHECK_NE(it, scheduled_delayed_tasks_.end());
144+
scheduled_delayed_tasks_.erase(it);
145+
}
146+
137147
voidPerIsolatePlatformData::RunForegroundTask(uv_timer_t* handle){
138148
DelayedTask* delayed = static_cast<DelayedTask*>(handle->data);
139-
auto& tasklist = delayed->platform_data->scheduled_delayed_tasks_;
140-
auto it = std::find(tasklist.begin(), tasklist.end(), delayed);
141-
CHECK_NE(it, tasklist.end());
142-
tasklist.erase(it);
143149
RunForegroundTask(std::move(delayed->task));
144-
uv_close(reinterpret_cast<uv_handle_t*>(&delayed->timer),
145-
[](uv_handle_t* handle){
146-
deletestatic_cast<DelayedTask*>(handle->data);
147-
});
150+
delayed->platform_data->DeleteFromScheduledTasks(delayed);
148151
}
149152

150153
voidPerIsolatePlatformData::CancelPendingDelayedTasks(){
151-
for (auto delayed : scheduled_delayed_tasks_){
152-
uv_close(reinterpret_cast<uv_handle_t*>(&delayed->timer),
153-
[](uv_handle_t* handle){
154-
deletestatic_cast<DelayedTask*>(handle->data);
155-
});
156-
}
157154
scheduled_delayed_tasks_.clear();
158155
}
159156

@@ -183,7 +180,14 @@ bool PerIsolatePlatformData::FlushForegroundTasksInternal(){
183180
// the delay is non-zero. This should not be a problem in practice.
184181
uv_timer_start(&delayed->timer, RunForegroundTask, delay_millis, 0);
185182
uv_unref(reinterpret_cast<uv_handle_t*>(&delayed->timer));
186-
scheduled_delayed_tasks_.push_back(delayed.release());
183+
184+
scheduled_delayed_tasks_.emplace_back(delayed.release(),
185+
[](DelayedTask* delayed){
186+
uv_close(reinterpret_cast<uv_handle_t*>(&delayed->timer),
187+
[](uv_handle_t* handle){
188+
deletestatic_cast<DelayedTask*>(handle->data);
189+
});
190+
});
187191
}
188192
while (std::unique_ptr<Task> task = foreground_tasks_.Pop()){
189193
did_work = true;

‎src/node_platform.h‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include<queue>
55
#include<unordered_map>
66
#include<vector>
7+
#include<functional>
78

89
#include"libplatform/libplatform.h"
910
#include"node.h"
@@ -64,6 +65,8 @@ class PerIsolatePlatformData{
6465
voidCancelPendingDelayedTasks();
6566

6667
private:
68+
voidDeleteFromScheduledTasks(DelayedTask* task);
69+
6770
staticvoidFlushTasks(uv_async_t* handle);
6871
staticvoidRunForegroundTask(std::unique_ptr<v8::Task> task);
6972
staticvoidRunForegroundTask(uv_timer_t* timer);
@@ -74,7 +77,11 @@ class PerIsolatePlatformData{
7477
uv_async_t* flush_tasks_ = nullptr;
7578
TaskQueue<v8::Task> foreground_tasks_;
7679
TaskQueue<DelayedTask> foreground_delayed_tasks_;
77-
std::vector<DelayedTask*> scheduled_delayed_tasks_;
80+
81+
// Use a custom deleter because libuv needs to close the handle first.
82+
typedef std::unique_ptr<DelayedTask, std::function<void(DelayedTask*)>>
83+
DelayedTaskPointer;
84+
std::vector<DelayedTaskPointer> scheduled_delayed_tasks_;
7885
};
7986

8087
classNodePlatform : publicMultiIsolatePlatform{

0 commit comments

Comments
(0)