refine promise timings

nodejs · Oct 7, 2019 · 96dd1b9 · 96dd1b9
1 parent ae46f45
commit 96dd1b9
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 16 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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);
   });
 }