Permalink
Browse files

src: prepare platform for upstream V8 changes

V8 platform tasks may schedule other tasks (both background and
foreground), and may perform asynchronous operations like
resolving Promises.

To address that:

- Run the task queue drain call inside a callback scope.
  This makes sure asynchronous operations inside it, like
  resolving promises, lead to the microtask queue and any
  subsequent operations not being silently forgotten.
- Move the task queue drain call before `EmitBeforeExit()`
  and only run `EmitBeforeExit()` if there is no new event
  loop work.
- Account for possible new foreground tasks scheduled by
  background tasks in `DrainBackgroundTasks()`.

PR-URL: #15428
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matthew Loring <mattloring@google.com>
  • Loading branch information...
addaleax committed Sep 15, 2017
1 parent 01c680b commit f27b5e4bdaafc73a830a0451ee3c641b8bcd08fe
Showing with 69 additions and 37 deletions.
  1. +16 −29 src/node.cc
  2. +28 −0 src/node_internals.h
  3. +23 −7 src/node_platform.cc
  4. +2 −1 src/node_platform.h
View
@@ -1339,30 +1339,6 @@ void AddPromiseHook(v8::Isolate* isolate, promise_hook_func fn, void* arg) {
env->AddPromiseHook(fn, arg);
}
class InternalCallbackScope {
public:
InternalCallbackScope(Environment* env,
Local<Object> object,
const async_context& asyncContext);
~InternalCallbackScope();
void Close();
inline bool Failed() const { return failed_; }
inline void MarkAsFailed() { failed_ = true; }
inline bool IsInnerMakeCallback() const {
return callback_scope_.in_makecallback();
}
private:
Environment* env_;
async_context async_context_;
v8::Local<v8::Object> object_;
Environment::AsyncCallbackScope callback_scope_;
bool failed_ = false;
bool pushed_ids_ = false;
bool closed_ = false;
};
CallbackScope::CallbackScope(Isolate* isolate,
Local<Object> object,
async_context asyncContext)
@@ -1381,17 +1357,21 @@ CallbackScope::~CallbackScope() {
InternalCallbackScope::InternalCallbackScope(Environment* env,
Local<Object> object,
const async_context& asyncContext)
const async_context& asyncContext,
ResourceExpectation expect)
: env_(env),
async_context_(asyncContext),
object_(object),
callback_scope_(env) {
CHECK(!object.IsEmpty());
if (expect == kRequireResource) {
CHECK(!object.IsEmpty());
}
HandleScope handle_scope(env->isolate());
// If you hit this assertion, you forgot to enter the v8::Context first.
CHECK_EQ(env->context(), env->isolate()->GetCurrentContext());
if (env->using_domains()) {
if (env->using_domains() && !object_.IsEmpty()) {
DomainEnter(env, object_);
}
@@ -1413,6 +1393,7 @@ InternalCallbackScope::~InternalCallbackScope() {
void InternalCallbackScope::Close() {
if (closed_) return;
closed_ = true;
HandleScope handle_scope(env_->isolate());
if (pushed_ids_)
env_->async_hooks()->pop_ids(async_context_.async_id);
@@ -1423,7 +1404,7 @@ void InternalCallbackScope::Close() {
AsyncWrap::EmitAfter(env_, async_context_.async_id);
}
if (env_->using_domains()) {
if (env_->using_domains() && !object_.IsEmpty()) {
DomainExit(env_, object_);
}
@@ -1463,6 +1444,7 @@ MaybeLocal<Value> InternalMakeCallback(Environment* env,
int argc,
Local<Value> argv[],
async_context asyncContext) {
CHECK(!recv.IsEmpty());
InternalCallbackScope scope(env, recv, asyncContext);
if (scope.Failed()) {
return Undefined(env->isolate());
@@ -4726,9 +4708,14 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data,
do {
uv_run(env.event_loop(), UV_RUN_DEFAULT);
v8_platform.DrainVMTasks();
more = uv_loop_alive(env.event_loop());
if (more)
continue;
EmitBeforeExit(&env);
v8_platform.DrainVMTasks();
// Emit `beforeExit` if the loop became alive either after emitting
// event, or after running some callbacks.
more = uv_loop_alive(env.event_loop());
View
@@ -294,8 +294,36 @@ v8::MaybeLocal<v8::Value> InternalMakeCallback(
v8::Local<v8::Value> argv[],
async_context asyncContext);
class InternalCallbackScope {
public:
// Tell the constructor whether its `object` parameter may be empty or not.
enum ResourceExpectation { kRequireResource, kAllowEmptyResource };
InternalCallbackScope(Environment* env,
v8::Local<v8::Object> object,
const async_context& asyncContext,
ResourceExpectation expect = kRequireResource);
~InternalCallbackScope();
void Close();
inline bool Failed() const { return failed_; }
inline void MarkAsFailed() { failed_ = true; }
inline bool IsInnerMakeCallback() const {
return callback_scope_.in_makecallback();
}
private:
Environment* env_;
async_context async_context_;
v8::Local<v8::Object> object_;
Environment::AsyncCallbackScope callback_scope_;
bool failed_ = false;
bool pushed_ids_ = false;
bool closed_ = false;
};
} // namespace node
#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
#endif // SRC_NODE_INTERNALS_H_
View
@@ -1,10 +1,14 @@
#include "node_platform.h"
#include "node_internals.h"
#include "util.h"
namespace node {
using v8::HandleScope;
using v8::Isolate;
using v8::Local;
using v8::Object;
using v8::Platform;
using v8::Task;
using v8::TracingController;
@@ -63,22 +67,33 @@ size_t NodePlatform::NumberOfAvailableBackgroundThreads() {
return threads_.size();
}
static void RunForegroundTask(uv_timer_t* handle) {
Task* task = static_cast<Task*>(handle->data);
static void RunForegroundTask(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;
}
static void RunForegroundTask(uv_timer_t* handle) {
Task* task = static_cast<Task*>(handle->data);
RunForegroundTask(task);
uv_close(reinterpret_cast<uv_handle_t*>(handle), [](uv_handle_t* handle) {
delete reinterpret_cast<uv_timer_t*>(handle);
});
}
void NodePlatform::DrainBackgroundTasks() {
FlushForegroundTasksInternal();
background_tasks_.BlockingDrain();
while (FlushForegroundTasksInternal())
background_tasks_.BlockingDrain();
}
void NodePlatform::FlushForegroundTasksInternal() {
bool NodePlatform::FlushForegroundTasksInternal() {
bool did_work = false;
while (auto delayed = foreground_delayed_tasks_.Pop()) {
did_work = true;
uint64_t delay_millis =
static_cast<uint64_t>(delayed->second + 0.5) * 1000;
uv_timer_t* handle = new uv_timer_t();
@@ -91,9 +106,10 @@ void NodePlatform::FlushForegroundTasksInternal() {
delete delayed;
}
while (Task* task = foreground_tasks_.Pop()) {
task->Run();
delete task;
did_work = true;
RunForegroundTask(task);
}
return did_work;
}
void NodePlatform::CallOnBackgroundThread(Task* task,
View
@@ -39,7 +39,8 @@ class NodePlatform : public v8::Platform {
virtual ~NodePlatform() {}
void DrainBackgroundTasks();
void FlushForegroundTasksInternal();
// Returns true iff work was dispatched or executed.
bool FlushForegroundTasksInternal();
void Shutdown();
// v8::Platform implementation.

0 comments on commit f27b5e4

Please sign in to comment.