From d170321d684530423cb2efda46d21edbb828c7b6 Mon Sep 17 00:00:00 2001 From: isaacs Date: Wed, 27 Jun 2012 14:44:25 -0700 Subject: [PATCH] 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. --- lib/cache.js | 98 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 59 insertions(+), 39 deletions(-) diff --git a/lib/cache.js b/lib/cache.js index 06f518eb154..55897bc42c1 100644 --- 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,8 +385,10 @@ 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] @@ -390,19 +396,27 @@ function addNamed (name, x, 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) }