-
-
Notifications
You must be signed in to change notification settings - Fork 107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Does not successfully lock on Windows #82
Comments
I've just been looking at this, the problem seems to be:
Consulting: https://docs.python.org/3/library/msvcrt.html Looks to me that _aquire needs the ability to report that it failed and a new attempt to be made. |
@cbehopkins if we could create a test case it should be caught on #85 which adds Windows as a target |
Turns out it was PEBKAC, but the pattern to using it with pytest - what directed me here - if you are trying to do other file operations if you are also trying to clean up afterwards, is non-trivial and masquerades as FileLock not working. My mistake in the understanding above comes from not realising the design was such that the lock is per thread, not absolute. i.e.: locker = FileLock("dbg_locking.lock")
assert not locker.is_locked
locker.acquire(timeout=1)
assert locker.is_locked
locker.acquire(timeout=0.1) does not raise an exception, as the second lock acquire gets the lock just fine(as it knows this thread has the lock). This clouded my understanding as I have not experienced a lock behaving like this previously. |
Hello, if you make a PR for this (with tests) we would be happy to review it, thanks! |
We should probably document better that our platform locks are re-entrant. |
This issue seems to also apply to MacOS 12.6.3 as well (just to confirm it is not a Windows specific issue).
Perhaps clearer documentation that filelock shares the lock between threads? This is different than the lock just being re-entrant. |
The threads are fake. Python isn't freely multi-threaded. There's just 1 process in Python, and it only runs 1 thread at a time via the "Global Interpreter Lock (GIL)", which divides up the work using fakery. The locks are at the file-level and are registered via OS-kernel features. The OS cannot differentiate between the different fake Python threads. It's all just "that Python process" to the OS, so no it's not possible to do this differently. And those OS-kernel locking features are used to ensure that there aren't race conditions and other issues that homebrew locking solutions always have. |
I've opened a PR to resolve this: #219.. It was something that hit me all of a sudden and makes complete sense. By forcing the WindowsLock to be thread-local, it avoids any weird issues that can crop up with multiple python threads and the os thinking that its the same process so unlock, etc. |
If we wanted to 'fix' this everywhere maybe we should just make BaseFileLock inherit from threading.local. Edit: I've updated the PR to do this since folks have hit this in posix-land too. |
Hi,
On Windows 10.0.19041.687 Pro x64.
Python 3.7.0: x64.
Here is a test script:
Example run:
Using a ThreadPoolExecutor seems to lead to assertion errors making me think the lock file wasn't atomically created. If I sleep a bit (like .01), after doing submit() it seems to work, but sort of defeats the purpose of the test.
The text was updated successfully, but these errors were encountered: