From b4aa8f6778e86876db2e19c88a92a48c76ed1e49 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Fri, 4 Oct 2019 17:46:59 -0400 Subject: [PATCH 01/10] async_hooks: buffer bootstrap events --- doc/api/async_hooks.md | 18 +++- lib/async_hooks.js | 5 ++ lib/internal/async_hooks.js | 112 ++++++++++++++++++++++-- lib/internal/bootstrap/pre_execution.js | 5 ++ lib/internal/main/run_main_module.js | 32 +++++-- lib/internal/modules/cjs/loader.js | 18 ---- lib/internal/modules/esm/module_job.js | 10 ++- test/es-module/test-esm-async-hooks.mjs | 47 ++++++++++ 8 files changed, 210 insertions(+), 37 deletions(-) create mode 100644 test/es-module/test-esm-async-hooks.mjs diff --git a/doc/api/async_hooks.md b/doc/api/async_hooks.md index 90ff343ae0817c..795b80f203c5ed 100644 --- a/doc/api/async_hooks.md +++ b/doc/api/async_hooks.md @@ -107,7 +107,7 @@ specifics of all functions that can be passed to `callbacks` is in the const async_hooks = require('async_hooks'); const asyncHook = async_hooks.createHook({ - init(asyncId, type, triggerAsyncId, resource) { }, + init(asyncId, type, triggerAsyncId, resource, bootstrap) { }, destroy(asyncId) { } }); ``` @@ -116,7 +116,7 @@ The callbacks will be inherited via the prototype chain: ```js class MyAsyncCallbacks { - init(asyncId, type, triggerAsyncId, resource) { } + init(asyncId, type, triggerAsyncId, resource, bootstrap) { } destroy(asyncId) {} } @@ -203,7 +203,7 @@ Key events in the lifetime of asynchronous events have been categorized into four areas: instantiation, before/after the callback is called, and when the instance is destroyed. -##### init(asyncId, type, triggerAsyncId, resource) +##### init(asyncId, type, triggerAsyncId, resource, bootstrap) * `asyncId` {number} A unique ID for the async resource. * `type` {string} The type of the async resource. @@ -211,6 +211,8 @@ instance is destroyed. execution context this async resource was created. * `resource` {Object} Reference to the resource representing the async operation, needs to be released during _destroy_. +* `bootstrap` {boolean} Indicates whether this event was created during Node.js + bootstrap. Called when a class is constructed that has the _possibility_ to emit an asynchronous event. This _does not_ mean the instance must call @@ -319,8 +321,13 @@ elaborate to make calling context easier to see. ```js let indent = 0; +const bootstrapIds = new Set(); async_hooks.createHook({ - init(asyncId, type, triggerAsyncId) { + init(asyncId, type, triggerAsyncId, bootstrap) { + if (bootstrap) { + bootstrapIds.add(asyncId); + return; + } const eid = async_hooks.executionAsyncId(); const indentStr = ' '.repeat(indent); fs.writeSync( @@ -329,18 +336,21 @@ async_hooks.createHook({ ` trigger: ${triggerAsyncId} execution: ${eid}\n`); }, before(asyncId) { + if (bootstrapIds.has(asyncId)) return; const indentStr = ' '.repeat(indent); fs.writeFileSync('log.out', `${indentStr}before: ${asyncId}\n`, { flag: 'a' }); indent += 2; }, after(asyncId) { + if (bootstrapIds.has(asyncId)) return; indent -= 2; const indentStr = ' '.repeat(indent); fs.writeFileSync('log.out', `${indentStr}after: ${asyncId}\n`, { flag: 'a' }); }, destroy(asyncId) { + if (bootstrapIds.has(asyncId)) return; const indentStr = ' '.repeat(indent); fs.writeFileSync('log.out', `${indentStr}destroy: ${asyncId}\n`, { flag: 'a' }); diff --git a/lib/async_hooks.js b/lib/async_hooks.js index 0c177055364e41..0b73eb1f0530db 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -28,6 +28,7 @@ const { emitAfter, emitDestroy, initHooksExist, + emitBootstrapHooksBuffer } = internal_async_hooks; // Get symbols @@ -92,6 +93,10 @@ class AsyncHook { enableHooks(); } + // If there are unemitted bootstrap hooks, emit them now. + // This only applies during sync execution of user code after bootsrap. + emitBootstrapHooksBuffer(); + return this; } diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index bf803885cacfa3..8038a6b0b00b0c 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -125,7 +125,8 @@ function validateAsyncId(asyncId, type) { // Used by C++ to call all init() callbacks. Because some state can be setup // from C++ there's no need to perform all the same operations as in // emitInitScript. -function emitInitNative(asyncId, type, triggerAsyncId, resource) { +function emitInitNative(asyncId, type, triggerAsyncId, resource, + bootstrap = false) { active_hooks.call_depth += 1; // Use a single try/catch for all hooks to avoid setting up one per iteration. try { @@ -133,7 +134,7 @@ function emitInitNative(asyncId, type, triggerAsyncId, resource) { if (typeof active_hooks.array[i][init_symbol] === 'function') { active_hooks.array[i][init_symbol]( asyncId, type, triggerAsyncId, - resource + resource, bootstrap ); } } @@ -231,6 +232,103 @@ function restoreActiveHooks() { active_hooks.tmp_fields = null; } +// Bootstrap hooks are buffered to emit to userland listeners +let bootstrapHooks, bootstrapBuffer; +function bufferBootstrapHooks() { + const { getOptionValue } = require('internal/options'); + if (getOptionValue('--no-force-async-hooks-checks')) { + return; + } + async_hook_fields[kInit]++; + async_hook_fields[kBefore]++; + async_hook_fields[kAfter]++; + async_hook_fields[kDestroy]++; + async_hook_fields[kPromiseResolve]++; + async_hook_fields[kTotals] += 5; + bootstrapHooks = { + [init_symbol]: (asyncId, type, triggerAsyncId, resource) => { + bootstrapBuffer.push({ + type: kInit, + asyncId, + args: [type, triggerAsyncId, resource, true] + }); + }, + [before_symbol]: (asyncId) => { + bootstrapBuffer.push({ + type: kBefore, + asyncId, + args: null + }); + }, + [after_symbol]: (asyncId) => { + bootstrapBuffer.push({ + type: kAfter, + asyncId, + args: null + }); + }, + [destroy_symbol]: (asyncId) => { + bootstrapBuffer.push({ + type: kDestroy, + asyncId, + args: null + }); + }, + [promise_resolve_symbol]: (asyncId) => { + bootstrapBuffer.push({ + type: kPromiseResolve, + asyncId, + args: null + }); + } + }; + bootstrapBuffer = []; + active_hooks.array.push(bootstrapHooks); + if (async_hook_fields[kTotals] === 5) { + enableHooks(); + } +} + +function clearBootstrapHooksBuffer() { + if (!bootstrapBuffer) + return; + async_hook_fields[kInit]--; + async_hook_fields[kBefore]--; + async_hook_fields[kAfter]--; + async_hook_fields[kDestroy]--; + async_hook_fields[kPromiseResolve]--; + async_hook_fields[kTotals] -= 5; + active_hooks.array.splice(active_hooks.array.indexOf(bootstrapHooks), 1); + if (async_hook_fields[kTotals] === 0) + disableHooks(); + const _bootstrapBuffer = bootstrapBuffer; + bootstrapBuffer = null; + bootstrapHooks = null; + return _bootstrapBuffer; +} + +function emitBootstrapHooksBuffer() { + const bootstrapBuffer = clearBootstrapHooksBuffer(); + if (!bootstrapBuffer || async_hook_fields[kTotals] === 0) { + return; + } + for (const { type, asyncId, args } of bootstrapBuffer) { + switch (type) { + case kInit: + emitInitNative(asyncId, ...args); + break; + case kBefore: + emitBeforeNative(asyncId); + break; + case kAfter: + emitAfterNative(asyncId); + break; + case kDestroy: + emitDestroyNative(asyncId); + break; + } + } +} let wantPromiseHook = false; function enableHooks() { @@ -318,7 +416,8 @@ function destroyHooksExist() { } -function emitInitScript(asyncId, type, triggerAsyncId, resource) { +function emitInitScript(asyncId, type, triggerAsyncId, resource, + bootstrap = false) { validateAsyncId(asyncId, 'asyncId'); if (triggerAsyncId !== null) validateAsyncId(triggerAsyncId, 'triggerAsyncId'); @@ -338,7 +437,7 @@ function emitInitScript(asyncId, type, triggerAsyncId, resource) { triggerAsyncId = getDefaultTriggerAsyncId(); } - emitInitNative(asyncId, type, triggerAsyncId, resource); + emitInitNative(asyncId, type, triggerAsyncId, resource, bootstrap); } @@ -468,5 +567,8 @@ module.exports = { after: emitAfterNative, destroy: emitDestroyNative, promise_resolve: emitPromiseResolveNative - } + }, + bufferBootstrapHooks, + clearBootstrapHooksBuffer, + emitBootstrapHooksBuffer }; diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js index c1636d87f42814..13c5619f677534 100644 --- a/lib/internal/bootstrap/pre_execution.js +++ b/lib/internal/bootstrap/pre_execution.js @@ -5,6 +5,7 @@ const { Object, SafeWeakMap } = primordials; const { getOptionValue } = require('internal/options'); const { Buffer } = require('buffer'); const { ERR_MANIFEST_ASSERT_INTEGRITY } = require('internal/errors').codes; +const { bufferBootstrapHooks } = require('internal/async_hooks'); function prepareMainThreadExecution(expandArgv1 = false) { // Patch the process object with legacy properties and normalizations @@ -32,6 +33,10 @@ function prepareMainThreadExecution(expandArgv1 = false) { setupDebugEnv(); + // Buffer all async hooks emitted by core bootstrap + // to allow user listeners to attach to these + bufferBootstrapHooks(); + // Only main thread receives signals. setupSignalHandlers(); diff --git a/lib/internal/main/run_main_module.js b/lib/internal/main/run_main_module.js index 2cad569dcce9fd..46182626b96643 100644 --- a/lib/internal/main/run_main_module.js +++ b/lib/internal/main/run_main_module.js @@ -3,15 +3,29 @@ const { prepareMainThreadExecution } = require('internal/bootstrap/pre_execution'); +const { getOptionValue } = require('internal/options'); prepareMainThreadExecution(true); -const CJSModule = require('internal/modules/cjs/loader').Module; - -markBootstrapComplete(); - -// Note: this actually tries to run the module as a ESM first if -// --experimental-modules is on. -// TODO(joyeecheung): can we move that logic to here? Note that this -// is an undocumented method available via `require('module').runMain` -CJSModule.runMain(); +// Load the main module--the command line argument. +const experimentalModules = getOptionValue('--experimental-modules'); +if (experimentalModules) { + const asyncESM = require('internal/process/esm_loader'); + const { pathToFileURL } = require('internal/url'); + markBootstrapComplete(); + asyncESM.loaderPromise.then((loader) => { + return loader.import(pathToFileURL(process.argv[1]).href); + }) + .catch((e) => { + internalBinding('errors').triggerUncaughtException( + e, + true /* fromPromise */ + ); + }); +} else { + const { clearBootstrapHooksBuffer } = require('internal/async_hooks'); + clearBootstrapHooksBuffer(); + const CJSModule = require('internal/modules/cjs/loader').Module; + markBootstrapComplete(); + CJSModule._load(process.argv[1], null, true); +} diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 479044a26aaba9..8782bdc132a4ca 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -1007,24 +1007,6 @@ Module._extensions['.mjs'] = function(module, filename) { throw new ERR_REQUIRE_ESM(filename); }; -// Bootstrap main module. -Module.runMain = function() { - // Load the main module--the command line argument. - if (experimentalModules) { - asyncESM.loaderPromise.then((loader) => { - return loader.import(pathToFileURL(process.argv[1]).href); - }) - .catch((e) => { - internalBinding('errors').triggerUncaughtException( - e, - true /* fromPromise */ - ); - }); - return; - } - Module._load(process.argv[1], null, true); -}; - function createRequireFromPath(filename) { // Allow a directory to be passed as the filename const trailingSlash = diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index ef11e2ec833b89..a4c2ea658d9cf6 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -11,6 +11,7 @@ const { ModuleWrap } = internalBinding('module_wrap'); const { decorateErrorStack } = require('internal/util'); const { getOptionValue } = require('internal/options'); const assert = require('internal/assert'); +const { clearBootstrapHooksBuffer } = require('internal/async_hooks'); const resolvedPromise = SafePromise.resolve(); function noop() {} @@ -106,7 +107,14 @@ class ModuleJob { const module = await this.instantiate(); const timeout = -1; const breakOnSigint = false; - return { module, result: module.evaluate(timeout, breakOnSigint) }; + const output = { + module, + result: module.evaluate(timeout, breakOnSigint) + }; + if (this.isMain) { + clearBootstrapHooksBuffer(); + } + return output; } } Object.setPrototypeOf(ModuleJob.prototype, null); diff --git a/test/es-module/test-esm-async-hooks.mjs b/test/es-module/test-esm-async-hooks.mjs new file mode 100644 index 00000000000000..1a4718b73685f7 --- /dev/null +++ b/test/es-module/test-esm-async-hooks.mjs @@ -0,0 +1,47 @@ +// Flags: --experimental-modules +import '../common/index.mjs'; +import assert from 'assert'; +import { createHook } from 'async_hooks'; + +const bootstrapCalls = {}; +const calls = {}; + +const asyncHook = createHook({ init, before, after, destroy, promiseResolve }); +asyncHook.enable(); + +function init(asyncId, type, asyncTriggerId, resource, bootstrap) { + (bootstrap ? bootstrapCalls : calls)[asyncId] = { + init: true + }; +} + +function before(asyncId) { + (bootstrapCalls[asyncId] || calls[asyncId]).before = true; +} + +function after(asyncId) { + (bootstrapCalls[asyncId] || calls[asyncId]).after = true; +} + +function destroy(asyncId) { + (bootstrapCalls[asyncId] || calls[asyncId]).destroy = true; +} + +function promiseResolve(asyncId) { + (bootstrapCalls[asyncId] || calls[asyncId]).promiseResolve = true; +} + +// Ensure all hooks have inits +assert(Object.values(bootstrapCalls).every(({ init }) => init === true)); +assert(Object.values(calls).every(({ init }) => init === true)); + +// Ensure we have at least one of each type of callback +assert(Object.values(bootstrapCalls).some(({ before }) => before === true)); +assert(Object.values(bootstrapCalls).some(({ after }) => after === true)); +assert(Object.values(bootstrapCalls).some(({ destroy }) => destroy === true)); + +// Wait a tick to see that we have promise resolution +setTimeout(() => { + assert(Object.values(bootstrapCalls).some(({ promiseResolve }) => + promiseResolve === true)); +}, 10); From c28ae364a4e3cfbc251f68591eedda93a9eca6ff Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Fri, 4 Oct 2019 17:53:13 -0400 Subject: [PATCH 02/10] fixup example api --- doc/api/async_hooks.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/async_hooks.md b/doc/api/async_hooks.md index 795b80f203c5ed..a4c5c3830ca813 100644 --- a/doc/api/async_hooks.md +++ b/doc/api/async_hooks.md @@ -323,7 +323,7 @@ elaborate to make calling context easier to see. let indent = 0; const bootstrapIds = new Set(); async_hooks.createHook({ - init(asyncId, type, triggerAsyncId, bootstrap) { + init(asyncId, type, triggerAsyncId, resource, bootstrap) { if (bootstrap) { bootstrapIds.add(asyncId); return; From 1f9e1455103edf644d7e9adfd966f19f46f0ff96 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Fri, 4 Oct 2019 20:57:51 -0400 Subject: [PATCH 03/10] keep runMain in cjs --- lib/internal/main/run_main_module.js | 32 ++++++++-------------------- lib/internal/main/worker_thread.js | 4 ++-- lib/internal/modules/cjs/loader.js | 20 ++++++++++++++++- 3 files changed, 30 insertions(+), 26 deletions(-) diff --git a/lib/internal/main/run_main_module.js b/lib/internal/main/run_main_module.js index 46182626b96643..101a38119906f6 100644 --- a/lib/internal/main/run_main_module.js +++ b/lib/internal/main/run_main_module.js @@ -3,29 +3,15 @@ const { prepareMainThreadExecution } = require('internal/bootstrap/pre_execution'); -const { getOptionValue } = require('internal/options'); prepareMainThreadExecution(true); -// Load the main module--the command line argument. -const experimentalModules = getOptionValue('--experimental-modules'); -if (experimentalModules) { - const asyncESM = require('internal/process/esm_loader'); - const { pathToFileURL } = require('internal/url'); - markBootstrapComplete(); - asyncESM.loaderPromise.then((loader) => { - return loader.import(pathToFileURL(process.argv[1]).href); - }) - .catch((e) => { - internalBinding('errors').triggerUncaughtException( - e, - true /* fromPromise */ - ); - }); -} else { - const { clearBootstrapHooksBuffer } = require('internal/async_hooks'); - clearBootstrapHooksBuffer(); - const CJSModule = require('internal/modules/cjs/loader').Module; - markBootstrapComplete(); - CJSModule._load(process.argv[1], null, true); -} +const CJSModule = require('internal/modules/cjs/loader').Module; + +markBootstrapComplete(); + +// Note: this actually tries to run the module as a ESM first if +// --experimental-modules is on. +// TODO(joyeecheung): can we move that logic to here? Note that this +// is an undocumented method available via `require('module').runMain` +CJSModule.runMain(process.argv[1]); diff --git a/lib/internal/main/worker_thread.js b/lib/internal/main/worker_thread.js index 50a19be77dd78d..516ea98201b01f 100644 --- a/lib/internal/main/worker_thread.js +++ b/lib/internal/main/worker_thread.js @@ -140,8 +140,8 @@ port.on('message', (message) => { const { evalScript } = require('internal/process/execution'); evalScript('[worker eval]', filename); } else { - process.argv[1] = filename; // script filename - require('module').runMain(); + // script filename + require('module').runMain(process.argv[1] = filename); } } else if (message.type === STDIO_PAYLOAD) { const { stream, chunk, encoding } = message; diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 8782bdc132a4ca..33a97da885144c 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -62,7 +62,7 @@ const manifest = getOptionValue('--experimental-policy') ? require('internal/process/policy').manifest : null; const { compileFunction } = internalBinding('contextify'); - +const { clearBootstrapHooksBuffer } = require('internal/async_hooks'); const { ERR_INVALID_ARG_VALUE, ERR_INVALID_OPT_VALUE, @@ -1007,6 +1007,24 @@ Module._extensions['.mjs'] = function(module, filename) { throw new ERR_REQUIRE_ESM(filename); }; +Module.runMain = function(mainPath) { + // Load the main module--the command line argument. + if (experimentalModules) { + asyncESM.loaderPromise.then((loader) => { + return loader.import(pathToFileURL(mainPath).href); + }) + .catch((e) => { + internalBinding('errors').triggerUncaughtException( + e, + true /* fromPromise */ + ); + }); + } else { + clearBootstrapHooksBuffer(); + Module._load(mainPath, null, true); + } +}; + function createRequireFromPath(filename) { // Allow a directory to be passed as the filename const trailingSlash = From 3bff2743e3eedbce2d8abdd64368448a16e888cb Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sat, 5 Oct 2019 14:24:28 -0400 Subject: [PATCH 04/10] add enable comment --- test/es-module/test-esm-async-hooks.mjs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/es-module/test-esm-async-hooks.mjs b/test/es-module/test-esm-async-hooks.mjs index 1a4718b73685f7..af1cfa73f141cf 100644 --- a/test/es-module/test-esm-async-hooks.mjs +++ b/test/es-module/test-esm-async-hooks.mjs @@ -7,6 +7,9 @@ const bootstrapCalls = {}; const calls = {}; const asyncHook = createHook({ init, before, after, destroy, promiseResolve }); + +// Bootstrap hooks for modules pipeline will trigger sync with this enable +// call. asyncHook.enable(); function init(asyncId, type, asyncTriggerId, resource, bootstrap) { From f058f9fc83f655592a773e0b85a1318f5c367f40 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sat, 5 Oct 2019 15:23:33 -0400 Subject: [PATCH 05/10] dont delay disable --- lib/internal/async_hooks.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index 8038a6b0b00b0c..9070540fc7cc69 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -299,8 +299,12 @@ function clearBootstrapHooksBuffer() { async_hook_fields[kPromiseResolve]--; async_hook_fields[kTotals] -= 5; active_hooks.array.splice(active_hooks.array.indexOf(bootstrapHooks), 1); - if (async_hook_fields[kTotals] === 0) - disableHooks(); + if (async_hook_fields[kTotals] === 0) { + async_hook_fields[kCheck] -= 1; + wantPromiseHook = false; + // No delay, because timing is always carefully managed by the caller. + disablePromiseHookIfNecessary(); + } const _bootstrapBuffer = bootstrapBuffer; bootstrapBuffer = null; bootstrapHooks = null; From f8416cea0e2916f1f72d0aeedb8efc6dc45f059a Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sat, 5 Oct 2019 18:00:49 -0400 Subject: [PATCH 06/10] flush microtask queue instead of making sync disable --- lib/internal/async_hooks.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index 9070540fc7cc69..0d9de0bfbdc087 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -300,10 +300,9 @@ function clearBootstrapHooksBuffer() { async_hook_fields[kTotals] -= 5; active_hooks.array.splice(active_hooks.array.indexOf(bootstrapHooks), 1); if (async_hook_fields[kTotals] === 0) { - async_hook_fields[kCheck] -= 1; - wantPromiseHook = false; - // No delay, because timing is always carefully managed by the caller. - disablePromiseHookIfNecessary(); + disableHooks(); + // Flush microtasks to ensure disable has run. + process._tickCallback(); } const _bootstrapBuffer = bootstrapBuffer; bootstrapBuffer = null; From ac7fdca8cb05a6877694522bcc6833df4cfa2d5e Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sat, 5 Oct 2019 18:41:13 -0400 Subject: [PATCH 07/10] fixup doc links --- doc/api/async_hooks.md | 2 +- doc/api/n-api.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/async_hooks.md b/doc/api/async_hooks.md index a4c5c3830ca813..a42965a28ce853 100644 --- a/doc/api/async_hooks.md +++ b/doc/api/async_hooks.md @@ -695,7 +695,7 @@ never be called. [`after` callback]: #async_hooks_after_asyncid [`before` callback]: #async_hooks_before_asyncid [`destroy` callback]: #async_hooks_destroy_asyncid -[`init` callback]: #async_hooks_init_asyncid_type_triggerasyncid_resource +[`init` callback]: #async_hooks_init_asyncid_type_triggerasyncid_resource_bootstrap [`promiseResolve` callback]: #async_hooks_promiseresolve_asyncid [Hook Callbacks]: #async_hooks_hook_callbacks [PromiseHooks]: https://docs.google.com/document/d/1rda3yKGHimKIhg5YeoAmCOtyURgsbTH_qaYR79FELlk/edit diff --git a/doc/api/n-api.md b/doc/api/n-api.md index d0bd46e790f737..e30e005a3db879 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -5152,7 +5152,7 @@ This API may only be called from the main thread. [Working with JavaScript Properties]: #n_api_working_with_javascript_properties [Working with JavaScript Values - Abstract Operations]: #n_api_working_with_javascript_values_abstract_operations [Working with JavaScript Values]: #n_api_working_with_javascript_values -[`init` hooks]: async_hooks.html#async_hooks_init_asyncid_type_triggerasyncid_resource +[`init` hooks]: async_hooks.html#async_hooks_init_asyncid_type_triggerasyncid_resource_bootstrap [`napi_add_finalizer`]: #n_api_napi_add_finalizer [`napi_async_init`]: #n_api_napi_async_init [`napi_cancel_async_work`]: #n_api_napi_cancel_async_work From ae46f454877677e26e09b2793d8581556ab72650 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sun, 6 Oct 2019 17:57:45 -0400 Subject: [PATCH 08/10] stop bootstrap buffer as soon as usercode runs --- lib/internal/async_hooks.js | 18 +++++++++++------- lib/internal/modules/esm/module_job.js | 8 +++++--- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index 0d9de0bfbdc087..f64cbe5f031fb9 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -289,8 +289,8 @@ function bufferBootstrapHooks() { } } -function clearBootstrapHooksBuffer() { - if (!bootstrapBuffer) +function stopBootstrapHooksBuffer() { + if (!bootstrapHooks) return; async_hook_fields[kInit]--; async_hook_fields[kBefore]--; @@ -299,19 +299,21 @@ function clearBootstrapHooksBuffer() { async_hook_fields[kPromiseResolve]--; async_hook_fields[kTotals] -= 5; active_hooks.array.splice(active_hooks.array.indexOf(bootstrapHooks), 1); + bootstrapHooks = null; if (async_hook_fields[kTotals] === 0) { disableHooks(); // Flush microtasks to ensure disable has run. process._tickCallback(); } - const _bootstrapBuffer = bootstrapBuffer; +} + +function clearBootstrapHooksBuffer() { + if (bootstrapHooks) + stopBootstrapHooksBuffer(); bootstrapBuffer = null; - bootstrapHooks = null; - return _bootstrapBuffer; } function emitBootstrapHooksBuffer() { - const bootstrapBuffer = clearBootstrapHooksBuffer(); if (!bootstrapBuffer || async_hook_fields[kTotals] === 0) { return; } @@ -331,6 +333,7 @@ function emitBootstrapHooksBuffer() { break; } } + bootstrapBuffer = null; } let wantPromiseHook = false; @@ -573,5 +576,6 @@ module.exports = { }, bufferBootstrapHooks, clearBootstrapHooksBuffer, - emitBootstrapHooksBuffer + emitBootstrapHooksBuffer, + stopBootstrapHooksBuffer }; diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index a4c2ea658d9cf6..1bdb4c0d28f2c9 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -11,7 +11,8 @@ const { ModuleWrap } = internalBinding('module_wrap'); const { decorateErrorStack } = require('internal/util'); const { getOptionValue } = require('internal/options'); const assert = require('internal/assert'); -const { clearBootstrapHooksBuffer } = require('internal/async_hooks'); +const { clearBootstrapHooksBuffer, stopBootstrapHooksBuffer } = + require('internal/async_hooks'); const resolvedPromise = SafePromise.resolve(); function noop() {} @@ -107,13 +108,14 @@ class ModuleJob { const module = await this.instantiate(); const timeout = -1; const breakOnSigint = false; + if (this.isMain) + stopBootstrapHooksBuffer(); const output = { module, result: module.evaluate(timeout, breakOnSigint) }; - if (this.isMain) { + if (this.isMain) clearBootstrapHooksBuffer(); - } return output; } } From 96dd1b90b401b08bfb41cb0b8ebebc5e4f03b09c Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sun, 6 Oct 2019 19:56:13 -0400 Subject: [PATCH 09/10] refine promise timings --- lib/internal/async_hooks.js | 22 +++++++++++-------- lib/internal/modules/cjs/loader.js | 6 +++-- lib/internal/modules/esm/loader.js | 3 +++ lib/internal/modules/esm/module_map.js | 2 +- .../test-internal-module-map-asserts.js | 8 +++---- 5 files changed, 25 insertions(+), 16 deletions(-) diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index f64cbe5f031fb9..3f16635627ef0d 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -289,6 +289,18 @@ function bufferBootstrapHooks() { } } +function clearBootstrapHooksBuffer() { + if (!bootstrapBuffer) + return; + if (bootstrapHooks) { + stopBootstrapHooksBuffer(); + process._tickCallback(); + } + const _bootstrapBuffer = bootstrapBuffer; + bootstrapBuffer = null; + return _bootstrapBuffer; +} + function stopBootstrapHooksBuffer() { if (!bootstrapHooks) return; @@ -302,18 +314,11 @@ function stopBootstrapHooksBuffer() { bootstrapHooks = null; if (async_hook_fields[kTotals] === 0) { disableHooks(); - // Flush microtasks to ensure disable has run. - process._tickCallback(); } } -function clearBootstrapHooksBuffer() { - if (bootstrapHooks) - stopBootstrapHooksBuffer(); - bootstrapBuffer = null; -} - function emitBootstrapHooksBuffer() { + const bootstrapBuffer = clearBootstrapHooksBuffer(); if (!bootstrapBuffer || async_hook_fields[kTotals] === 0) { return; } @@ -333,7 +338,6 @@ function emitBootstrapHooksBuffer() { break; } } - bootstrapBuffer = null; } let wantPromiseHook = false; diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 33a97da885144c..d10a3fcf895331 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -821,10 +821,12 @@ Module.prototype.load = function(filename) { if (module !== undefined && module.module !== undefined) { if (module.module.getStatus() >= kInstantiated) module.module.setExport('default', exports); - } else { // preemptively cache + } else { + // Preemptively cache + // We use a function to defer promise creation for async hooks. ESMLoader.moduleMap.set( url, - new ModuleJob(ESMLoader, url, () => + () => new ModuleJob(ESMLoader, url, () => new ModuleWrap(function() { this.setExport('default', exports); }, ['default'], url) diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 138cf8b5ecc3ed..a58b1ecf19ffb8 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -146,6 +146,9 @@ class Loader { async getModuleJob(specifier, parentURL) { const { url, format } = await this.resolve(specifier, parentURL); let job = this.moduleMap.get(url); + // CJS injects jobs as functions to defer promise creation for async hooks. + if (typeof job === 'function') + this.moduleMap.set(url, job = job()); if (job !== undefined) return job; diff --git a/lib/internal/modules/esm/module_map.js b/lib/internal/modules/esm/module_map.js index 01521fb7885ee1..4fe2a6f36913bc 100644 --- a/lib/internal/modules/esm/module_map.js +++ b/lib/internal/modules/esm/module_map.js @@ -16,7 +16,7 @@ class ModuleMap extends SafeMap { } set(url, job) { validateString(url, 'url'); - if (job instanceof ModuleJob !== true) { + if (job instanceof ModuleJob !== true && typeof job !== 'function') { throw new ERR_INVALID_ARG_TYPE('job', 'ModuleJob', job); } debug(`Storing ${url} in ModuleMap`); diff --git a/test/parallel/test-internal-module-map-asserts.js b/test/parallel/test-internal-module-map-asserts.js index 4563fc605e0792..614da43aba0acb 100644 --- a/test/parallel/test-internal-module-map-asserts.js +++ b/test/parallel/test-internal-module-map-asserts.js @@ -12,7 +12,7 @@ const ModuleMap = require('internal/modules/esm/module_map'); code: 'ERR_INVALID_ARG_TYPE', type: TypeError, message: /^The "url" argument must be of type string/ - }, 15); + }, 12); const moduleMap = new ModuleMap(); @@ -21,7 +21,7 @@ const ModuleMap = require('internal/modules/esm/module_map'); // but I think it's useless, and was not simple to mock... const job = undefined; - [{}, [], true, 1, () => {}].forEach((value) => { + [{}, [], true, 1].forEach((value) => { assert.throws(() => moduleMap.get(value), errorReg); assert.throws(() => moduleMap.has(value), errorReg); assert.throws(() => moduleMap.set(value, job), errorReg); @@ -34,11 +34,11 @@ const ModuleMap = require('internal/modules/esm/module_map'); code: 'ERR_INVALID_ARG_TYPE', type: TypeError, message: /^The "job" argument must be of type ModuleJob/ - }, 5); + }, 4); const moduleMap = new ModuleMap(); - [{}, [], true, 1, () => {}].forEach((value) => { + [{}, [], true, 1].forEach((value) => { assert.throws(() => moduleMap.set('', value), errorReg); }); } From d98d0f1f58234bfbbe595a911d6890363411b280 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sun, 6 Oct 2019 22:35:34 -0400 Subject: [PATCH 10/10] ensure fully sync detachment --- lib/internal/async_hooks.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index 3f16635627ef0d..927797ad82bca6 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -294,7 +294,6 @@ function clearBootstrapHooksBuffer() { return; if (bootstrapHooks) { stopBootstrapHooksBuffer(); - process._tickCallback(); } const _bootstrapBuffer = bootstrapBuffer; bootstrapBuffer = null; @@ -314,6 +313,8 @@ function stopBootstrapHooksBuffer() { bootstrapHooks = null; if (async_hook_fields[kTotals] === 0) { disableHooks(); + // Ensure disable happens immediately and synchronously. + process._tickCallback(); } }