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 61c7308

Browse filesBrowse files
committed
async_hooks: minor refactor to callback invocation
Re-use the `init` function wherever possible, and move `try { … } catch` blocks that result in fatal errors to a larger scope. Also make the argument order for `init()` consistent in the codebase. PR-URL: #13419 Reviewed-By: Trevor Norris <trev.norris@gmail.com> Reviewed-By: Andreas Madsen <amwebdk@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>
1 parent c8db047 commit 61c7308
Copy full SHA for 61c7308

File tree

Expand file treeCollapse file tree

2 files changed

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

2 files changed

+42
-63
lines changed
Open diff view settings
Collapse file

‎lib/async_hooks.js‎

Copy file name to clipboardExpand all lines: lib/async_hooks.js
+41-62Lines changed: 41 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -214,17 +214,7 @@ class AsyncResource {
214214
if (async_hook_fields[kInit] === 0)
215215
return;
216216

217-
processing_hook = true;
218-
for (var i = 0; i < active_hooks_array.length; i++) {
219-
if (typeof active_hooks_array[i][init_symbol] === 'function') {
220-
runInitCallback(active_hooks_array[i][init_symbol],
221-
this[async_id_symbol],
222-
type,
223-
triggerId,
224-
this);
225-
}
226-
}
227-
processing_hook = false;
217+
init(this[async_id_symbol], type, triggerId, this);
228218
}
229219

230220
emitBefore() {
@@ -321,14 +311,7 @@ function emitInitS(asyncId, type, triggerId, resource) {
321311
if (!Number.isSafeInteger(triggerId) || triggerId < 0)
322312
throw new RangeError('triggerId must be an unsigned integer');
323313

324-
processing_hook = true;
325-
for (var i = 0; i < active_hooks_array.length; i++) {
326-
if (typeof active_hooks_array[i][init_symbol] === 'function') {
327-
runInitCallback(
328-
active_hooks_array[i][init_symbol], asyncId, type, triggerId, resource);
329-
}
330-
}
331-
processing_hook = false;
314+
init(asyncId, type, triggerId, resource);
332315

333316
// Isn't null if hooks were added/removed while the hooks were running.
334317
if (tmp_active_hooks_array !== null) {
@@ -339,10 +322,15 @@ function emitInitS(asyncId, type, triggerId, resource) {
339322

340323
function emitBeforeN(asyncId) {
341324
processing_hook = true;
342-
for (var i = 0; i < active_hooks_array.length; i++) {
343-
if (typeof active_hooks_array[i][before_symbol] === 'function') {
344-
runCallback(active_hooks_array[i][before_symbol], asyncId);
325+
// Use a single try/catch for all hook to avoid setting up one per iteration.
326+
try {
327+
for (var i = 0; i < active_hooks_array.length; i++) {
328+
if (typeof active_hooks_array[i][before_symbol] === 'function') {
329+
active_hooks_array[i][before_symbol](asyncId);
330+
}
345331
}
332+
} catch (e) {
333+
fatalError(e);
346334
}
347335
processing_hook = false;
348336

@@ -366,26 +354,27 @@ function emitBeforeS(asyncId, triggerId = asyncId) {
366354

367355
pushAsyncIds(asyncId, triggerId);
368356

369-
if (async_hook_fields[kBefore] === 0) {
357+
if (async_hook_fields[kBefore] === 0)
370358
return;
371-
}
372-
373359
emitBeforeN(asyncId);
374360
}
375361

376362

377363
// Called from native. The asyncId stack handling is taken care of there before
378364
// this is called.
379365
function emitAfterN(asyncId) {
380-
if (async_hook_fields[kAfter] > 0) {
381-
processing_hook = true;
366+
processing_hook = true;
367+
// Use a single try/catch for all hook to avoid setting up one per iteration.
368+
try {
382369
for (var i = 0; i < active_hooks_array.length; i++) {
383370
if (typeof active_hooks_array[i][after_symbol] === 'function') {
384-
runCallback(active_hooks_array[i][after_symbol], asyncId);
371+
active_hooks_array[i][after_symbol](asyncId);
385372
}
386373
}
387-
processing_hook = false;
374+
} catch (e) {
375+
fatalError(e);
388376
}
377+
processing_hook = false;
389378

390379
if (tmp_active_hooks_array !== null) {
391380
restoreTmpHooks();
@@ -397,7 +386,9 @@ function emitAfterN(asyncId) {
397386
// kIdStackIndex. But what happens if the user doesn't have both before and
398387
// after callbacks.
399388
function emitAfterS(asyncId) {
400-
emitAfterN(asyncId);
389+
if (async_hook_fields[kAfter] > 0)
390+
emitAfterN(asyncId);
391+
401392
popAsyncIds(asyncId);
402393
}
403394

@@ -413,10 +404,15 @@ function emitDestroyS(asyncId) {
413404

414405
function emitDestroyN(asyncId) {
415406
processing_hook = true;
416-
for (var i = 0; i < active_hooks_array.length; i++) {
417-
if (typeof active_hooks_array[i][destroy_symbol] === 'function') {
418-
runCallback(active_hooks_array[i][destroy_symbol], asyncId);
407+
// Use a single try/catch for all hook to avoid setting up one per iteration.
408+
try {
409+
for (var i = 0; i < active_hooks_array.length; i++) {
410+
if (typeof active_hooks_array[i][destroy_symbol] === 'function') {
411+
active_hooks_array[i][destroy_symbol](asyncId);
412+
}
419413
}
414+
} catch (e) {
415+
fatalError(e);
420416
}
421417
processing_hook = false;
422418

@@ -434,41 +430,24 @@ function emitDestroyN(asyncId) {
434430
// init().
435431
// TODO(trevnorris): Perhaps have MakeCallback call a single JS function that
436432
// does the before/callback/after calls to remove two additional calls to JS.
437-
function init(asyncId, type, resource, triggerId) {
438-
processing_hook = true;
439-
for (var i = 0; i < active_hooks_array.length; i++) {
440-
if (typeof active_hooks_array[i][init_symbol] === 'function') {
441-
runInitCallback(
442-
active_hooks_array[i][init_symbol], asyncId, type, triggerId, resource);
443-
}
444-
}
445-
processing_hook = false;
446-
}
447-
448-
449-
// Generalized callers for all callbacks that handles error handling.
450433

451-
// If either runInitCallback() or runCallback() throw then force the
452-
// application to shutdown if one of the callbacks throws. This may change in
453-
// the future depending on whether it can be determined if there's a slim
454-
// chance of the application remaining stable after handling one of these
434+
// Force the application to shutdown if one of the callbacks throws. This may
435+
// change in the future depending on whether it can be determined if there's a
436+
// slim chance of the application remaining stable after handling one of these
455437
// exceptions.
456-
457-
function runInitCallback(cb, asyncId, type, triggerId, resource) {
458-
try {
459-
cb(asyncId, type, triggerId, resource);
460-
} catch (e) {
461-
fatalError(e);
462-
}
463-
}
464-
465-
466-
function runCallback(cb, asyncId) {
438+
function init(asyncId, type, triggerId, resource) {
439+
processing_hook = true;
440+
// Use a single try/catch for all hook to avoid setting up one per iteration.
467441
try {
468-
cb(asyncId);
442+
for (var i = 0; i < active_hooks_array.length; i++) {
443+
if (typeof active_hooks_array[i][init_symbol] === 'function') {
444+
active_hooks_array[i][init_symbol](asyncId, type, triggerId, resource);
445+
}
446+
}
469447
} catch (e) {
470448
fatalError(e);
471449
}
450+
processing_hook = false;
472451
}
473452

474453

Collapse file

‎src/async-wrap.cc‎

Copy file name to clipboardExpand all lines: src/async-wrap.cc
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -560,8 +560,8 @@ void AsyncWrap::EmitAsyncInit(Environment* env,
560560
Local<Value> argv[] = {
561561
Number::New(env->isolate(), async_id),
562562
type,
563-
object,
564563
Number::New(env->isolate(), trigger_id),
564+
object,
565565
};
566566

567567
TryCatch try_catch(env->isolate());

0 commit comments

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