Skip to content

Commit

Permalink
src: use unique_ptr for scheduled delayed tasks
Browse files Browse the repository at this point in the history
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 <anna@addaleax.net>
  • Loading branch information
fhinkel authored and rvagg committed Aug 16, 2018
1 parent 2148b19 commit 2cf2635
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 16 deletions.
34 changes: 19 additions & 15 deletions src/node_platform.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down Expand Up @@ -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;
Expand Down
9 changes: 8 additions & 1 deletion src/node_platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <queue>
#include <unordered_map>
#include <vector>
#include <functional>

#include "libplatform/libplatform.h"
#include "node.h"
Expand Down Expand Up @@ -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);
Expand All @@ -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 {
Expand Down

0 comments on commit 2cf2635

Please sign in to comment.