diff --git a/lib/lockfile.js b/lib/lockfile.js index 2078e82..ff6d183 100644 --- a/lib/lockfile.js +++ b/lib/lockfile.js @@ -128,7 +128,7 @@ function updateLock(file, options) { options.fs.utimes(getLockFile(file, options), mtime, mtime, (err) => { const isOverThreshold = lock.lastUpdate + options.stale < Date.now(); - // Ignore if the lock was released + // Ignore if the lock was compromised or released if (lock.released) { return; } @@ -162,13 +162,14 @@ function updateLock(file, options) { // may be using this module outside of NodeJS (e.g., in an electron app), // and in those cases `setTimeout` return an integer. /* istanbul ignore else */ - if (lock.updateTimeout.unref) { + if (lock.updateTimeout && lock.updateTimeout.unref) { lock.updateTimeout.unref(); } } function setLockAsCompromised(file, lock, err) { - // Signal the lock has been released + // Signal the lock has been compromised + lock.compromised = true; lock.released = true; // Cancel lock mtime update @@ -237,8 +238,13 @@ function lock(file, options, callback) { callback(null, (releasedCallback) => { if (lock.released) { - return releasedCallback && - releasedCallback(Object.assign(new Error('Lock is already released'), { code: 'ERELEASED' })); + let error; + + if (!lock.compromised) { + error = Object.assign(new Error('Lock is already released'), { code: 'ERELEASED' }); + } + + return releasedCallback && releasedCallback(error); } // Not necessary to use realpath twice when unlocking diff --git a/test/release.test.js b/test/release.test.js index 3150b31..b769a92 100644 --- a/test/release.test.js +++ b/test/release.test.js @@ -5,6 +5,7 @@ const mkdirp = require('mkdirp'); const rimraf = require('rimraf'); const lockfile = require('../'); const unlockAll = require('./util/unlockAll'); +const { waitUntil } = require('./util/wait'); const tmpDir = `${__dirname}/tmp`; @@ -52,3 +53,32 @@ it('should fail when releasing twice', async () => { expect(err.code).toBe('ERELEASED'); } }); + +it('shouldnt error on releasing compromised lock', async () => { + const lockPath = `${tmpDir}/foo`; + const lockDir = `${lockPath}.lock`; + const stale = 10; + const onCompromised = jest.fn(); + + jest.useFakeTimers(); + fs.writeFileSync(lockPath, ''); + + const release = await lockfile.lock(lockPath, { stale, onCompromised }); + const mtime = (new Date().getTime() / 1000) - stale; + + fs.utimesSync(lockDir, mtime, mtime); + + // Advance timers to trigger the updateTimeout so the lock gets compromised + jest.advanceTimersByTime(stale * 2 * 1000); + + // switch back to use realTimers to wait max 5 seconds as there is no + // other way to wait for all callbacks from the internal timeout to return + jest.useRealTimers(); + await waitUntil(() => onCompromised.mock.calls.length, 5, 50); + + expect(onCompromised).toHaveBeenCalledTimes(1); + + await expect(release()).resolves.not.toThrow(); + expect(fs.existsSync(lockDir)).toBe(true); +}); + diff --git a/test/util/wait.js b/test/util/wait.js new file mode 100644 index 0000000..9232749 --- /dev/null +++ b/test/util/wait.js @@ -0,0 +1,23 @@ +'use strict'; + +function waitFor(ms) { + return new Promise((resolve) => setTimeout(resolve, ms || 0)); +} + +async function waitUntil(condition, duration = 20, interval = 250) { + let iterator = -1; + const steps = Math.ceil(duration * 1000 / interval); + + let ready; + + do { + ready = condition(); + await waitFor(interval); // eslint-disable-line no-await-in-loop + iterator += 1; + } while (!ready && iterator < steps); + + return ready; +} + +module.exports.waitFor = waitFor; +module.exports.waitUntil = waitUntil;