unlocking an already-unlocked( or not-locked) lock in current process is not really a no-op.

[tdzl2003]

If there's a.js running:

lf.lockSync(".lock") // do some stuff cost time. lf.unlockSync(.lock")

and b.js: lf.unlockSync(".lock") // by mistake or something

run a.js first, then b.js, then another a.js, it's possible that lock on a.js is unlocked by b.js, which shouldn't happen.

[othiym23]

When this happens, it's an error, but how is it a flaw in lockfile? And how would you suggest that it be remedied? To me it sounds like you're discussing a user error, but not a bug in the package.

The purpose of this library is to allow multiple processes to coordinate exclusive access to a shared resource. It doesn't prescribe exactly how that is done, but it does all it can to make sure that given the two operations of locking and unlocking a resource, it will do so safely. Like any form of manual resource management, though, it doesn't – and can't, without limiting its usefulness – prevent users from doing things that are errors in the context of their own apps.

[tdzl2003]

In my opinion, It would be better to be either a no-op or throw a error if a “not-locked" lock was unlocked. It can be tested simply by checking locks[path]. Silently remove the file is worst.

[othiym23]

The main thing that lockfile is used for in npm is to communicate across processes. Sure, it's possible to use unlock wrong, but it's not inherently wrong for one process to unlock a lock created by another process.

[tdzl2003]

https://github.com/npm/npm/blob/932b86e2e9930eb5d4dc0f0853ec627870013786/lib/install.js

Actually, in npm, the process who created the lock do the unlock thing.

Because all locks locked in process will be unlocked when process exit, I think it's rarely necessary to unlock a lock created by another process. (It happens only while creator process is still alive.) This caused unexpected result in most case (with wrong use).

I think lockfile acts like a "named mutex" across processes, so It's more likely the creator "own" the lock. That's why auto-unlocking happens when process exit.

[isaacs]

This request is indeed outside the scope of the original design of this module. However, I think there's something of value here that is worth exploring a little bit.

Lockfiles are, as @othiym23 says, designed to facilitate cross-process communication. Additionally, sometimes processes die in interesting ways, and lockfiles are not removed. It must be possible to remove old expired locks when the application decides that a lock is no longer relevant. (For example, if it is beyond a certain age, etc.)

It is assumed by the design/intent of this module that when a process exits, all of its locks expire. Though it happens occasionally due to the vagaries of abrupt process termination, you should not (usually) be holding a lock longer than the life of the process. So, this kind of situation should not be by design:

1. process 100 takes a lock, and then exits.
2. process 101 tries to acquire lock, fails, waits.
3. process 102 unlocks the file
4. process 101 now can acquire the lock

What might happen instead is:

1. process 100 takes a lock, and then exits
2. process 101 tries to acquire the lock, fails, waits
3. waits long enough that lock expires, acquires lock

Nevertheless, there should probably not ever be a case where you unlock() a lock that you did not create. Of course, you can manually unlink the file, but that doesn't have the same semantics as lock/unlock.

We could raise an error when you do lockfile.unlock('file-i-didnt-lock', cb). I'm not sure if this is ever going to not be a bug. Of course, it means that, internally, this module will need to use fs.unlink explicitly in those cases, but that's fine. Also, note that unlocking a file that doesn't exist needs to still be a no-op (maybe you just held the lock for too long, and someone else booted you, but then eventually you did come back and unlock it).

Of course, that'll be a significant breaking change, semver major, blah blah blah.

@tdzl2003 would that solve your problem? @othiym23 what's your feels on this?

[tdzl2003]

Seems good.