FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: cancel pending delayed platform tasks on exit · nodejs/node@c2431d5 · GitHub

/ node Public

Commit c2431d5

Browse files
addaleax
authored and
MylesBorins
committed
src: cancel pending delayed platform tasks on exit
Worker threads need an event loop without active libuv handles in order to shut down. One source of handles that was previously not accounted for were delayed V8 tasks; these create timers that would be standing in the way of clearing the event loop. To solve this, keep track of the scheduled tasks in a list and close their timer handles before the corresponding isolate/loop is removed from the platform. It is not clear from the V8 documentation what the expectation is with respect to pending background tasks at the end of the isolate lifetime; however, an alternative approach of executing these scheduled tasks when flushing them led to an infinite loop of tasks scheduling each other; so it seems safe to assume that the behaviour implemented in this patch is at least acceptable. Original-PR-URL: ayojs/ayo#120 Original-Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> PR-URL: #16700 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 37a60a8 commit c2431d5

4 files changed

Lines changed: 61 additions & 16 deletions

File tree

‎src/node.cc‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,10 @@ static struct {
284284
platform_->DrainBackgroundTasks(isolate);
285285
}
286286

287+
void CancelVMTasks(Isolate* isolate) {
288+
platform_->CancelPendingDelayedTasks(isolate);
289+
}
290+
287291
#if HAVE_INSPECTOR
288292
bool StartInspector(Environment *env, const char* script_path,
289293
const node::DebugOptions& options) {
@@ -316,6 +320,7 @@ static struct {
316320
void Initialize(int thread_pool_size) {}
317321
void Dispose() {}
318322
void DrainVMTasks(Isolate* isolate) {}
323+
void CancelVMTasks(Isolate* isolate) {}
319324
bool StartInspector(Environment *env, const char* script_path,
320325
const node::DebugOptions& options) {
321326
env->ThrowError("Node compiled with NODE_USE_V8_PLATFORM=0");
@@ -4891,6 +4896,7 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data,
48914896
uv_key_delete(&thread_local_env);
48924897

48934898
v8_platform.DrainVMTasks(isolate);
4899+
v8_platform.CancelVMTasks(isolate);
48944900
WaitForInspectorDisconnect(&env);
48954901
#if defined(LEAK_SANITIZER)
48964902
__lsan_do_leak_check();

‎src/node.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,7 @@ class MultiIsolatePlatform : public v8::Platform {
214214
public:
215215
virtual ~MultiIsolatePlatform() { }
216216
virtual void DrainBackgroundTasks(v8::Isolate* isolate) = 0;
217+
virtual void CancelPendingDelayedTasks(v8::Isolate* isolate) = 0;
217218

218219
// These will be called by the `IsolateData` creation/destruction functions.
219220
virtual void RegisterIsolate(IsolateData* isolate_data,

‎src/node_platform.cc‎

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include "env.h"
55
#include "env-inl.h"
66
#include "util.h"
7+
#include <algorithm>
78

89
namespace node {
910

@@ -45,13 +46,17 @@ void PerIsolatePlatformData::CallOnForegroundThread(Task* task) {
4546

4647
void PerIsolatePlatformData::CallDelayedOnForegroundThread(
4748
Task* task, double delay_in_seconds) {
48-
auto pair = new std::pair<Task*, double>(task, delay_in_seconds);
49-
foreground_delayed_tasks_.Push(pair);
49+
auto delayed = new DelayedTask();
50+
delayed->task = task;
51+
delayed->platform_data = this;
52+
delayed->timeout = delay_in_seconds;
53+
foreground_delayed_tasks_.Push(delayed);
5054
uv_async_send(flush_tasks_);
5155
}
5256

5357
PerIsolatePlatformData::~PerIsolatePlatformData() {
5458
FlushForegroundTasksInternal();
59+
CancelPendingDelayedTasks();
5560

5661
uv_close(reinterpret_cast<uv_handle_t*>(flush_tasks_),
5762
[](uv_handle_t* handle) {
@@ -120,7 +125,7 @@ size_t NodePlatform::NumberOfAvailableBackgroundThreads() {
120125
return threads_.size();
121126
}
122127

123-
static void RunForegroundTask(Task* task) {
128+
void PerIsolatePlatformData::RunForegroundTask(Task* task) {
124129
Isolate* isolate = Isolate::GetCurrent();
125130
HandleScope scope(isolate);
126131
Environment* env = Environment::GetCurrent(isolate);
@@ -130,14 +135,29 @@ static void RunForegroundTask(Task* task) {
130135
delete task;
131136
}
132137

133-
static void RunForegroundTask(uv_timer_t* handle) {
134-
Task* task = static_cast<Task*>(handle->data);
135-
RunForegroundTask(task);
136-
uv_close(reinterpret_cast<uv_handle_t*>(handle), [](uv_handle_t* handle) {
137-
delete reinterpret_cast<uv_timer_t*>(handle);
138+
void PerIsolatePlatformData::RunForegroundTask(uv_timer_t* handle) {
139+
DelayedTask* delayed = static_cast<DelayedTask*>(handle->data);
140+
auto& tasklist = delayed->platform_data->scheduled_delayed_tasks_;
141+
auto it = std::find(tasklist.begin(), tasklist.end(), delayed);
142+
CHECK_NE(it, tasklist.end());
143+
tasklist.erase(it);
144+
RunForegroundTask(delayed->task);
145+
uv_close(reinterpret_cast<uv_handle_t*>(&delayed->timer),
146+
[](uv_handle_t* handle) {
147+
delete static_cast<DelayedTask*>(handle->data);
138148
});
139149
}
140150

151+
void PerIsolatePlatformData::CancelPendingDelayedTasks() {
152+
for (auto delayed : scheduled_delayed_tasks_) {
153+
uv_close(reinterpret_cast<uv_handle_t*>(&delayed->timer),
154+
[](uv_handle_t* handle) {
155+
delete static_cast<DelayedTask*>(handle->data);
156+
});
157+
}
158+
scheduled_delayed_tasks_.clear();
159+
}
160+
141161
void NodePlatform::DrainBackgroundTasks(Isolate* isolate) {
142162
PerIsolatePlatformData* per_isolate = ForIsolate(isolate);
143163

@@ -152,18 +172,18 @@ void NodePlatform::DrainBackgroundTasks(Isolate* isolate) {
152172

153173
bool PerIsolatePlatformData::FlushForegroundTasksInternal() {
154174
bool did_work = false;
175+
155176
while (auto delayed = foreground_delayed_tasks_.Pop()) {
156177
did_work = true;
157178
uint64_t delay_millis =
158-
static_cast<uint64_t>(delayed->second + 0.5) * 1000;
159-
uv_timer_t* handle = new uv_timer_t();
160-
handle->data = static_cast<void*>(delayed->first);
161-
uv_timer_init(loop_, handle);
179+
static_cast<uint64_t>(delayed->timeout + 0.5) * 1000;
180+
delayed->timer.data = static_cast<void*>(delayed);
181+
uv_timer_init(loop_, &delayed->timer);
162182
// Timers may not guarantee queue ordering of events with the same delay if
163183
// the delay is non-zero. This should not be a problem in practice.
164-
uv_timer_start(handle, RunForegroundTask, delay_millis, 0);
165-
uv_unref(reinterpret_cast<uv_handle_t*>(handle));
166-
delete delayed;
184+
uv_timer_start(&delayed->timer, RunForegroundTask, delay_millis, 0);
185+
uv_unref(reinterpret_cast<uv_handle_t*>(&delayed->timer));
186+
scheduled_delayed_tasks_.push_back(delayed);
167187
}
168188
while (Task* task = foreground_tasks_.Pop()) {
169189
did_work = true;
@@ -199,6 +219,10 @@ void NodePlatform::FlushForegroundTasks(v8::Isolate* isolate) {
199219
ForIsolate(isolate)->FlushForegroundTasksInternal();
200220
}
201221

222+
void NodePlatform::CancelPendingDelayedTasks(v8::Isolate* isolate) {
223+
ForIsolate(isolate)->CancelPendingDelayedTasks();
224+
}
225+
202226
bool NodePlatform::IdleTasksEnabled(Isolate* isolate) { return false; }
203227

204228
double NodePlatform::MonotonicallyIncreasingTime() {

‎src/node_platform.h‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ namespace node {
1414

1515
class NodePlatform;
1616
class IsolateData;
17+
class PerIsolatePlatformData;
1718

1819
template <class T>
1920
class TaskQueue {
@@ -37,6 +38,13 @@ class TaskQueue {
3738
std::queue<T*> task_queue_;
3839
};
3940

41+
struct DelayedTask {
42+
v8::Task* task;
43+
uv_timer_t timer;
44+
double timeout;
45+
PerIsolatePlatformData* platform_data;
46+
};
47+
4048
class PerIsolatePlatformData {
4149
public:
4250
PerIsolatePlatformData(v8::Isolate* isolate, uv_loop_t* loop);
@@ -52,15 +60,20 @@ class PerIsolatePlatformData {
5260

5361
// Returns true iff work was dispatched or executed.
5462
bool FlushForegroundTasksInternal();
63+
void CancelPendingDelayedTasks();
64+
5565
private:
5666
static void FlushTasks(uv_async_t* handle);
67+
static void RunForegroundTask(v8::Task* task);
68+
static void RunForegroundTask(uv_timer_t* timer);
5769

5870
int ref_count_ = 1;
5971
v8::Isolate* isolate_;
6072
uv_loop_t* const loop_;
6173
uv_async_t* flush_tasks_ = nullptr;
6274
TaskQueue<v8::Task> foreground_tasks_;
63-
TaskQueue<std::pair<v8::Task*, double>> foreground_delayed_tasks_;
75+
TaskQueue<DelayedTask> foreground_delayed_tasks_;
76+
std::vector<DelayedTask*> scheduled_delayed_tasks_;
6477
};
6578

6679
class NodePlatform : public MultiIsolatePlatform {
@@ -69,6 +82,7 @@ class NodePlatform : public MultiIsolatePlatform {
6982
virtual ~NodePlatform() {}
7083

7184
void DrainBackgroundTasks(v8::Isolate* isolate) override;
85+
void CancelPendingDelayedTasks(v8::Isolate* isolate) override;
7286
void Shutdown();
7387

7488
// v8::Platform implementation.

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL