One concurrent read (merge #1697)

Author: kriskowal

packages/compartment-mapper/src/import-hook.js

@@ -257,7 +257,7 @@ export const makeImportHookMaker = (
         }
       }
 
-      const { read } = unpackReadPowers(readPowers);
+      const { maybeRead } = unpackReadPowers(readPowers);
 
       for (const candidateSpecifier of candidates) {
         const candidateModuleDescriptor = moduleDescriptors[candidateSpecifier];
@@ -298,9 +298,7 @@ export const makeImportHookMaker = (
           packageLocation,
         );
         // eslint-disable-next-line no-await-in-loop
-        const moduleBytes = await read(moduleLocation).catch(
-          _error => undefined,
-        );
+        const moduleBytes = await maybeRead(moduleLocation);
         if (moduleBytes !== undefined) {
           /** @type {string | undefined} */
           let sourceMap;

packages/compartment-mapper/src/node-modules.js

@@ -3,12 +3,14 @@
 
 /** @typedef {import('./types.js').Language} Language */
 /** @typedef {import('./types.js').ReadFn} ReadFn */
+/** @typedef {import('./types.js').MaybeReadFn} MaybeReadFn */
 /** @typedef {import('./types.js').CanonicalFn} CanonicalFn */
 /** @typedef {import('./types.js').CompartmentMapDescriptor} CompartmentMapDescriptor */
 /** @typedef {import('./types.js').ModuleDescriptor} ModuleDescriptor */
 /** @typedef {import('./types.js').ScopeDescriptor} ScopeDescriptor */
 /** @typedef {import('./types.js').CompartmentDescriptor} CompartmentDescriptor */
 /** @typedef {import('./types.js').ReadPowers} ReadPowers */
+/** @typedef {import('./types.js').MaybeReadPowers} MaybeReadPowers */
 
 /**
  * The graph is an intermediate object model that the functions of this module
@@ -83,15 +85,13 @@ const basename = location => {
 };
 
 /**
- * @param {ReadFn} read
+ * @param {MaybeReadFn} maybeRead
  * @param {string} packageLocation
  * @returns {Promise<object>}
  */
-const readDescriptor = async (read, packageLocation) => {
+const readDescriptor = async (maybeRead, packageLocation) => {
   const descriptorLocation = resolveLocation('package.json', packageLocation);
-  const descriptorBytes = await read(descriptorLocation).catch(
-    _error => undefined,
-  );
+  const descriptorBytes = await maybeRead(descriptorLocation);
   if (descriptorBytes === undefined) {
     return undefined;
   }
@@ -102,16 +102,16 @@ const readDescriptor = async (read, packageLocation) => {
 
 /**
  * @param {Record<string, object>} memo
- * @param {ReadFn} read
+ * @param {MaybeReadFn} maybeRead
  * @param {string} packageLocation
  * @returns {Promise<object>}
  */
-const readDescriptorWithMemo = async (memo, read, packageLocation) => {
+const readDescriptorWithMemo = async (memo, maybeRead, packageLocation) => {
   let promise = memo[packageLocation];
   if (promise !== undefined) {
     return promise;
   }
-  promise = readDescriptor(read, packageLocation);
+  promise = readDescriptor(maybeRead, packageLocation);
   memo[packageLocation] = promise;
   return promise;
 };
@@ -479,7 +479,7 @@ const gatherDependency = async (
  * the URL that was used as the key of graph).
  * The URLs in dependencies will all exist as other keys of graph.
  *
- * @param {ReadFn} read
+ * @param {MaybeReadFn} maybeRead
  * @param {CanonicalFn} canonical
  * @param {string} packageLocation - location of the main package.
  * @param {Set<string>} tags
@@ -490,7 +490,7 @@ const gatherDependency = async (
  * @param {Record<string,string>} [commonDependencies] - dependencies to be added to all packages
  */
 const graphPackages = async (
-  read,
+  maybeRead,
   canonical,
   packageLocation,
   tags,
@@ -504,7 +504,7 @@ const graphPackages = async (
    * @returns {Promise<object>}
    */
   const readDescriptor = packageLocation =>
-    readDescriptorWithMemo(memo, read, packageLocation);
+    readDescriptorWithMemo(memo, maybeRead, packageLocation);
 
   if (mainPackageDescriptor !== undefined) {
     memo[packageLocation] = Promise.resolve(mainPackageDescriptor);
@@ -697,7 +697,7 @@ const translateGraph = (
 };
 
 /**
- * @param {ReadFn | ReadPowers} readPowers
+ * @param {ReadFn | ReadPowers | MaybeReadPowers} readPowers
  * @param {string} packageLocation
  * @param {Set<string>} tags
  * @param {object} packageDescriptor
@@ -717,9 +717,9 @@ export const compartmentMapForNodeModules = async (
   options = {},
 ) => {
   const { dev = false, commonDependencies, policy } = options;
-  const { read, canonical } = unpackReadPowers(readPowers);
+  const { maybeRead, canonical } = unpackReadPowers(readPowers);
   const graph = await graphPackages(
-    read,
+    maybeRead,
     canonical,
     packageLocation,
     tags,

packages/compartment-mapper/src/node-powers.js

@@ -40,18 +40,43 @@ const makeReadPowersSloppy = ({ fs, url = undefined, crypto = undefined }) => {
   const pathToFileURL =
     url === undefined ? fakePathToFileURL : url.pathToFileURL;
 
+  let readMutex = Promise.resolve(undefined);
+
   /**
    * @param {string} location
    */
   const read = async location => {
+    const promise = readMutex;
+    let release = Function.prototype;
+    readMutex = new Promise(resolve => {
+      release = resolve;
+    });
+    await promise;
+
+    const path = fileURLToPath(location);
     try {
-      const path = fileURLToPath(location);
+      // We await here to ensure that we release the mutex only after
+      // completing the read.
       return await fs.promises.readFile(path);
-    } catch (error) {
-      throw Error(error.message);
+    } finally {
+      release(undefined);
     }
   };
 
+  /**
+   * @param {string} location
+   */
+  const maybeRead = location =>
+    read(location).catch(error => {
+      if (
+        error.message.startsWith('ENOENT: ') ||
+        error.message.startsWith('EISDIR: ')
+      ) {
+        return undefined;
+      }
+      throw error;
+    });
+
   const requireResolve = (from, specifier, options) =>
     createRequire(from).resolve(specifier, options);
 
@@ -97,6 +122,7 @@ const makeReadPowersSloppy = ({ fs, url = undefined, crypto = undefined }) => {
 
   return {
     read,
+    maybeRead,
     fileURLToPath,
     pathToFileURL,
     canonical,

packages/compartment-mapper/src/powers.js

@@ -1,21 +1,42 @@
 // @ts-check
-/** @typedef {import('./types.js').ReadFn} ReadFn */
-/** @typedef {import('./types.js').CanonicalFn} CanonicalFn */
-/** @typedef {import('./types.js').ReadPowers} ReadPowers */
 
-/** @type {CanonicalFn} */
+/** @type {import('./types.js').CanonicalFn} */
 const canonicalShim = async path => path;
 
 /**
- * @param {ReadFn | ReadPowers} powers
- * @returns {ReadPowers}
+ * @param {import('./types.js').ReadFn | import('./types.js').ReadPowers | import('./types.js').MaybeReadPowers} powers
+ * @returns {import('./types.js').MaybeReadPowers}
  */
 export const unpackReadPowers = powers => {
+  /** @type {import('./types.js').ReadFn | undefined} */
+  let read;
+  /** @type {import('./types.js').MaybeReadFn | undefined} */
+  let maybeRead;
+  /** @type {import('./types.js').CanonicalFn | undefined} */
+  let canonical;
+
   if (typeof powers === 'function') {
-    return {
-      read: powers,
-      canonical: canonicalShim,
-    };
+    read = powers;
+  } else {
+    ({ read, maybeRead, canonical } = powers);
+  }
+
+  if (canonical === undefined) {
+    canonical = canonicalShim;
   }
-  return powers;
+
+  if (maybeRead === undefined) {
+    /** @param {string} path */
+    maybeRead = path =>
+      /** @type {import('./types.js').ReadFn} */ (read)(path).catch(
+        _error => undefined,
+      );
+  }
+
+  return {
+    ...powers,
+    read,
+    maybeRead,
+    canonical,
+  };
 };

packages/compartment-mapper/src/types.js

@@ -113,6 +113,14 @@ export {};
  * @returns {Promise<Uint8Array>} bytes
  */
 
+/**
+ * A resolution of `undefined` indicates `ENOENT` or the equivalent.
+ *
+ * @callback MaybeReadFn
+ * @param {string} location
+ * @returns {Promise<Uint8Array | undefined>} bytes
+ */
+
 /**
  * Returns a canonical URL for a given URL, following redirects or symbolic
  * links if any exist along the path.
@@ -156,6 +164,11 @@ export {};
  * @property {Function} [requireResolve]
  */
 
+/**
+ * @typedef {ReadPowers | object} MaybeReadPowers
+ * @property {MaybeReadFn} maybeRead
+ */
+
 /**
  * @typedef {object} HashPowers
  * @property {ReadFn} read

packages/compartment-mapper/test/scaffold.js

@@ -15,7 +15,11 @@ import {
 } from '../index.js';
 import { makeReadPowers } from '../src/node-powers.js';
 
-export const readPowers = makeReadPowers({ fs, crypto, url });
+export const readPowers = makeReadPowers({
+  fs,
+  crypto,
+  url,
+});
 
 export const sanitizePaths = (text = '', tolerateLineChange = false) => {
   if (tolerateLineChange) {

packages/nat/package.json

@@ -9,6 +9,7 @@
     "./package.json": "./package.json"
   },
   "scripts": {
+    "build": "exit 0",
     "test": "ava test/**/test-*.js",
     "lint-fix": "eslint --fix",
     "lint-check": "eslint"