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 abc4cc2

Browse filesBrowse files
joyeecheungaddaleax
joyeecheung
authored and
addaleax
committed
src: refactor tickInfo access
- Wrap access to tickInfo fields in functions - Rename `kHasScheduled` to `kHasTickScheduled` and `kHasPromiseRejections` to `kHasRejectionToWarn` for clarity - note the latter will be set to false if the rejection does not lead to a warning so the previous description is not accurate. - Set `kHasRejectionToWarn` in JS land of relying on C++ to use an implict contract (return value of the promise rejection handler) to set it, as the decision is made entirely in JS land. - Destructure promise reject event constants. 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 5b1a987 commit abc4cc2
Copy full SHA for abc4cc2

File tree

Expand file treeCollapse file tree

6 files changed

+63
-43
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

6 files changed

+63
-43
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
+15-7Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ const {
1111
} = internalBinding('task_queue');
1212

1313
const {
14+
setHasRejectionToWarn,
15+
hasRejectionToWarn,
1416
promiseRejectHandler,
1517
emitPromiseRejectionWarnings
1618
} = require('internal/process/promises');
@@ -30,15 +32,21 @@ const { ERR_INVALID_CALLBACK } = require('internal/errors').codes;
3032
const FixedQueue = require('internal/fixed_queue');
3133

3234
// *Must* match Environment::TickInfo::Fields in src/env.h.
33-
const kHasScheduled = 0;
34-
const kHasPromiseRejections = 1;
35+
const kHasTickScheduled = 0;
36+
37+
function hasTickScheduled() {
38+
return tickInfo[kHasTickScheduled] === 1;
39+
}
40+
function setHasTickScheduled(value) {
41+
tickInfo[kHasTickScheduled] = value ? 1 : 0;
42+
}
3543

3644
const queue = new FixedQueue();
3745

3846
function runNextTicks() {
39-
if (tickInfo[kHasScheduled] === 0 && tickInfo[kHasPromiseRejections] === 0)
47+
if (!hasTickScheduled() && !hasRejectionToWarn())
4048
runMicrotasks();
41-
if (tickInfo[kHasScheduled] === 0 && tickInfo[kHasPromiseRejections] === 0)
49+
if (!hasTickScheduled() && !hasRejectionToWarn())
4250
return;
4351

4452
internalTickCallback();
@@ -70,10 +78,10 @@ function internalTickCallback() {
7078

7179
emitAfter(asyncId);
7280
}
73-
tickInfo[kHasScheduled] = 0;
81+
setHasTickScheduled(false);
7482
runMicrotasks();
7583
} while (!queue.isEmpty() || emitPromiseRejectionWarnings());
76-
tickInfo[kHasPromiseRejections] = 0;
84+
setHasRejectionToWarn(false);
7785
}
7886

7987
class TickObject {
@@ -119,7 +127,7 @@ function nextTick(callback) {
119127
}
120128

121129
if (queue.isEmpty())
122-
tickInfo[kHasScheduled] = 1;
130+
setHasTickScheduled(true);
123131
queue.push(new TickObject(callback, args, getDefaultTriggerAsyncId()));
124132
}
125133

Collapse file

‎lib/internal/process/promises.js‎

Copy file name to clipboardExpand all lines: lib/internal/process/promises.js
+36-12Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,24 +2,45 @@
22

33
const { safeToString } = internalBinding('util');
44
const {
5-
promiseRejectEvents
5+
tickInfo,
6+
promiseRejectEvents: {
7+
kPromiseRejectWithNoHandler,
8+
kPromiseHandlerAddedAfterReject,
9+
kPromiseResolveAfterResolved,
10+
kPromiseRejectAfterResolved
11+
}
612
} = internalBinding('task_queue');
713

14+
// *Must* match Environment::TickInfo::Fields in src/env.h.
15+
const kHasRejectionToWarn = 1;
16+
817
const maybeUnhandledPromises = new WeakMap();
918
const pendingUnhandledRejections = [];
1019
const asyncHandledRejections = [];
1120
let lastPromiseId = 0;
1221

22+
function setHasRejectionToWarn(value) {
23+
tickInfo[kHasRejectionToWarn] = value ? 1 : 0;
24+
}
25+
26+
function hasRejectionToWarn() {
27+
return tickInfo[kHasRejectionToWarn] === 1;
28+
}
29+
1330
function promiseRejectHandler(type, promise, reason) {
1431
switch (type) {
15-
case promiseRejectEvents.kPromiseRejectWithNoHandler:
16-
return unhandledRejection(promise, reason);
17-
case promiseRejectEvents.kPromiseHandlerAddedAfterReject:
18-
return handledRejection(promise);
19-
case promiseRejectEvents.kPromiseResolveAfterResolved:
20-
return resolveError('resolve', promise, reason);
21-
case promiseRejectEvents.kPromiseRejectAfterResolved:
22-
return resolveError('reject', promise, reason);
32+
case kPromiseRejectWithNoHandler:
33+
unhandledRejection(promise, reason);
34+
break;
35+
case kPromiseHandlerAddedAfterReject:
36+
handledRejection(promise);
37+
break;
38+
case kPromiseResolveAfterResolved:
39+
resolveError('resolve', promise, reason);
40+
break;
41+
case kPromiseRejectAfterResolved:
42+
resolveError('reject', promise, reason);
43+
break;
2344
}
2445
}
2546

@@ -38,7 +59,7 @@ function unhandledRejection(promise, reason) {
3859
warned: false
3960
});
4061
pendingUnhandledRejections.push(promise);
41-
return true;
62+
setHasRejectionToWarn(true);
4263
}
4364

4465
function handledRejection(promise) {
@@ -54,10 +75,11 @@ function handledRejection(promise) {
5475
warning.name = 'PromiseRejectionHandledWarning';
5576
warning.id = uid;
5677
asyncHandledRejections.push({ promise, warning });
57-
return true;
78+
setHasRejectionToWarn(true);
79+
return;
5880
}
5981
}
60-
return false;
82+
setHasRejectionToWarn(false);
6183
}
6284

6385
const unhandledRejectionErrName = 'UnhandledPromiseRejectionWarning';
@@ -121,6 +143,8 @@ function emitPromiseRejectionWarnings() {
121143
}
122144

123145
module.exports = {
146+
hasRejectionToWarn,
147+
setHasRejectionToWarn,
124148
promiseRejectHandler,
125149
emitPromiseRejectionWarnings
126150
};
Collapse file

‎src/callback_scope.cc‎

Copy file name to clipboardExpand all lines: src/callback_scope.cc
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ void InternalCallbackScope::Close() {
9595
Environment::TickInfo* tick_info = env_->tick_info();
9696

9797
if (!env_->can_call_into_js()) return;
98-
if (!tick_info->has_scheduled()) {
98+
if (!tick_info->has_tick_scheduled()) {
9999
env_->isolate()->RunMicrotasks();
100100
}
101101

@@ -106,7 +106,7 @@ void InternalCallbackScope::Close() {
106106
CHECK_EQ(env_->trigger_async_id(), 0);
107107
}
108108

109-
if (!tick_info->has_scheduled() && !tick_info->has_promise_rejections()) {
109+
if (!tick_info->has_tick_scheduled() && !tick_info->has_rejection_to_warn()) {
110110
return;
111111
}
112112

Collapse file

‎src/env-inl.h‎

Copy file name to clipboardExpand all lines: src/env-inl.h
+4-8Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -265,16 +265,12 @@ inline AliasedBuffer<uint8_t, v8::Uint8Array>& Environment::TickInfo::fields() {
265265
return fields_;
266266
}
267267

268-
inline bool Environment::TickInfo::has_scheduled() const {
269-
return fields_[kHasScheduled] == 1;
268+
inline bool Environment::TickInfo::has_tick_scheduled() const {
269+
return fields_[kHasTickScheduled] == 1;
270270
}
271271

272-
inline bool Environment::TickInfo::has_promise_rejections() const {
273-
return fields_[kHasPromiseRejections] == 1;
274-
}
275-
276-
inline void Environment::TickInfo::promise_rejections_toggle_on() {
277-
fields_[kHasPromiseRejections] = 1;
272+
inline bool Environment::TickInfo::has_rejection_to_warn() const {
273+
return fields_[kHasRejectionToWarn] == 1;
278274
}
279275

280276
inline void Environment::AssignToContext(v8::Local<v8::Context> context,
Collapse file

‎src/env.h‎

Copy file name to clipboardExpand all lines: src/env.h
+4-6Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -572,18 +572,16 @@ class Environment {
572572
class TickInfo {
573573
public:
574574
inline AliasedBuffer<uint8_t, v8::Uint8Array>& fields();
575-
inline bool has_scheduled() const;
576-
inline bool has_promise_rejections() const;
577-
578-
inline void promise_rejections_toggle_on();
575+
inline bool has_tick_scheduled() const;
576+
inline bool has_rejection_to_warn() const;
579577

580578
private:
581579
friend class Environment; // So we can call the constructor.
582580
inline explicit TickInfo(v8::Isolate* isolate);
583581

584582
enum Fields {
585-
kHasScheduled,
586-
kHasPromiseRejections,
583+
kHasTickScheduled = 0,
584+
kHasRejectionToWarn,
587585
kFieldsCount
588586
};
589587

Collapse file

‎src/node_task_queue.cc‎

Copy file name to clipboardExpand all lines: src/node_task_queue.cc
+2-8Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ using v8::kPromiseRejectAfterResolved;
1717
using v8::kPromiseRejectWithNoHandler;
1818
using v8::kPromiseResolveAfterResolved;
1919
using v8::Local;
20-
using v8::MaybeLocal;
2120
using v8::Number;
2221
using v8::Object;
2322
using v8::Promise;
@@ -80,13 +79,8 @@ static void PromiseRejectCallback(PromiseRejectMessage message) {
8079
}
8180

8281
Local<Value> args[] = { type, promise, value };
83-
MaybeLocal<Value> ret = callback->Call(env->context(),
84-
Undefined(isolate),
85-
arraysize(args),
86-
args);
87-
88-
if (!ret.IsEmpty() && ret.ToLocalChecked()->IsTrue())
89-
env->tick_info()->promise_rejections_toggle_on();
82+
USE(callback->Call(
83+
env->context(), Undefined(isolate), arraysize(args), args));
9084
}
9185

9286
static void InitializePromiseRejectCallback(

0 commit comments

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