diff --git a/lib/async_hooks.js b/lib/async_hooks.js index a730738c0f9144..4056fe1635f7c8 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -26,10 +26,10 @@ const { pushAsyncIds, popAsyncIds } = async_wrap; // Using var instead of (preferably const) in order to assign // tmp_active_hooks_array if a hook is enabled/disabled during hook execution. var active_hooks_array = []; -// Track whether a hook callback is currently being processed. Used to make -// sure active_hooks_array isn't altered in mid execution if another hook is -// added or removed. -var processing_hook = false; +// Use a counter to track whether a hook callback is currently being processed. +// Used to make sure active_hooks_array isn't altered in mid execution if +// another hook is added or removed. A counter is used to track nested calls. +var processing_hook = 0; // Use to temporarily store and updated active_hooks_array if the user enables // or disables a hook while hooks are being processed. var tmp_active_hooks_array = null; @@ -151,7 +151,7 @@ class AsyncHook { function getHookArrays() { - if (!processing_hook) + if (processing_hook === 0) return [active_hooks_array, async_hook_fields]; // If this hook is being enabled while in the middle of processing the array // of currently active hooks then duplicate the current set of active hooks @@ -335,11 +335,6 @@ function emitInitS(asyncId, type, triggerAsyncId, resource) { throw new RangeError('triggerAsyncId must be an unsigned integer'); init(asyncId, type, triggerAsyncId, resource); - - // Isn't null if hooks were added/removed while the hooks were running. - if (tmp_active_hooks_array !== null) { - restoreTmpHooks(); - } } function emitHookFactory(symbol, name) { @@ -347,7 +342,7 @@ function emitHookFactory(symbol, name) { // before this is called. // eslint-disable-next-line func-style const fn = function(asyncId) { - processing_hook = true; + processing_hook += 1; // Use a single try/catch for all hook to avoid setting up one per // iteration. try { @@ -358,10 +353,11 @@ function emitHookFactory(symbol, name) { } } catch (e) { fatalError(e); + } finally { + processing_hook -= 1; } - processing_hook = false; - if (tmp_active_hooks_array !== null) { + if (processing_hook === 0 && tmp_active_hooks_array !== null) { restoreTmpHooks(); } }; @@ -427,7 +423,7 @@ function emitDestroyS(asyncId) { // slim chance of the application remaining stable after handling one of these // exceptions. function init(asyncId, type, triggerAsyncId, resource) { - processing_hook = true; + processing_hook += 1; // Use a single try/catch for all hook to avoid setting up one per iteration. try { for (var i = 0; i < active_hooks_array.length; i++) { @@ -440,8 +436,20 @@ function init(asyncId, type, triggerAsyncId, resource) { } } catch (e) { fatalError(e); + } finally { + processing_hook -= 1; + } + + // * `tmp_active_hooks_array` is null if no hooks were added/removed while + // the hooks were running. In that case no restoration is needed. + // * In the case where another hook was added/removed while the hooks were + // running and a handle was created causing the `init` hooks to fire again, + // then `restoreTmpHooks` should not be called for the nested `hooks`. + // Otherwise `active_hooks_array` can change during execution of the + // `hooks`. + if (processing_hook === 0 && tmp_active_hooks_array !== null) { + restoreTmpHooks(); } - processing_hook = false; } diff --git a/test/async-hooks/test-disable-in-init.js b/test/async-hooks/test-disable-in-init.js new file mode 100644 index 00000000000000..21588bf504651f --- /dev/null +++ b/test/async-hooks/test-disable-in-init.js @@ -0,0 +1,23 @@ +'use strict'; + +const common = require('../common'); +const async_hooks = require('async_hooks'); +const fs = require('fs'); + +let nestedCall = false; + +async_hooks.createHook({ + init: common.mustCall(function(id, type) { + nestedHook.disable(); + if (!nestedCall) { + nestedCall = true; + fs.access(__filename, common.mustCall()); + } + }, 2) +}).enable(); + +const nestedHook = async_hooks.createHook({ + init: common.mustCall(2) +}).enable(); + +fs.access(__filename, common.mustCall()); diff --git a/test/async-hooks/test-enable-in-init.js b/test/async-hooks/test-enable-in-init.js new file mode 100644 index 00000000000000..e4be9f9f87e3eb --- /dev/null +++ b/test/async-hooks/test-enable-in-init.js @@ -0,0 +1,22 @@ +'use strict'; + +const common = require('../common'); +const async_hooks = require('async_hooks'); +const fs = require('fs'); + +const nestedHook = async_hooks.createHook({ + init: common.mustNotCall() +}); +let nestedCall = false; + +async_hooks.createHook({ + init: common.mustCall(function(id, type) { + nestedHook.enable(); + if (!nestedCall) { + nestedCall = true; + fs.access(__filename, common.mustCall()); + } + }, 2) +}).enable(); + +fs.access(__filename, common.mustCall()); diff --git a/test/parallel/test-async-hooks-enable-recursive.js b/test/parallel/test-async-hooks-enable-recursive.js new file mode 100644 index 00000000000000..bcb0dcc0ce0a66 --- /dev/null +++ b/test/parallel/test-async-hooks-enable-recursive.js @@ -0,0 +1,19 @@ +'use strict'; + +const common = require('../common'); +const async_hooks = require('async_hooks'); +const fs = require('fs'); + +const nestedHook = async_hooks.createHook({ + init: common.mustCall() +}); + +async_hooks.createHook({ + init: common.mustCall((id, type) => { + nestedHook.enable(); + }, 2) +}).enable(); + +fs.access(__filename, common.mustCall(() => { + fs.access(__filename, common.mustCall()); +}));