Commit a15d4a44 authored by Linus Heckemann's avatar Linus Heckemann
Browse files

linkNodeModulesHook: replace symlinks atomically 

Up to now, existing symlinks were replaced by removing the existing link
and then creating a new one. This left a window where the link did not
exist, which would break cases where the hook was running concurrently
with other things using node_modules.

In my case, I would run multiple webpack builds at once like:

  nix develop -c webpack build --mode production
  nix develop -c webpack build --mode development

If one of the shells was ready faster than the other, webpack would
start running, then the second would run the linkNodeModules hook, and
the earlier webpack build would run into missing files as the second
linkNodeModules hook removed symlinks to replace them.


This change mitigates this race condition in two ways:

- Not replacing the symlink if it already points to the right target
  (this also avoids superfluous filesystem writes)

- Replacing the symlink atomically, using a rename,
  otherwise. The symlink was replaced atomically prior to
  db7050bb as well, but the name of the
  temporary symlink was always the same, which led to a similar race
  condition. This change reintroduces the atomic replacement, but adds
  the PID to the temporary filename in order to avoid conflicting with
  other instances of the hook running concurrently.
parent 5b2c2d84

pkgs/build-support/node/import-npm-lock/hooks/link-node-modules.js

+11 −6
Original line number Diff line number Diff line
@@ -71,17 +71,22 @@ async function main() { 
      // Don't unlink this file, we just wrote it.

      managed.delete(file);



      // Link file

      // No need to replace the symlink if it already points to the right target

      try {

        await fs.promises.symlink(sourcePath, targetPath);

        if (await fs.promises.readlink(targetPath) === sourcePath) {

          return

        }

      } catch (err) {

        // If the target file already exists remove it and try again

        if (err.code !== "EEXIST") {

        // If the symlink doesn't already exist, keep going. If we get a

        // different error trying to stat it, fail.

        if (err.code !== "ENOENT") {

          throw err;

        }

        await fs.promises.unlink(targetPath);

        await fs.promises.symlink(sourcePath, targetPath);

      }

      // Replace the link, if it exists, atomically.

      const tmpPath = `${targetPath}.tmp.${process.pid}`

      await fs.promises.symlink(sourcePath, tmpPath);

      await fs.promises.rename(tmpPath, targetPath);

    })

  );