arvenil/mutex

FlockLock failed to remove temporary lock file.

laurent-clouet opened this issue · 4 comments

laurent-clouet commented

Hello,

I'm using FlockLock and after usage i have a lot of .lock file in the working directory.
I tried to look for a cleanup function but i didn't found one, I'm guessing the .lock files should be automatically removed after usage.
I'm using lock like this:

$key = 'some_key';
$value = 'some_value';
$lock = new NinjaMutex\Lock\FlockLock("/tmp");
$mutex = new NinjaMutex\Mutex($key, $lock);
if ($mutex->acquireLock(1000)) {
$mutex->releaseLock();
unset($mutex);
}

After running the code i have "/tmp/some_key.lock" file remaining.
Did I do something wrong or is it a bug ?

I tried on 5.6.17-0+deb8u1 (Debian), 5.6.11-1ubuntu3.1 (Ubuntu) i have the same behaviour.

Thanks!

arvenil commented

I'm open for pull requests but it's hard for me to find time for this library especially that I didn't touch php for like 2 years.
Just be aware that removing file might not be an easy approach as you are introducing another operation you need to be sure doesn't generate race condition.
I would rather expect developing new method of locking based on creation and deletion of files being atomic operations - but that should be first verified and tested... and the drawback would be that lock remain if program crashes.
So, on second thought... the only reasonable way would be your original idea to find a way to delete file in flock flow.

laurent-clouet commented

My point is, it's the library who created this .lock file when the mutex created so it should be the lib who should delete it.
There is no api for that and when i looked at the code I didn't saw any remove/unlink/delete.
File lock are a bit different from the other lock because they need a temporary element (a file) but it was not really thought on the FlockLock class.
I'll check if i can to a PR but first i want to be sure it's not me who is using the lib wrongly.

arvenil commented

In other words, believe me I understand the point that this library should do some cleanups and I totally agree with that:) Sadly it's not as easy as it may seams so. So if someone will provide a way without introducing race conditions I would be more than happy to merge it.