Skip to content

Commit 1b76cfe

Browse files
fhinkelMylesBorins
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. PR-URL: #17083 Reviewed-By: Anna Henningsen <[email protected]>
1 parent af63df8 commit 1b76cfe

File tree

2 files changed

+26
-16
lines changed

2 files changed

+26
-16
lines changed

src/node_platform.cc

+19-15
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+
void PerIsolatePlatformData::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
void PerIsolatePlatformData::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-
delete static_cast<DelayedTask*>(handle->data);
147-
});
150+
delayed->platform_data->DeleteFromScheduledTasks(delayed);
148151
}
149152

150153
void PerIsolatePlatformData::CancelPendingDelayedTasks() {
151-
for (auto delayed : scheduled_delayed_tasks_) {
152-
uv_close(reinterpret_cast<uv_handle_t*>(&delayed->timer),
153-
[](uv_handle_t* handle) {
154-
delete static_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+
delete static_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

+7-1
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@ class PerIsolatePlatformData {
6464
void CancelPendingDelayedTasks();
6565

6666
private:
67+
void DeleteFromScheduledTasks(DelayedTask* task);
68+
6769
static void FlushTasks(uv_async_t* handle);
6870
static void RunForegroundTask(std::unique_ptr<v8::Task> task);
6971
static void RunForegroundTask(uv_timer_t* timer);
@@ -74,7 +76,11 @@ class PerIsolatePlatformData {
7476
uv_async_t* flush_tasks_ = nullptr;
7577
TaskQueue<v8::Task> foreground_tasks_;
7678
TaskQueue<DelayedTask> foreground_delayed_tasks_;
77-
std::vector<DelayedTask*> scheduled_delayed_tasks_;
79+
80+
// Use a custom deleter because libuv needs to close the handle first.
81+
typedef std::unique_ptr<DelayedTask, std::function<void(DelayedTask*)>>
82+
DelayedTaskPointer;
83+
std::vector<DelayedTaskPointer> scheduled_delayed_tasks_;
7884
};
7985

8086
class NodePlatform : public MultiIsolatePlatform {

0 commit comments

Comments
 (0)