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 460cc62

Browse filesBrowse files
apapirovskiBridgeAR
apapirovski
authored and
BridgeAR
committed
process: code cleanup for nextTick
Fix a few edge cases and non-obvious issues with nextTick: 1. Emit destroy hook in a try-finally rather than triggering it before the callback runs. 2. Re-word comment for processPromiseRejections and make sure it returns true in the rejectionHandled case too. 3. Small readability improvements. PR-URL: #28047 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
1 parent d4bb88e commit 460cc62
Copy full SHA for 460cc62

File tree

Expand file treeCollapse file tree

3 files changed

+73
-29
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+73
-29
lines changed
Open diff view settings
Collapse file

‎lib/internal/process/promises.js‎

Copy file name to clipboardExpand all lines: lib/internal/process/promises.js
+8-5Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -139,17 +139,19 @@ function emitDeprecationWarning() {
139139
}
140140
}
141141

142-
// If this method returns true, at least one more tick need to be
143-
// scheduled to process any potential pending rejections
142+
// If this method returns true, we've executed user code or triggered
143+
// a warning to be emitted which requires the microtask and next tick
144+
// queues to be drained again.
144145
function processPromiseRejections() {
146+
let maybeScheduledTicksOrMicrotasks = asyncHandledRejections.length > 0;
147+
145148
while (asyncHandledRejections.length > 0) {
146149
const { promise, warning } = asyncHandledRejections.shift();
147150
if (!process.emit('rejectionHandled', promise)) {
148151
process.emitWarning(warning);
149152
}
150153
}
151154

152-
let maybeScheduledTicks = false;
153155
let len = pendingUnhandledRejections.length;
154156
while (len--) {
155157
const promise = pendingUnhandledRejections.shift();
@@ -167,9 +169,10 @@ function processPromiseRejections() {
167169
state === states.warn) {
168170
emitWarning(uid, reason);
169171
}
170-
maybeScheduledTicks = true;
172+
maybeScheduledTicksOrMicrotasks = true;
171173
}
172-
return maybeScheduledTicks || pendingUnhandledRejections.length !== 0;
174+
return maybeScheduledTicksOrMicrotasks ||
175+
pendingUnhandledRejections.length !== 0;
173176
}
174177

175178
function getError(name, message) {
Collapse file

‎lib/internal/process/task_queues.js‎

Copy file name to clipboardExpand all lines: lib/internal/process/task_queues.js
+16-24Lines changed: 16 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -65,46 +65,38 @@ function processTicksAndRejections() {
6565
while (tock = queue.shift()) {
6666
const asyncId = tock[async_id_symbol];
6767
emitBefore(asyncId, tock[trigger_async_id_symbol]);
68-
// emitDestroy() places the async_id_symbol into an asynchronous queue
69-
// that calls the destroy callback in the future. It's called before
70-
// calling tock.callback so destroy will be called even if the callback
71-
// throws an exception that is handled by 'uncaughtException' or a
72-
// domain.
73-
// TODO(trevnorris): This is a bit of a hack. It relies on the fact
74-
// that nextTick() doesn't allow the event loop to proceed, but if
75-
// any async hooks are enabled during the callback's execution then
76-
// this tock's after hook will be called, but not its destroy hook.
77-
if (destroyHooksExist())
78-
emitDestroy(asyncId);
79-
80-
const callback = tock.callback;
81-
if (tock.args === undefined)
82-
callback();
83-
else
84-
callback(...tock.args);
68+
69+
try {
70+
const callback = tock.callback;
71+
if (tock.args === undefined)
72+
callback();
73+
else
74+
callback(...tock.args);
75+
} finally {
76+
if (destroyHooksExist())
77+
emitDestroy(asyncId);
78+
}
8579

8680
emitAfter(asyncId);
8781
}
88-
setHasTickScheduled(false);
8982
runMicrotasks();
9083
} while (!queue.isEmpty() || processPromiseRejections());
84+
setHasTickScheduled(false);
9185
setHasRejectionToWarn(false);
9286
}
9387

9488
class TickObject {
95-
constructor(callback, args, triggerAsyncId) {
89+
constructor(callback, args) {
9690
this.callback = callback;
9791
this.args = args;
9892

9993
const asyncId = newAsyncId();
94+
const triggerAsyncId = getDefaultTriggerAsyncId();
10095
this[async_id_symbol] = asyncId;
10196
this[trigger_async_id_symbol] = triggerAsyncId;
10297

10398
if (initHooksExist()) {
104-
emitInit(asyncId,
105-
'TickObject',
106-
triggerAsyncId,
107-
this);
99+
emitInit(asyncId, 'TickObject', triggerAsyncId, this);
108100
}
109101
}
110102
}
@@ -132,7 +124,7 @@ function nextTick(callback) {
132124

133125
if (queue.isEmpty())
134126
setHasTickScheduled(true);
135-
queue.push(new TickObject(callback, args, getDefaultTriggerAsyncId()));
127+
queue.push(new TickObject(callback, args));
136128
}
137129

138130
let AsyncResource;
Collapse file

‎test/async-hooks/test-late-hook-enable.js‎

Copy file name to clipboard
+49Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const async_hooks = require('async_hooks');
5+
6+
// Checks that enabling async hooks in a callback actually
7+
// triggers after & destroy as expected.
8+
9+
const fnsToTest = [setTimeout, (cb) => {
10+
setImmediate(() => {
11+
cb();
12+
13+
// We need to keep the event loop open for this to actually work
14+
// since destroy hooks are triggered in unrefed Immediates
15+
setImmediate(() => {
16+
hook.disable();
17+
});
18+
});
19+
}, (cb) => {
20+
process.nextTick(() => {
21+
cb();
22+
23+
// We need to keep the event loop open for this to actually work
24+
// since destroy hooks are triggered in unrefed Immediates
25+
setImmediate(() => {
26+
hook.disable();
27+
assert.strictEqual(fnsToTest.length, 0);
28+
});
29+
});
30+
}];
31+
32+
const hook = async_hooks.createHook({
33+
before: common.mustNotCall(),
34+
after: common.mustCall(() => {}, 3),
35+
destroy: common.mustCall(() => {
36+
hook.disable();
37+
nextTest();
38+
}, 3)
39+
});
40+
41+
nextTest();
42+
43+
function nextTest() {
44+
if (fnsToTest.length > 0) {
45+
fnsToTest.shift()(common.mustCall(() => {
46+
hook.enable();
47+
}));
48+
}
49+
}

0 commit comments

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