From 3ecc389e6632fb16efce39ecc02fbec2b489fb49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Greg=20Berg=C3=A9?= Date: Mon, 27 Jan 2014 23:42:19 +0100 Subject: [PATCH] Fix concurrent renaming. We stock the renaming promise in cache to ensure that there is not concurrent renaming. --- lib/core/ResolveCache.js | 22 +++++++++++++++++----- test/core/resolveCache.js | 24 ++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/lib/core/ResolveCache.js b/lib/core/ResolveCache.js index 0bf503dd..5730278a 100644 --- a/lib/core/ResolveCache.js +++ b/lib/core/ResolveCache.js @@ -124,7 +124,18 @@ ResolveCache.prototype.store = function (canonicalDir, pkgMeta) { }) // Move the canonical to sourceId/target .then(function () { - return Q.nfcall(fs.rename, canonicalDir, dir) + // We store the renaming in cache. + var key = 'moving:' + dir; + var promise; + if (that._cache.has(key)) { + promise = that._cache.get(key); + } + else { + promise = Q.nfcall(fs.rename, canonicalDir, dir); + that._cache.set(key, promise); + } + + return promise .fail(function (err) { // If error is EXDEV it means that we are trying to rename // across different drives, so we copy and remove it instead @@ -132,10 +143,11 @@ ResolveCache.prototype.store = function (canonicalDir, pkgMeta) { throw err; } - return copy.copyDir(canonicalDir, dir) - .then(function () { - return Q.nfcall(rimraf, canonicalDir); - }); + return copy.copyDir(canonicalDir, dir); + }) + // To match all case, we remove the directory. + .then(function () { + return Q.nfcall(rimraf, canonicalDir); }); }); }) diff --git a/test/core/resolveCache.js b/test/core/resolveCache.js index 5ebb82f9..102879c5 100644 --- a/test/core/resolveCache.js +++ b/test/core/resolveCache.js @@ -310,6 +310,30 @@ describe('ResolveCache', function () { }) .done(); }); + + it('should be possible to store two package at same time', function (next) { + var store = resolveCache.store.bind(resolveCache, tempPackage, { + name: 'foo', + _source: 'foo', + _target: 'foo/bar' + }); + var store2 = resolveCache.store.bind(resolveCache, tempPackage2, { + name: 'foo', + _source: 'foo', + _target: 'foo/bar' + }); + + Q.all([store(), store2()]).then(function (dirs) { + var dir = dirs[0]; + expect(dir).to.equal(path.join(cacheDir, md5('foo'), 'foo%2Fbar')); + expect(fs.existsSync(dir)).to.be(true); + expect(fs.existsSync(path.join(dir, 'baz'))).to.be(true); + expect(fs.existsSync(tempPackage)).to.be(false); + expect(fs.existsSync(tempPackage2)).to.be(false); + + next(); + }).done(); + }); }); describe('.versions', function () {