Skip to content

Commit

Permalink
fix: fix mtime precision on some filesystems
Browse files Browse the repository at this point in the history 
Closes #82, #87
  • Loading branch information
@satazor
satazor committed Apr 3, 2019
1 parent c0cdea2 commit a88b324
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 52 deletions.
62 changes: 27 additions & 35 deletions lib/lockfile.js
View file
Expand Up @@ -4,6 +4,7 @@ const path = require('path');
const fs = require('graceful-fs');
const retry = require('retry');
const onExit = require('signal-exit');
const mtimePrecision = require('./mtime-precision');

const locks = {};

Expand All @@ -22,11 +23,23 @@ function resolveCanonicalPath(file, options, callback) {
}

function acquireLock(file, options, callback) {
const lockfilePath = getLockFile(file, options);

// Use mkdir to create the lockfile (atomic operation)
options.fs.mkdir(getLockFile(file, options), (err) => {
// If successful, we are done
options.fs.mkdir(lockfilePath, (err) => {
if (!err) {
return options.fs.stat(getLockFile(file, options), callback);
// At this point, we acquired the lock!
// Probe the mtime precision
return mtimePrecision.probe(lockfilePath, options.fs, (err, mtime, mtimePrecision) => {
// If it failed, try to remove the lock..
if (err) {
options.fs.rmdir(lockfilePath, () => {});

return callback(err);
}

callback(null, mtime, mtimePrecision);
});
}

// If error is not EEXIST then some other error occurred while locking
Expand All @@ -39,7 +52,7 @@ function acquireLock(file, options, callback) {
return callback(Object.assign(new Error('Lock file is already being held'), { code: 'ELOCKED', file }));
}

options.fs.stat(getLockFile(file, options), (err, stat) => {
options.fs.stat(lockfilePath, (err, stat) => {
if (err) {
// Retry if the lockfile has been removed (meanwhile)
// Skip stale check to avoid recursiveness
Expand Down Expand Up @@ -95,8 +108,9 @@ 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
options.fs.stat(getLockFile(file, options), (err, stat) => {
// 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(lock.lockfilePath, (err, stat) => {
const isOverThreshold = lock.lastUpdate + options.stale < Date.now();

// If it failed to update the lockfile, keep trying unless
Expand All @@ -111,7 +125,7 @@ function updateLock(file, options) {
return updateLock(file, options);
}

const isMtimeOurs = lock.mtimeChecker(lock.mtime, stat.mtime);
const isMtimeOurs = lock.mtime.getTime() === stat.mtime.getTime();

if (!isMtimeOurs) {
return setLockAsCompromised(
Expand All @@ -123,9 +137,9 @@ function updateLock(file, options) {
));
}

const mtime = new Date();
const mtime = mtimePrecision.getMtime(lock.mtimePrecision);

options.fs.utimes(getLockFile(file, options), mtime, mtime, (err) => {
options.fs.utimes(lock.lockfilePath, mtime, mtime, (err) => {
const isOverThreshold = lock.lastUpdate + options.stale < Date.now();

// Ignore if the lock was released
Expand Down Expand Up @@ -215,7 +229,7 @@ function lock(file, options, callback) {
const operation = retry.operation(options.retries);

operation.attempt(() => {
acquireLock(file, options, (err, stat) => {
acquireLock(file, options, (err, mtime, mtimePrecision) => {
if (operation.retry(err)) {
return;
}
Expand All @@ -226,10 +240,11 @@ function lock(file, options, callback) {

// We now own the lock
const lock = locks[file] = {
mtime: stat.mtime,
lockfilePath: getLockFile(file, options),
mtime,
mtimePrecision,
options,
lastUpdate: Date.now(),
mtimeChecker: createMtimeChecker(),
};

// We must keep the lock fresh to avoid staleness
Expand Down Expand Up @@ -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(() => {
Expand Down
54 changes: 54 additions & 0 deletions lib/mtime-precision.js
View file
@@ -0,0 +1,54 @@
'use strict';

const symbol = Symbol();

function probe(file, fs, callback) {
const cachedPrecision = fs[symbol];

if (cachedPrecision) {
return fs.stat(file, (err, stat) => {
if (err) {
return callback(err);
}

callback(null, stat.mtime, cachedPrecision);
});
}

// Set mtime by ceiling Date.now() to seconds + 5ms so that it's "not on the second"
const mtime = new Date((Math.ceil(Date.now() / 1000) * 1000) + 5);

fs.utimes(file, mtime, mtime, (err) => {
if (err) {
return callback(err);
}

fs.stat(file, (err, stat) => {
if (err) {
return callback(err);
}

const precision = fs[symbol] = stat.mtime.getTime() % 1000 === 0 ? 's' : 'ms';

callback(null, stat.mtime, precision);
});
});
}

function getMtime(precision) {
let now = Date.now();

if (precision === 's') {
now = Math.ceil(now / 1000) * 1000;
}

return new Date(now);
}

function clearCache(fs) {
delete fs[symbol];
}

module.exports.probe = probe;
module.exports.getMtime = getMtime;
module.exports.clearCache = clearCache;
31 changes: 14 additions & 17 deletions test/lock.test.js
View file
Expand Up @@ -278,22 +278,22 @@ it('should fail if removing a stale lockfile errors out', async () => {
it('should update the lockfile mtime automatically', async () => {
fs.writeFileSync(`${tmpDir}/foo`, '');

await lockfile.lock(`${tmpDir}/foo`, { update: 1000 });
await lockfile.lock(`${tmpDir}/foo`, { update: 1500 });

expect.assertions(2);

let mtime = fs.statSync(`${tmpDir}/foo.lock`).mtime;

// First update occurs at 1000ms
await pDelay(1500);
// First update occurs at 1500ms
await pDelay(2000);

let stat = fs.statSync(`${tmpDir}/foo.lock`);

expect(stat.mtime.getTime()).toBeGreaterThan(mtime.getTime());
mtime = stat.mtime;

// Second update occurs at 2000ms
await pDelay(1000);
// Second update occurs at 3000ms
await pDelay(2000);

stat = fs.statSync(`${tmpDir}/foo.lock`);

Expand Down Expand Up @@ -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();

Expand All @@ -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();

Expand All @@ -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();

Expand All @@ -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 });
Expand Down

0 comments on commit a88b324

Please sign in to comment.