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 127439d

Browse filesBrowse files
joyeecheungaddaleax
joyeecheung
authored and
addaleax
committed
process: make tick callback and promise rejection callback more robust
- Rename `internalTickCallback` to `processTicksAndRejections`, make sure it does not get called if it's not set in C++. - Rename `emitPromiseRejectionWarnings` to `processPromiseRejections` since it also emit events that are not warnings. - Sets `SetPromiseRejectCallback` in the `Environment` constructor to make sure it only gets called once per-isolate, and make sure it does not get called if it's not set in C++. - Wrap promise rejection callback initialization into `listenForRejections()`. - Add comments. PR-URL: #25200 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent abc4cc2 commit 127439d
Copy full SHA for 127439d

File tree

Expand file treeCollapse file tree

9 files changed

+50
-32
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

9 files changed

+50
-32
lines changed
Open diff view settings
Collapse file

‎lib/internal/process/next_tick.js‎

Copy file name to clipboardExpand all lines: lib/internal/process/next_tick.js
+9-11Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,14 @@ const {
66
tickInfo,
77
// Used to run V8's micro task queue.
88
runMicrotasks,
9-
setTickCallback,
10-
initializePromiseRejectCallback
9+
setTickCallback
1110
} = internalBinding('task_queue');
1211

1312
const {
1413
setHasRejectionToWarn,
1514
hasRejectionToWarn,
16-
promiseRejectHandler,
17-
emitPromiseRejectionWarnings
15+
listenForRejections,
16+
processPromiseRejections
1817
} = require('internal/process/promises');
1918

2019
const {
@@ -49,10 +48,10 @@ function runNextTicks() {
4948
if (!hasTickScheduled() && !hasRejectionToWarn())
5049
return;
5150

52-
internalTickCallback();
51+
processTicksAndRejections();
5352
}
5453

55-
function internalTickCallback() {
54+
function processTicksAndRejections() {
5655
let tock;
5756
do {
5857
while (tock = queue.shift()) {
@@ -80,7 +79,7 @@ function internalTickCallback() {
8079
}
8180
setHasTickScheduled(false);
8281
runMicrotasks();
83-
} while (!queue.isEmpty() || emitPromiseRejectionWarnings());
82+
} while (!queue.isEmpty() || processPromiseRejections());
8483
setHasRejectionToWarn(false);
8584
}
8685

@@ -134,11 +133,10 @@ function nextTick(callback) {
134133
// TODO(joyeecheung): make this a factory class so that node.js can
135134
// control the side effects caused by the initializers.
136135
exports.setup = function() {
137-
// Initializes the per-isolate promise rejection callback which
138-
// will call the handler being passed into this function.
139-
initializePromiseRejectCallback(promiseRejectHandler);
136+
// Sets the per-isolate promise rejection callback
137+
listenForRejections();
140138
// Sets the callback to be run in every tick.
141-
setTickCallback(internalTickCallback);
139+
setTickCallback(processTicksAndRejections);
142140
return {
143141
nextTick,
144142
runNextTicks
Collapse file

‎lib/internal/process/promises.js‎

Copy file name to clipboardExpand all lines: lib/internal/process/promises.js
+11-4Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ const {
88
kPromiseHandlerAddedAfterReject,
99
kPromiseResolveAfterResolved,
1010
kPromiseRejectAfterResolved
11-
}
11+
},
12+
setPromiseRejectCallback
1213
} = internalBinding('task_queue');
1314

1415
// *Must* match Environment::TickInfo::Fields in src/env.h.
@@ -117,7 +118,9 @@ function emitDeprecationWarning() {
117118
}
118119
}
119120

120-
function emitPromiseRejectionWarnings() {
121+
// If this method returns true, at least one more tick need to be
122+
// scheduled to process any potential pending rejections
123+
function processPromiseRejections() {
121124
while (asyncHandledRejections.length > 0) {
122125
const { promise, warning } = asyncHandledRejections.shift();
123126
if (!process.emit('rejectionHandled', promise)) {
@@ -142,9 +145,13 @@ function emitPromiseRejectionWarnings() {
142145
return maybeScheduledTicks || pendingUnhandledRejections.length !== 0;
143146
}
144147

148+
function listenForRejections() {
149+
setPromiseRejectCallback(promiseRejectHandler);
150+
}
151+
145152
module.exports = {
146153
hasRejectionToWarn,
147154
setHasRejectionToWarn,
148-
promiseRejectHandler,
149-
emitPromiseRejectionWarnings
155+
listenForRejections,
156+
processPromiseRejections
150157
};
Collapse file

‎src/callback_scope.cc‎

Copy file name to clipboardExpand all lines: src/callback_scope.cc
+8-2Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
namespace node {
77

8+
using v8::Function;
89
using v8::HandleScope;
910
using v8::Isolate;
1011
using v8::Local;
@@ -114,8 +115,13 @@ void InternalCallbackScope::Close() {
114115

115116
if (!env_->can_call_into_js()) return;
116117

117-
if (env_->tick_callback_function()
118-
->Call(env_->context(), process, 0, nullptr).IsEmpty()) {
118+
Local<Function> tick_callback = env_->tick_callback_function();
119+
120+
// The tick is triggered before JS land calls SetTickCallback
121+
// to initializes the tick callback during bootstrap.
122+
CHECK(!tick_callback.IsEmpty());
123+
124+
if (tick_callback->Call(env_->context(), process, 0, nullptr).IsEmpty()) {
119125
failed_ = true;
120126
}
121127
}
Collapse file

‎src/env.cc‎

Copy file name to clipboardExpand all lines: src/env.cc
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,10 @@ Environment::Environment(IsolateData* isolate_data,
235235
if (options_->no_force_async_hooks_checks) {
236236
async_hooks_.no_force_checks();
237237
}
238+
239+
// TODO(addaleax): the per-isolate state should not be controlled by
240+
// a single Environment.
241+
isolate()->SetPromiseRejectCallback(task_queue::PromiseRejectCallback);
238242
}
239243

240244
Environment::~Environment() {
Collapse file

‎src/node_internals.h‎

Copy file name to clipboardExpand all lines: src/node_internals.h
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,10 @@ SlicedArguments::SlicedArguments(
180180
size_ = size;
181181
}
182182

183+
namespace task_queue {
184+
void PromiseRejectCallback(v8::PromiseRejectMessage message);
185+
} // namespace task_queue
186+
183187
v8::Maybe<bool> ProcessEmitWarning(Environment* env, const char* fmt, ...);
184188
v8::Maybe<bool> ProcessEmitDeprecationWarning(Environment* env,
185189
const char* warning,
Collapse file

‎src/node_task_queue.cc‎

Copy file name to clipboardExpand all lines: src/node_task_queue.cc
+8-9Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ static void SetTickCallback(const FunctionCallbackInfo<Value>& args) {
3636
env->set_tick_callback_function(args[0].As<Function>());
3737
}
3838

39-
static void PromiseRejectCallback(PromiseRejectMessage message) {
39+
void PromiseRejectCallback(PromiseRejectMessage message) {
4040
static std::atomic<uint64_t> unhandledRejections{0};
4141
static std::atomic<uint64_t> rejectionsHandledAfter{0};
4242

@@ -49,6 +49,10 @@ static void PromiseRejectCallback(PromiseRejectMessage message) {
4949
if (env == nullptr) return;
5050

5151
Local<Function> callback = env->promise_reject_callback();
52+
// The promise is rejected before JS land calls SetPromiseRejectCallback
53+
// to initializes the promise reject callback during bootstrap.
54+
CHECK(!callback.IsEmpty());
55+
5256
Local<Value> value;
5357
Local<Value> type = Number::New(env->isolate(), event);
5458

@@ -83,17 +87,12 @@ static void PromiseRejectCallback(PromiseRejectMessage message) {
8387
env->context(), Undefined(isolate), arraysize(args), args));
8488
}
8589

86-
static void InitializePromiseRejectCallback(
90+
static void SetPromiseRejectCallback(
8791
const FunctionCallbackInfo<Value>& args) {
8892
Environment* env = Environment::GetCurrent(args);
8993
Isolate* isolate = env->isolate();
9094

9195
CHECK(args[0]->IsFunction());
92-
93-
// TODO(joyeecheung): this may be moved to somewhere earlier in the bootstrap
94-
// to make sure it's only called once
95-
isolate->SetPromiseRejectCallback(PromiseRejectCallback);
96-
9796
env->set_promise_reject_callback(args[0].As<Function>());
9897
}
9998

@@ -120,8 +119,8 @@ static void Initialize(Local<Object> target,
120119
FIXED_ONE_BYTE_STRING(isolate, "promiseRejectEvents"),
121120
events).FromJust();
122121
env->SetMethod(target,
123-
"initializePromiseRejectCallback",
124-
InitializePromiseRejectCallback);
122+
"setPromiseRejectCallback",
123+
SetPromiseRejectCallback);
125124
}
126125

127126
} // namespace task_queue
Collapse file

‎test/message/events_unhandled_error_nexttick.out‎

Copy file name to clipboardExpand all lines: test/message/events_unhandled_error_nexttick.out
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ Error
1515
at startup (internal/bootstrap/node.js:*:*)
1616
Emitted 'error' event at:
1717
at process.nextTick (*events_unhandled_error_nexttick.js:*:*)
18-
at internalTickCallback (internal/process/next_tick.js:*:*)
18+
at processTicksAndRejections (internal/process/next_tick.js:*:*)
1919
at process.runNextTicks [as _tickCallback] (internal/process/next_tick.js:*:*)
2020
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
2121
at executeUserCode (internal/bootstrap/node.js:*:*)
Collapse file

‎test/message/nexttick_throw.out‎

Copy file name to clipboardExpand all lines: test/message/nexttick_throw.out
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
^
55
ReferenceError: undefined_reference_error_maker is not defined
66
at *test*message*nexttick_throw.js:*:*
7-
at internalTickCallback (internal/process/next_tick.js:*:*)
7+
at processTicksAndRejections (internal/process/next_tick.js:*:*)
88
at process.runNextTicks [as _tickCallback] (internal/process/next_tick.js:*:*)
99
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
1010
at executeUserCode (internal/bootstrap/node.js:*:*)
Collapse file

‎test/message/stdin_messages.out‎

Copy file name to clipboardExpand all lines: test/message/stdin_messages.out
+4-4Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ SyntaxError: Strict mode code may not include a with statement
1212
at Socket.process.stdin.on (internal/bootstrap/node.js:*:*)
1313
at Socket.emit (events.js:*:*)
1414
at endReadableNT (_stream_readable.js:*:*)
15-
at process.internalTickCallback (internal/process/next_tick.js:*:*)
15+
at processTicksAndRejections (internal/process/next_tick.js:*:*)
1616
42
1717
42
1818
[stdin]:1
@@ -29,7 +29,7 @@ Error: hello
2929
at Socket.process.stdin.on (internal/bootstrap/node.js:*:*)
3030
at Socket.emit (events.js:*:*)
3131
at endReadableNT (_stream_readable.js:*:*)
32-
at process.internalTickCallback (internal/process/next_tick.js:*:*)
32+
at processTicksAndRejections (internal/process/next_tick.js:*:*)
3333
[stdin]:1
3434
throw new Error("hello")
3535
^
@@ -44,7 +44,7 @@ Error: hello
4444
at Socket.process.stdin.on (internal/bootstrap/node.js:*:*)
4545
at Socket.emit (events.js:*:*)
4646
at endReadableNT (_stream_readable.js:*:*)
47-
at process.internalTickCallback (internal/process/next_tick.js:*:*)
47+
at processTicksAndRejections (internal/process/next_tick.js:*:*)
4848
100
4949
[stdin]:1
5050
var x = 100; y = x;
@@ -60,7 +60,7 @@ ReferenceError: y is not defined
6060
at Socket.process.stdin.on (internal/bootstrap/node.js:*:*)
6161
at Socket.emit (events.js:*:*)
6262
at endReadableNT (_stream_readable.js:*:*)
63-
at process.internalTickCallback (internal/process/next_tick.js:*:*)
63+
at processTicksAndRejections (internal/process/next_tick.js:*:*)
6464

6565
[stdin]:1
6666
var ______________________________________________; throw 10

0 commit comments

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