Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 5ddef29

Browse filesBrowse files
apapirovskiaddaleax
authored andcommitted
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>
1 parent b4c933d commit 5ddef29
Copy full SHA for 5ddef29

File tree

Expand file treeCollapse file tree

4 files changed

+37
-2
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

4 files changed

+37
-2
lines changed
Open diff view settings
Collapse file

‎src/async_wrap.cc‎

Copy file name to clipboardExpand all lines: src/async_wrap.cc
+9-1Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,12 @@ static void DestroyAsyncIdsCallback(Environment* env, void* data) {
159159
} while (!env->destroy_async_id_list()->empty());
160160
}
161161

162+
static void DestroyAsyncIdsCallback(void* arg) {
163+
Environment* env = static_cast<Environment*>(arg);
164+
if (!env->destroy_async_id_list()->empty())
165+
DestroyAsyncIdsCallback(env, nullptr);
166+
}
167+
162168

163169
void AsyncWrap::EmitPromiseResolve(Environment* env, double async_id) {
164170
AsyncHooks* async_hooks = env->async_hooks();
@@ -501,6 +507,8 @@ void AsyncWrap::Initialize(Local<Object> target,
501507
Isolate* isolate = env->isolate();
502508
HandleScope scope(isolate);
503509

510+
env->BeforeExit(DestroyAsyncIdsCallback, env);
511+
504512
env->SetMethod(target, "setupHooks", SetupHooks);
505513
env->SetMethod(target, "pushAsyncIds", PushAsyncIds);
506514
env->SetMethod(target, "popAsyncIds", PopAsyncIds);
@@ -662,7 +670,7 @@ void AsyncWrap::EmitDestroy(Environment* env, double async_id) {
662670
return;
663671

664672
if (env->destroy_async_id_list()->empty()) {
665-
env->SetImmediate(DestroyAsyncIdsCallback, nullptr);
673+
env->SetUnrefImmediate(DestroyAsyncIdsCallback, nullptr);
666674
}
667675

668676
env->destroy_async_id_list()->push_back(async_id);
Collapse file

‎src/env.cc‎

Copy file name to clipboardExpand all lines: src/env.cc
+11Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,17 @@ void Environment::PrintSyncTrace() const {
209209
fflush(stderr);
210210
}
211211

212+
void Environment::RunBeforeExitCallbacks() {
213+
for (BeforeExitCallback before_exit : before_exit_functions_) {
214+
before_exit.cb_(before_exit.arg_);
215+
}
216+
before_exit_functions_.clear();
217+
}
218+
219+
void Environment::BeforeExit(void (*cb)(void* arg), void* arg) {
220+
before_exit_functions_.push_back(BeforeExitCallback{cb, arg});
221+
}
222+
212223
void Environment::RunAtExitCallbacks() {
213224
for (AtExitCallback at_exit : at_exit_functions_) {
214225
at_exit.cb_(at_exit.arg_);
Collapse file

‎src/env.h‎

Copy file name to clipboardExpand all lines: src/env.h
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,8 @@ class Environment {
644644
const char* name,
645645
v8::FunctionCallback callback);
646646

647+
void BeforeExit(void (*cb)(void* arg), void* arg);
648+
void RunBeforeExitCallbacks();
647649
void AtExit(void (*cb)(void* arg), void* arg);
648650
void RunAtExitCallbacks();
649651

@@ -770,6 +772,12 @@ class Environment {
770772

771773
double* fs_stats_field_array_;
772774

775+
struct BeforeExitCallback {
776+
void (*cb_)(void* arg);
777+
void* arg_;
778+
};
779+
std::list<BeforeExitCallback> before_exit_functions_;
780+
773781
struct AtExitCallback {
774782
void (*cb_)(void* arg);
775783
void* arg_;
Collapse file

‎src/node.cc‎

Copy file name to clipboardExpand all lines: src/node.cc
+9-1Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4555,6 +4555,14 @@ void AtExit(Environment* env, void (*cb)(void* arg), void* arg) {
45554555
}
45564556

45574557

4558+
void RunBeforeExit(Environment* env) {
4559+
env->RunBeforeExitCallbacks();
4560+
4561+
if (!uv_loop_alive(env->event_loop()))
4562+
EmitBeforeExit(env);
4563+
}
4564+
4565+
45584566
void EmitBeforeExit(Environment* env) {
45594567
HandleScope handle_scope(env->isolate());
45604568
Context::Scope context_scope(env->context());
@@ -4705,7 +4713,7 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data,
47054713
if (more)
47064714
continue;
47074715

4708-
EmitBeforeExit(&env);
4716+
RunBeforeExit(&env);
47094717

47104718
// Emit `beforeExit` if the loop became alive either after emitting
47114719
// event, or after running some callbacks.

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.