Permalink
Browse files

async_wrap: schedule destroy hook as unref

Since the `DestroyAsyncIdsCallback` in Node.js can be scheduled as
a result of GC, that means that it can accidentally keep the event
loop open when it shouldn't.

Replace `SetImmediate` with the newly introduced `SetUnrefImmediate`
and in addition introduce RunBeforeExit callbacks, of which
`DestroyAsyncIdsCallback` is now the first. These callbacks will run
before the `beforeExit` event is emitted (which will now only be
emitted if the event loop is still not active).

PR-URL: #18241
Fixes: #18190
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
  • Loading branch information...
apapirovski committed Jan 18, 2018
1 parent d62566e commit 8803b69c724e7b03ec0fc4c82b71575262487624
Showing with 37 additions and 2 deletions.
  1. +9 −1 src/async_wrap.cc
  2. +11 −0 src/env.cc
  3. +8 −0 src/env.h
  4. +9 −1 src/node.cc
@@ -159,6 +159,12 @@ static void DestroyAsyncIdsCallback(Environment* env, void* data) {
} while (!env->destroy_async_id_list()->empty());
}
static void DestroyAsyncIdsCallback(void* arg) {
Environment* env = static_cast<Environment*>(arg);
if (!env->destroy_async_id_list()->empty())
DestroyAsyncIdsCallback(env, nullptr);
}
void AsyncWrap::EmitPromiseResolve(Environment* env, double async_id) {
AsyncHooks* async_hooks = env->async_hooks();
@@ -502,6 +508,8 @@ void AsyncWrap::Initialize(Local<Object> target,
Isolate* isolate = env->isolate();
HandleScope scope(isolate);
env->BeforeExit(DestroyAsyncIdsCallback, env);
env->SetMethod(target, "setupHooks", SetupHooks);
env->SetMethod(target, "pushAsyncIds", PushAsyncIds);
env->SetMethod(target, "popAsyncIds", PopAsyncIds);
@@ -663,7 +671,7 @@ void AsyncWrap::EmitDestroy(Environment* env, double async_id) {
return;
if (env->destroy_async_id_list()->empty()) {
env->SetImmediate(DestroyAsyncIdsCallback, nullptr);
env->SetUnrefImmediate(DestroyAsyncIdsCallback, nullptr);
}
env->destroy_async_id_list()->push_back(async_id);
@@ -211,6 +211,17 @@ void Environment::PrintSyncTrace() const {
fflush(stderr);
}
void Environment::RunBeforeExitCallbacks() {
for (BeforeExitCallback before_exit : before_exit_functions_) {
before_exit.cb_(before_exit.arg_);
}
before_exit_functions_.clear();
}
void Environment::BeforeExit(void (*cb)(void* arg), void* arg) {
before_exit_functions_.push_back(BeforeExitCallback{cb, arg});
}
void Environment::RunAtExitCallbacks() {
for (AtExitCallback at_exit : at_exit_functions_) {
at_exit.cb_(at_exit.arg_);
@@ -658,6 +658,8 @@ class Environment {
const char* name,
v8::FunctionCallback callback);
void BeforeExit(void (*cb)(void* arg), void* arg);
void RunBeforeExitCallbacks();
void AtExit(void (*cb)(void* arg), void* arg);
void RunAtExitCallbacks();
@@ -778,6 +780,12 @@ class Environment {
double* fs_stats_field_array_;
struct BeforeExitCallback {
void (*cb_)(void* arg);
void* arg_;
};
std::list<BeforeExitCallback> before_exit_functions_;
struct AtExitCallback {
void (*cb_)(void* arg);
void* arg_;
@@ -4319,6 +4319,14 @@ void AtExit(Environment* env, void (*cb)(void* arg), void* arg) {
}
void RunBeforeExit(Environment* env) {
env->RunBeforeExitCallbacks();
if (!uv_loop_alive(env->event_loop()))
EmitBeforeExit(env);
}
void EmitBeforeExit(Environment* env) {
HandleScope handle_scope(env->isolate());
Context::Scope context_scope(env->context());
@@ -4469,7 +4477,7 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data,
if (more)
continue;
EmitBeforeExit(&env);
RunBeforeExit(&env);
// Emit `beforeExit` if the loop became alive either after emitting
// event, or after running some callbacks.

0 comments on commit 8803b69

Please sign in to comment.