Lock all the things

Any time a thing is added to the cache, by url, path or name@version/range/tag, set a lock file, and then clear it after completion. npm can now safely share the cache folder between separate processes. However, it is still *not* safe to do multiple parallel installs in the same prefix.
brianloveswords · Jun 27, 2012 · d170321 · d170321
1 parent 3eea0e4
commit d170321
Showing 1 changed file with 59 additions and 39 deletions.
diff --git a/lib/cache.js b/lib/cache.js
@@ -264,7 +264,11 @@ function add (args, cb) {
     default:
       // if we have a name and a spec, then try name@spec
       // if not, then try just spec (which may try name@"" if not found)
-      return name ? addNamed(name, spec, cb) : addLocal(spec, cb)
+      if (name) {
+        addNamed(name, spec, cb)
+      } else {
+        addLocal(spec, cb)
+      }
   }
 }
 
@@ -281,14 +285,14 @@ function addRemoteTarball (u, shasum, name, cb_) {
   if (iF.length > 1) return
 
   function cb (er, data) {
-    urlUnlock(u, function () {
+    unlock(u, function () {
       var c
       while (c = iF.shift()) c(er, data)
       delete inFlightURLs[u]
     })
   }
 
-  urlLock(u, function (er) {
+  lock(u, function (er) {
     if (er) return cb(er)
 
     log.verbose("addRemoteTarball", [u, shasum])
@@ -324,14 +328,14 @@ function addRemoteGit (u, parsed, name, cb_) {
   if (iF.length > 1) return
 
   function cb (er, data) {
-    urlUnlock(u, function () {
+    unlock(u, function () {
       var c
       while (c = iF.shift()) c(er, data)
       delete inFlightURLs[u]
     })
   }
 
-  urlLock(u, function (er) {
+  lock(u, function (er) {
     if (er) return cb(er)
 
     // figure out what we should check out.
@@ -381,28 +385,38 @@ function addRemoteGit (u, parsed, name, cb_) {
 // only have one request in flight for a given
 // name@blah thing.
 var inFlightNames = {}
-function addNamed (name, x, cb_) {
+function addNamed (name, x, data, cb_) {
+  if (typeof cb_ !== "function") cb_ = data, data = null
   log.verbose("addNamed", [name, x])
+
   var k = name + "@" + x
   if (!inFlightNames[k]) inFlightNames[k] = []
   var iF = inFlightNames[k]
   iF.push(cb_)
   if (iF.length > 1) return
 
   function cb (er, data) {
-    var c
-    while (c = iF.shift()) c(er, data)
-    delete inFlightNames[k]
+    unlock(k, function () {
+      var c
+      while (c = iF.shift()) c(er, data)
+      delete inFlightNames[k]
+    })
   }
 
   log.verbose("addNamed", [semver.valid(x), semver.validRange(x)])
-  return ( null !== semver.valid(x) ? addNameVersion
-         : null !== semver.validRange(x) ? addNameRange
-         : addNameTag
-         )(name, x, cb)
+  lock(k, function (er, fd) {
+    if (er) return cb(er)
+
+    var fn = ( null !== semver.valid(x) ? addNameVersion
+             : null !== semver.validRange(x) ? addNameRange
+             : addNameTag
+             )
+    fn(name, x, data, cb)
+  })
 }
 
-function addNameTag (name, tag, cb) {
+function addNameTag (name, tag, data, cb) {
+  if (typeof cb !== "function") cb = data, data = null
   log.info("addNameTag", [name, tag])
   var explicit = true
   if (!tag) {
@@ -416,10 +430,10 @@ function addNameTag (name, tag, cb) {
     if (data["dist-tags"] && data["dist-tags"][tag]
         && data.versions[data["dist-tags"][tag]]) {
       var ver = data["dist-tags"][tag]
-      return addNameVersion(name, ver, data.versions[ver], cb)
+      return addNamed(name, ver, data.versions[ver], cb)
     }
     if (!explicit && Object.keys(data.versions).length) {
-      return addNameRange(name, "*", data, cb)
+      return addNamed(name, "*", data, cb)
     }
     return cb(installTargetsError(tag, data))
   })
@@ -476,12 +490,12 @@ function addNameRange (name, range, data, cb) {
 
   function next_ () {
     log.silly("addNameRange", "versions"
-             , [data.name, Object.keys(data.versions)])
+             , [data.name, Object.keys(data.versions || {})])
 
     // if the tagged version satisfies, then use that.
     var tagged = data["dist-tags"][npm.config.get("tag")]
     if (tagged && data.versions[tagged] && semver.satisfies(tagged, range)) {
-      return addNameVersion(name, tagged, data.versions[tagged], cb)
+      return addNamed(name, tagged, data.versions[tagged], cb)
     }
 
     // find the max satisfying version.
@@ -492,7 +506,7 @@ function addNameRange (name, range, data, cb) {
 
     // if we don't have a registry connection, try to see if
     // there's a cached copy that will be ok.
-    addNameVersion(name, ms, data.versions[ms], cb)
+    addNamed(name, ms, data.versions[ms], cb)
   }
 }
 
@@ -611,24 +625,29 @@ function addLocal (p, name, cb_) {
   if (typeof cb_ !== "function") cb_ = name, name = ""
 
   function cb (er, data) {
-    if (er) {
-      // if it doesn't have a / in it, it might be a
-      // remote thing.
-      if (p.indexOf("/") === -1 && p.charAt(0) !== "."
-         && (process.platform !== "win32" || p.indexOf("\\") === -1)) {
-        return addNamed(p, "", cb_)
+    unlock(p, function () {
+      if (er) {
+        // if it doesn't have a / in it, it might be a
+        // remote thing.
+        if (p.indexOf("/") === -1 && p.charAt(0) !== "."
+           && (process.platform !== "win32" || p.indexOf("\\") === -1)) {
+          return addNamed(p, "", cb_)
+        }
+        log.error("addLocal", "Could not install %s", p)
+        return cb_(er)
       }
-      log.error("addLocal", "Could not install %s", p)
-      return cb_(er)
-    }
-    return cb_(er, data)
+      return cb_(er, data)
+    })
   }
 
-  // figure out if this is a folder or file.
-  fs.stat(p, function (er, s) {
+  lock(p, function (er) {
     if (er) return cb(er)
-    if (s.isDirectory()) addLocalDirectory(p, name, cb)
-    else addLocalTarball(p, name, cb)
+    // figure out if this is a folder or file.
+    fs.stat(p, function (er, s) {
+      if (er) return cb(er)
+      if (s.isDirectory()) addLocalDirectory(p, name, cb)
+      else addLocalTarball(p, name, cb)
+    })
   })
 }
 
@@ -886,20 +905,21 @@ function deprCheck (data) {
   }
 }
 
-function urlLockFile (u) {
+function lockFileName (u) {
   var c = u.replace(/[^a-zA-Z0-9]+/g, '-')
     , h = crypto.createHash("sha1").update(u).digest("hex")
   return path.resolve(npm.config.get("cache"), h + "-" + c + ".lock")
 }
 
-function urlLock (u, cb) {
+function lock (u, cb) {
   var opts = { stale: npm.config.get("cache-lock-stale")
              , retries: npm.config.get("cache-lock-retries")
              , wait: npm.config.get("cache-lock-wait") }
-  log.warn('lockFile', u, urlLockFile(u))
-  lockFile.lock(urlLockFile(u), opts, cb)
+  var lf = lockFileName(u)
+  log.verbose("lock", u, lf)
+  lockFile.lock(lf, opts, cb)
 }
 
-function urlUnlock (u, cb) {
-  lockFile.unlock(urlLockFile(u), cb)
+function unlock (u, cb) {
+  lockFile.unlock(lockFileName(u), cb)
 }