diff --git a/lib/lockfile.js b/lib/lockfile.js index 3cd769b..5884b88 100644 --- a/lib/lockfile.js +++ b/lib/lockfile.js @@ -4,6 +4,7 @@ const path = require('path'); const fs = require('graceful-fs'); const retry = require('retry'); const onExit = require('signal-exit'); +const createMtimeChecker = require('./mtime-checker'); const locks = {}; @@ -24,9 +25,22 @@ function resolveCanonicalPath(file, options, callback) { function acquireLock(file, options, callback) { // Use mkdir to create the lockfile (atomic operation) options.fs.mkdir(getLockFile(file, options), (err) => { - // If successful, we are done if (!err) { - return options.fs.stat(getLockFile(file, options), callback); + // At this point, we acquired the lock! + // Initialize the mtime checker and set the mtime of the lockfile + const mtimeChecker = createMtimeChecker(); + const mtime = mtimeChecker.getDate(); + + return options.fs.utimes(getLockFile(file, options), mtime, mtime, (err) => { + // If it failed, try to remove the lock.. + if (err) { + options.fs.rmdir(getLockFile(file, options), () => {}); + + return callback(err); + } + + callback(null, mtime, mtimeChecker); + }); } // If error is not EEXIST then some other error occurred while locking @@ -95,7 +109,8 @@ function updateLock(file, options) { lock.updateTimeout = setTimeout(() => { lock.updateTimeout = null; - // Check if mtime is still ours if it is we can still recover from a system sleep or a busy event loop + // Stat the file to check if mtime is still ours + // If it is we can still recover from a system sleep or a busy event loop options.fs.stat(getLockFile(file, options), (err, stat) => { const isOverThreshold = lock.lastUpdate + options.stale < Date.now(); @@ -111,7 +126,7 @@ function updateLock(file, options) { return updateLock(file, options); } - const isMtimeOurs = lock.mtimeChecker(lock.mtime, stat.mtime); + const isMtimeOurs = lock.mtimeChecker.check(lock.mtime, stat.mtime); if (!isMtimeOurs) { return setLockAsCompromised( @@ -123,7 +138,7 @@ function updateLock(file, options) { )); } - const mtime = new Date(); + const mtime = lock.mtimeChecker.getDate(); options.fs.utimes(getLockFile(file, options), mtime, mtime, (err) => { const isOverThreshold = lock.lastUpdate + options.stale < Date.now(); @@ -215,7 +230,7 @@ function lock(file, options, callback) { const operation = retry.operation(options.retries); operation.attempt(() => { - acquireLock(file, options, (err, stat) => { + acquireLock(file, options, (err, mtime, mtimeChecker) => { if (operation.retry(err)) { return; } @@ -226,10 +241,10 @@ function lock(file, options, callback) { // We now own the lock const lock = locks[file] = { - mtime: stat.mtime, + mtime, + mtimeChecker, options, lastUpdate: Date.now(), - mtimeChecker: createMtimeChecker(), }; // We must keep the lock fresh to avoid staleness @@ -310,29 +325,6 @@ function getLocks() { return locks; } -function createMtimeChecker() { - let precision; - - return (lockMtime, statMtime) => { - // If lock time was not on the second we can determine precision - if (!precision && lockMtime % 1000 !== 0) { - precision = statMtime % 1000 === 0 ? 's' : 'ms'; - } - - if (precision === 's') { - const lockTs = lockMtime.getTime(); - const statTs = statMtime.getTime(); - - // Maybe the file system truncates or rounds... - return Math.trunc(lockTs / 1000) === Math.trunc(statTs / 1000) || - Math.round(lockTs / 1000) === Math.round(statTs / 1000); - } - - // Must be ms or lockMtime was on the second - return lockMtime.getTime() === statMtime.getTime(); - }; -} - // Remove acquired locks on exit /* istanbul ignore next */ onExit(() => { diff --git a/lib/mtime-checker.js b/lib/mtime-checker.js new file mode 100644 index 0000000..e97a179 --- /dev/null +++ b/lib/mtime-checker.js @@ -0,0 +1,50 @@ +'use strict'; + +function getMtimeToProbe() { + const now = Date.now(); + + return new Date(now % 1000 === 0 ? now + 1 : now); +} + +function maybeTruncateMtime(mtime, precision) { + if (precision === 'ms') { + return mtime; + } + + const mtimeMs = Math.trunc(mtime.getTime() / 1000) * 1000; + + return new Date(mtimeMs); +} + +function calculatePrecision(probedMtime) { + return probedMtime.getTime() % 1000 === 0 ? 's' : 'ms'; +} + +function createMtimeChecker() { + let precision; + + return { + check(issuedMtime, statMtime) { + if (!precision) { + precision = calculatePrecision(statMtime); + } + + return maybeTruncateMtime(issuedMtime, precision).getTime() === maybeTruncateMtime(statMtime, precision).getTime(); + }, + getDate() { + let mtime; + + // If we already calculated the precision, return Date.now() according to it + if (precision) { + mtime = maybeTruncateMtime(new Date(), precision); + // Otherwise, return "not on the second" Date.now() in ms in order to probe + } else { + mtime = getMtimeToProbe(); + } + + return mtime; + }, + }; +} + +module.exports = createMtimeChecker; diff --git a/test/lock.test.js b/test/lock.test.js index 4846874..e4997ba 100644 --- a/test/lock.test.js +++ b/test/lock.test.js @@ -204,7 +204,7 @@ it('should retry if the lockfile was removed when verifying staleness', async () await lockfile.lock(`${tmpDir}/foo`, { fs: customFs }); expect(customFs.mkdir).toHaveBeenCalledTimes(2); - expect(customFs.stat).toHaveBeenCalledTimes(2); + expect(customFs.stat).toHaveBeenCalledTimes(1); expect(fs.statSync(`${tmpDir}/foo.lock`).mtime.getTime()).toBeGreaterThan(Date.now() - 3000); }); @@ -363,10 +363,7 @@ it('should call the compromised function if ENOENT was detected when updating th it('should call the compromised function if failed to update the lockfile mtime too many times', async () => { fs.writeFileSync(`${tmpDir}/foo`, ''); - const customFs = { - ...fs, - utimes: (path, atime, mtime, callback) => callback(new Error('foo')), - }; + const customFs = { ...fs }; const deferred = pDefer(); @@ -384,16 +381,15 @@ it('should call the compromised function if failed to update the lockfile mtime onCompromised: handleCompromised, }); + customFs.utimes = (path, atime, mtime, callback) => callback(new Error('foo')); + await deferred.promise; }, 10000); it('should call the compromised function if updating the lockfile took too much time', async () => { fs.writeFileSync(`${tmpDir}/foo`, ''); - const customFs = { - ...fs, - utimes: (path, atime, mtime, callback) => setTimeout(() => callback(new Error('foo')), 6000), - }; + const customFs = { ...fs }; const deferred = pDefer(); @@ -411,16 +407,15 @@ it('should call the compromised function if updating the lockfile took too much onCompromised: handleCompromised, }); + customFs.utimes = (path, atime, mtime, callback) => setTimeout(() => callback(new Error('foo')), 6000); + await deferred.promise; }, 10000); it('should call the compromised function if lock was acquired by someone else due to staleness', async () => { fs.writeFileSync(`${tmpDir}/foo`, ''); - const customFs = { - ...fs, - utimes: (path, atime, mtime, callback) => setTimeout(() => callback(new Error('foo')), 6000), - }; + const customFs = { ...fs }; const deferred = pDefer(); @@ -438,6 +433,8 @@ it('should call the compromised function if lock was acquired by someone else du onCompromised: handleCompromised, }); + customFs.utimes = (path, atime, mtime, callback) => setTimeout(() => callback(new Error('foo')), 6000); + await pDelay(5500); await lockfile.lock(`${tmpDir}/foo`, { stale: 5000 }); diff --git a/test/unlock.test.js b/test/unlock.test.js index 7a6995b..f0ca338 100644 --- a/test/unlock.test.js +++ b/test/unlock.test.js @@ -114,7 +114,7 @@ it('should stop updating the lockfile mtime', async () => { // First update occurs at 2000ms await pDelay(2500); - expect(customFs.utimes).toHaveBeenCalledTimes(0); + expect(customFs.utimes).toHaveBeenCalledTimes(1); }, 10000); it('should stop updating the lockfile mtime (slow fs)', async () => { @@ -133,7 +133,7 @@ it('should stop updating the lockfile mtime (slow fs)', async () => { await pDelay(3000); - expect(customFs.utimes).toHaveBeenCalledTimes(1); + expect(customFs.utimes).toHaveBeenCalledTimes(2); }, 10000); it('should stop updating the lockfile mtime (slow fs + new lock)', async () => { @@ -154,7 +154,7 @@ it('should stop updating the lockfile mtime (slow fs + new lock)', async () => { await pDelay(3000); - expect(customFs.utimes).toHaveBeenCalledTimes(1); + expect(customFs.utimes).toHaveBeenCalledTimes(2); }, 10000); it('should resolve symlinks by default', async () => {