fix: fix mtime precision on some filesystems (#88)

Closes #82, #87

Author: satazor

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 mtimePrecision = require('./mtime-precision');
 
 const locks = {};
 
@@ -22,11 +23,24 @@ 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..
+                /* istanbul ignore if */
+                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
@@ -39,7 +53,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
@@ -95,8 +109,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
@@ -111,7 +126,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(
@@ -123,9 +138,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
@@ -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, mtimePrecision) => {
                 if (operation.retry(err)) {
                     return;
                 }
@@ -226,10 +241,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
@@ -310,29 +326,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(() => {

lib/mtime-precision.js

@@ -0,0 +1,55 @@
+'use strict';
+
+const cacheSymbol = Symbol();
+
+function probe(file, fs, callback) {
+    const cachedPrecision = fs[cacheSymbol];
+
+    if (cachedPrecision) {
+        return fs.stat(file, (err, stat) => {
+            /* istanbul ignore if */
+            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) => {
+        /* istanbul ignore if */
+        if (err) {
+            return callback(err);
+        }
+
+        fs.stat(file, (err, stat) => {
+            /* istanbul ignore if */
+            if (err) {
+                return callback(err);
+            }
+
+            const precision = stat.mtime.getTime() % 1000 === 0 ? 's' : 'ms';
+
+            // Cache the precision in a non-enumerable way
+            Object.defineProperty(fs, cacheSymbol, { value: precision });
+
+            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);
+}
+
+module.exports.probe = probe;
+module.exports.getMtime = getMtime;

test/check.test.js

@@ -108,7 +108,7 @@ it('should not resolve symlinks if realpath is false', async () => {
 it('should fail if stating the lockfile errors out when verifying staleness', async () => {
     fs.writeFileSync(`${tmpDir}/foo`, '');
 
-    const mtime = (Date.now() - 60000) / 1000;
+    const mtime = new Date(Date.now() - 60000);
     const customFs = {
         ...fs,
         stat: (path, callback) => callback(new Error('foo')),