diff options
author | isaacs <i@izs.me> | 2012-06-29 22:40:10 +0400 |
---|---|---|
committer | isaacs <i@izs.me> | 2012-06-29 22:40:10 +0400 |
commit | d517f420c2f5164b7fbb80adcf634a4805dceb18 (patch) | |
tree | 0fd856c67fcb0a2d79f25c7ff6539985b26b761e | |
parent | ccc775ccff054ca7478ec966a8bf27e8a887d330 (diff) |
Stronger protection around parallel installs
Now all tarball pack/unpack operations are wrapped in a lock
-rw-r--r-- | lib/cache.js | 27 | ||||
-rw-r--r-- | lib/utils/tar.js | 85 |
2 files changed, 77 insertions, 35 deletions
diff --git a/lib/cache.js b/lib/cache.js index c6ca8b477..66fe9e833 100644 --- a/lib/cache.js +++ b/lib/cache.js @@ -54,6 +54,8 @@ exports = module.exports = cache exports.read = read exports.clean = clean exports.unpack = unpack +exports.lock = lock +exports.unlock = unlock var mkdir = require("mkdirp") , exec = require("./utils/exec.js") @@ -788,11 +790,24 @@ function addPlacedTarball_ (p, name, uid, gid, cb) { var target = path.dirname(p) , folder = path.join(target, "package") - rm(folder, function (er) { - if (er) { - log.error("addPlacedTarball", "Could not remove %j", folder) - return cb(er) - } + lock(folder, function (er) { + if (er) return cb(er) + rmUnpack() + }) + + function rmUnpack () { + rm(folder, function (er) { + unlock(folder, function () { + if (er) { + log.error("addPlacedTarball", "Could not remove %j", folder) + return cb(er) + } + thenUnpack() + }) + }) + } + + function thenUnpack () { tar.unpack(p, folder, null, null, uid, gid, function (er) { if (er) { log.error("addPlacedTarball", "Could not unpack %j to %j", p, target) @@ -837,7 +852,7 @@ function addPlacedTarball_ (p, name, uid, gid, cb) { }) }) }) - }) + } } function addLocalDirectory (p, name, cb) { diff --git a/lib/utils/tar.js b/lib/utils/tar.js index 1fdd9baf2..5d738ba58 100644 --- a/lib/utils/tar.js +++ b/lib/utils/tar.js @@ -9,6 +9,8 @@ var npm = require("../npm.js") , rm = require("rimraf") , readJson = require("read-package-json") , cache = require("../cache.js") + , lock = cache.lock + , unlock = cache.unlock , myUid = process.getuid && process.getuid() , myGid = process.getgid && process.getgid() , tar = require("tar") @@ -43,34 +45,43 @@ function pack (targetTarball, folder, pkg, dfc, cb) { } } -function pack_ (targetTarball, folder, pkg, cb) { - new Packer({ path: folder, type: "Directory", isDirectory: true }) - .on("error", function (er) { - if (er) log.error("tar pack", "Error reading " + folder) - return cb(er) +function pack_ (targetTarball, folder, pkg, cb_) { + function cb (er) { + unlock(targetTarball, function () { + return cb_(er) }) + } + lock(targetTarball, function (er) { + if (er) return cb(er) - // By default, npm includes some proprietary attributes in the - // package tarball. This is sane, and allowed by the spec. - // However, npm *itself* excludes these from its own package, - // so that it can be more easily bootstrapped using old and - // non-compliant tar implementations. - .pipe(tar.Pack({ noProprietary: !npm.config.get("proprietary-attribs") })) - .on("error", function (er) { - if (er) log.error("tar.pack", "tar creation error", targetTarball) - cb(er) - }) - .pipe(zlib.Gzip()) - .on("error", function (er) { - if (er) log.error("tar.pack", "gzip error "+targetTarball) - cb(er) - }) - .pipe(fstream.Writer({ type: "File", path: targetTarball })) - .on("error", function (er) { - if (er) log.error("tar.pack", "Could not write "+targetTarball) - cb(er) - }) - .on("close", cb) + new Packer({ path: folder, type: "Directory", isDirectory: true }) + .on("error", function (er) { + if (er) log.error("tar pack", "Error reading " + folder) + return cb(er) + }) + + // By default, npm includes some proprietary attributes in the + // package tarball. This is sane, and allowed by the spec. + // However, npm *itself* excludes these from its own package, + // so that it can be more easily bootstrapped using old and + // non-compliant tar implementations. + .pipe(tar.Pack({ noProprietary: !npm.config.get("proprietary-attribs") })) + .on("error", function (er) { + if (er) log.error("tar.pack", "tar creation error", targetTarball) + cb(er) + }) + .pipe(zlib.Gzip()) + .on("error", function (er) { + if (er) log.error("tar.pack", "gzip error "+targetTarball) + cb(er) + }) + .pipe(fstream.Writer({ type: "File", path: targetTarball })) + .on("error", function (er) { + if (er) log.error("tar.pack", "Could not write "+targetTarball) + cb(er) + }) + .on("close", cb) + }) } @@ -87,13 +98,29 @@ function unpack (tarball, unpackTarget, dMode, fMode, uid, gid, cb) { }) } -function unpack_ ( tarball, unpackTarget, dMode, fMode, uid, gid, cb ) { +function unpack_ ( tarball, unpackTarget, dMode, fMode, uid, gid, cb_ ) { var parent = path.dirname(unpackTarget) , base = path.basename(unpackTarget) - rm(unpackTarget, function (er) { + function cb (er) { + unlock(unpackTarget, function () { + return cb_(er) + }) + } + + lock(unpackTarget, function (er) { if (er) return cb(er) + rmGunz() + }) + function rmGunz () { + rm(unpackTarget, function (er) { + if (er) return cb(er) + gtp() + }) + } + + function gtp () { // gzip {tarball} --decompress --stdout \ // | tar -mvxpf - --strip-components=1 -C {unpackTarget} gunzTarPerm( tarball, unpackTarget @@ -103,7 +130,7 @@ function unpack_ ( tarball, unpackTarget, dMode, fMode, uid, gid, cb ) { if (er) return cb(er) readJson(path.resolve(folder, "package.json"), cb) }) - }) + } } |