Race condition in singleton.py about tendo HOT 11 CLOSED

Hey @dirk-attraktor, I've noticed your issue but haven't read into the detailed code yet.

Would you like to provide a pull request that I can merge and release? I'll test the code.

from tendo.

The real fix is to remove
fcntl.lockf(self.fp, fcntl.LOCK_UN)
Because lock will be removed when you delete the file

from tendo.

No thats not "the real fix". Did you even bother to run the test code?
I updated the test code to a nicer testing suite:

EDIT: what @andrew-azarov says.

Regards
Dirk

from tendo.

I know this "Delete removes lock" is stated all over the internet, but what it actually means is
"delete removes the lock and then deletes the file".

Not exactly. It is done atomically at the FS level. What it means - nothing can read or write or lock the file until the bulk transaction is finished.

In your way there is short short time between calls. So say run your program with cpulimit -l 1 for example (this will limit it to 1% of available CPU and it will be really slow) and if you unlock the file and then another process comes to terms to lock it within the timeframe between calls to unlock and to delete means your app will allow the second app to run.

The probability is low but it is there and if it happens - it would be horrible to debug because your code advertises as a locking code

The only correct one which works atomically in all modern operating systems is https://github.com/dirk-attraktor/Singleton-Test/blob/master/SingletonOsremove.py .

However to atomically lock after creation you need to os.open it with the flags to lock before and not just create the file and lock it (same race condition).

from tendo.

Could you elaborate the last sentence?
"you need to os.open it with the flags to lock before and not just create the file and lock it"
Can you show an example how to do that?
I tried to open the file with os.O_SHLOCK or os.O_EXLOCK, but both flags seem to be BSB specific and don't exist on linux. https://docs.python.org/2/library/os.html#open-constants

Regards
Dirk

from tendo.

Ok i did some more testing. You are right:

The correct way is

os.open(self.lockfile, os.O_CREAT | os.O_EXCL | os.O_RDWR)

to acquire a lock, and

os.remove(self.lockfile)

to release it. No fcntl needed what so ever.

Regards
Dirk

from tendo.

from tendo.

This is not done yet :)

What should happen if the program crashes or the computer just suddenly resets?
__del__ might not be called, and the lock file won't be deleted, effectively stalling further code executions until the lock file is manually deleted.
Thats not a good solution.
This does not happen with fcntl, but then the lock file will be around forever.

from tendo.

@dirk-attraktor that's an easy one and solved long time before us for daemons (it's called .pid file).

Put the lockfile creator PID into the file.
When another script runs and reads the file but finds no process - it can replace the line with its own PID.

from tendo.

So I made the module based on your singleton. It passes all the unittests however they are not very wide. I'd really want you to test it out and include if you want it. Also I saw your software marks MIT-compatible license (BSD), however I cannot find any copyright/LICENSE in any files except your main tendo project where there are just licenses from PSF which hardly work here.

https://github.com/andrew-azarov/singletony

from tendo.

I don't use this module anymore but if someone makes PRs, I will review, test, merge and release new versions.

from tendo.