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 9d26358

Browse filesBrowse files
apapirovskiMylesBorins
apapirovski
authored and
MylesBorins
committed
async_hooks: remove internal only error checking
This error checking is mostly unnecessary and is just a Node core developer nicety, rather than something that is needed for the user-land. It can be safely removed without any practical impact while making nextTick, timers, immediates and AsyncResource substantially faster. PR-URL: #30967 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
1 parent d48f592 commit 9d26358
Copy full SHA for 9d26358

File tree

Expand file treeCollapse file tree

4 files changed

+26
-110
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

4 files changed

+26
-110
lines changed
Open diff view settings
Collapse file

‎lib/async_hooks.js‎

Copy file name to clipboardExpand all lines: lib/async_hooks.js
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const {
88

99
const {
1010
ERR_ASYNC_CALLBACK,
11+
ERR_ASYNC_TYPE,
1112
ERR_INVALID_ASYNC_ID
1213
} = require('internal/errors').codes;
1314
const { validateString } = require('internal/validators');
@@ -31,6 +32,7 @@ const {
3132
emitBefore,
3233
emitAfter,
3334
emitDestroy,
35+
enabledHooksExist,
3436
initHooksExist,
3537
} = internal_async_hooks;
3638

@@ -157,6 +159,10 @@ class AsyncResource {
157159
this[trigger_async_id_symbol] = triggerAsyncId;
158160

159161
if (initHooksExist()) {
162+
if (enabledHooksExist() && type.length === 0) {
163+
throw new ERR_ASYNC_TYPE(type);
164+
}
165+
160166
emitInit(asyncId, type, triggerAsyncId, this);
161167
}
162168

Collapse file

‎lib/internal/async_hooks.js‎

Copy file name to clipboardExpand all lines: lib/internal/async_hooks.js
+5-37Lines changed: 5 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,10 @@
33
const {
44
Error,
55
FunctionPrototypeBind,
6-
NumberIsSafeInteger,
76
ObjectDefineProperty,
87
Symbol,
98
} = primordials;
109

11-
const {
12-
ERR_ASYNC_TYPE,
13-
ERR_INVALID_ASYNC_ID
14-
} = require('internal/errors').codes;
15-
1610
const async_wrap = internalBinding('async_wrap');
1711
/* async_hook_fields is a Uint32Array wrapping the uint32_t array of
1812
* Environment::AsyncHooks::fields_[]. Each index tracks the number of active
@@ -117,15 +111,6 @@ function fatalError(e) {
117111
}
118112

119113

120-
function validateAsyncId(asyncId, type) {
121-
// Skip validation when async_hooks is disabled
122-
if (async_hook_fields[kCheck] <= 0) return;
123-
124-
if (!NumberIsSafeInteger(asyncId) || asyncId < -1) {
125-
fatalError(new ERR_INVALID_ASYNC_ID(type, asyncId));
126-
}
127-
}
128-
129114
// Emit From Native //
130115

131116
// Used by C++ to call all init() callbacks. Because some state can be setup
@@ -314,6 +299,9 @@ function defaultTriggerAsyncIdScope(triggerAsyncId, block, ...args) {
314299
}
315300
}
316301

302+
function enabledHooksExist() {
303+
return async_hook_fields[kCheck] > 0;
304+
}
317305

318306
function initHooksExist() {
319307
return async_hook_fields[kInit] > 0;
@@ -329,21 +317,11 @@ function destroyHooksExist() {
329317

330318

331319
function emitInitScript(asyncId, type, triggerAsyncId, resource) {
332-
validateAsyncId(asyncId, 'asyncId');
333-
if (triggerAsyncId !== null)
334-
validateAsyncId(triggerAsyncId, 'triggerAsyncId');
335-
if (async_hook_fields[kCheck] > 0 &&
336-
(typeof type !== 'string' || type.length <= 0)) {
337-
throw new ERR_ASYNC_TYPE(type);
338-
}
339-
340320
// Short circuit all checks for the common case. Which is that no hooks have
341321
// been set. Do this to remove performance impact for embedders (and core).
342322
if (async_hook_fields[kInit] === 0)
343323
return;
344324

345-
// This can run after the early return check b/c running this function
346-
// manually means that the embedder must have used getDefaultTriggerAsyncId().
347325
if (triggerAsyncId === null) {
348326
triggerAsyncId = getDefaultTriggerAsyncId();
349327
}
@@ -353,12 +331,6 @@ function emitInitScript(asyncId, type, triggerAsyncId, resource) {
353331

354332

355333
function emitBeforeScript(asyncId, triggerAsyncId) {
356-
// Validate the ids. An id of -1 means it was never set and is visible on the
357-
// call graph. An id < -1 should never happen in any circumstance. Throw
358-
// on user calls because async state should still be recoverable.
359-
validateAsyncId(asyncId, 'asyncId');
360-
validateAsyncId(triggerAsyncId, 'triggerAsyncId');
361-
362334
pushAsyncIds(asyncId, triggerAsyncId);
363335

364336
if (async_hook_fields[kBefore] > 0)
@@ -367,8 +339,6 @@ function emitBeforeScript(asyncId, triggerAsyncId) {
367339

368340

369341
function emitAfterScript(asyncId) {
370-
validateAsyncId(asyncId, 'asyncId');
371-
372342
if (async_hook_fields[kAfter] > 0)
373343
emitAfterNative(asyncId);
374344

@@ -377,8 +347,6 @@ function emitAfterScript(asyncId) {
377347

378348

379349
function emitDestroyScript(asyncId) {
380-
validateAsyncId(asyncId, 'asyncId');
381-
382350
// Return early if there are no destroy callbacks, or invalid asyncId.
383351
if (async_hook_fields[kDestroy] === 0 || asyncId <= 0)
384352
return;
@@ -418,8 +386,7 @@ function popAsyncIds(asyncId) {
418386
const stackLength = async_hook_fields[kStackLength];
419387
if (stackLength === 0) return false;
420388

421-
if (async_hook_fields[kCheck] > 0 &&
422-
async_id_fields[kExecutionAsyncId] !== asyncId) {
389+
if (enabledHooksExist() && async_id_fields[kExecutionAsyncId] !== asyncId) {
423390
// Do the same thing as the native code (i.e. crash hard).
424391
return popAsyncIds_(asyncId);
425392
}
@@ -464,6 +431,7 @@ module.exports = {
464431
getOrSetAsyncId,
465432
getDefaultTriggerAsyncId,
466433
defaultTriggerAsyncIdScope,
434+
enabledHooksExist,
467435
initHooksExist,
468436
afterHooksExist,
469437
destroyHooksExist,
Collapse file

‎test/async-hooks/test-emit-before-after.js‎

Copy file name to clipboardExpand all lines: test/async-hooks/test-emit-before-after.js
+15-33Lines changed: 15 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -3,40 +3,9 @@
33

44
const common = require('../common');
55
const assert = require('assert');
6-
const spawnSync = require('child_process').spawnSync;
76
const async_hooks = require('internal/async_hooks');
87
const initHooks = require('./init-hooks');
98

10-
if (!common.isMainThread)
11-
common.skip('Worker bootstrapping works differently -> different async IDs');
12-
13-
switch (process.argv[2]) {
14-
case 'test_invalid_async_id':
15-
async_hooks.emitBefore(-2, 1);
16-
return;
17-
case 'test_invalid_trigger_id':
18-
async_hooks.emitBefore(1, -2);
19-
return;
20-
}
21-
assert.ok(!process.argv[2]);
22-
23-
24-
const c1 = spawnSync(process.execPath, [
25-
'--expose-internals', __filename, 'test_invalid_async_id'
26-
]);
27-
assert.strictEqual(
28-
c1.stderr.toString().split(/[\r\n]+/g)[0],
29-
'RangeError [ERR_INVALID_ASYNC_ID]: Invalid asyncId value: -2');
30-
assert.strictEqual(c1.status, 1);
31-
32-
const c2 = spawnSync(process.execPath, [
33-
'--expose-internals', __filename, 'test_invalid_trigger_id'
34-
]);
35-
assert.strictEqual(
36-
c2.stderr.toString().split(/[\r\n]+/g)[0],
37-
'RangeError [ERR_INVALID_ASYNC_ID]: Invalid triggerAsyncId value: -2');
38-
assert.strictEqual(c2.status, 1);
39-
409
const expectedId = async_hooks.newAsyncId();
4110
const expectedTriggerId = async_hooks.newAsyncId();
4211
const expectedType = 'test_emit_before_after_type';
@@ -45,9 +14,22 @@ const expectedType = 'test_emit_before_after_type';
4514
async_hooks.emitBefore(expectedId, expectedTriggerId);
4615
async_hooks.emitAfter(expectedId);
4716

17+
const chkBefore = common.mustCall((id) => assert.strictEqual(id, expectedId));
18+
const chkAfter = common.mustCall((id) => assert.strictEqual(id, expectedId));
19+
20+
const checkOnce = (fn) => {
21+
let called = false;
22+
return (...args) => {
23+
if (called) return;
24+
25+
called = true;
26+
fn(...args);
27+
};
28+
};
29+
4830
initHooks({
49-
onbefore: common.mustCall((id) => assert.strictEqual(id, expectedId)),
50-
onafter: common.mustCall((id) => assert.strictEqual(id, expectedId)),
31+
onbefore: checkOnce(chkBefore),
32+
onafter: checkOnce(chkAfter),
5133
allowNoInit: true
5234
}).enable();
5335

Collapse file

‎test/async-hooks/test-emit-init.js‎

Copy file name to clipboardExpand all lines: test/async-hooks/test-emit-init.js
-40Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
const common = require('../common');
55
const assert = require('assert');
6-
const spawnSync = require('child_process').spawnSync;
76
const async_hooks = require('internal/async_hooks');
87
const initHooks = require('./init-hooks');
98

@@ -23,45 +22,6 @@ const hooks1 = initHooks({
2322

2423
hooks1.enable();
2524

26-
switch (process.argv[2]) {
27-
case 'test_invalid_async_id':
28-
async_hooks.emitInit();
29-
return;
30-
case 'test_invalid_trigger_id':
31-
async_hooks.emitInit(expectedId);
32-
return;
33-
case 'test_invalid_trigger_id_negative':
34-
async_hooks.emitInit(expectedId, expectedType, -2);
35-
return;
36-
}
37-
assert.ok(!process.argv[2]);
38-
39-
40-
const c1 = spawnSync(process.execPath, [
41-
'--expose-internals', __filename, 'test_invalid_async_id'
42-
]);
43-
assert.strictEqual(
44-
c1.stderr.toString().split(/[\r\n]+/g)[0],
45-
'RangeError [ERR_INVALID_ASYNC_ID]: Invalid asyncId value: undefined');
46-
assert.strictEqual(c1.status, 1);
47-
48-
const c2 = spawnSync(process.execPath, [
49-
'--expose-internals', __filename, 'test_invalid_trigger_id'
50-
]);
51-
assert.strictEqual(
52-
c2.stderr.toString().split(/[\r\n]+/g)[0],
53-
'RangeError [ERR_INVALID_ASYNC_ID]: Invalid triggerAsyncId value: undefined');
54-
assert.strictEqual(c2.status, 1);
55-
56-
const c3 = spawnSync(process.execPath, [
57-
'--expose-internals', __filename, 'test_invalid_trigger_id_negative'
58-
]);
59-
assert.strictEqual(
60-
c3.stderr.toString().split(/[\r\n]+/g)[0],
61-
'RangeError [ERR_INVALID_ASYNC_ID]: Invalid triggerAsyncId value: -2');
62-
assert.strictEqual(c3.status, 1);
63-
64-
6525
async_hooks.emitInit(expectedId, expectedType, expectedTriggerId,
6626
expectedResource);
6727

0 commit comments

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