-
-
Notifications
You must be signed in to change notification settings - Fork 50
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
Windows shared lock with Python 3 #64
Comments
Sorry for the really slow response, I didn't get an email about this issue. I'm honestly a bit hesitant to change this behaviour at this point for 2 reasons:
It does sound like a serious bug though... but I'm wondering if the better fix isn't to replace the win32file locking with msvcrt instead and remove win32file completely. |
Unfortunately for me I ran into a Windows limit on the number of locked
files and moved to a different solution. I understand your hesitance and
leave it to your discretion as to how/if to change the code.
…On Mon, Sep 13, 2021 at 10:07 PM Rick van Hattem ***@***.***> wrote:
Sorry for the really slow response, I didn't get an email about this issue.
I'm honestly a bit hesitant to change this behaviour at this point for 2
reasons:
1. I can't easily test the possible scenarios
2. It breaks backwards compatibility and some people might expect
these results.
It does sound like a serious bug though... but I'm wondering if the better
fix isn't to replace the win32file locking with msvcrt instead and remove
win32file completely.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#64 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFPX4Q7QJL7WT5GS6GHGVMTUB2U47ANCNFSM45CIXDUQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
Are there any updates about this? The shared lock feature is completely broken for me right now. |
I've done some testing in the mean time and I believe the proposed solution is probably the best one. Even though it might break backwards compatibility in a few select cases. I'll create a new release within the next few days |
@martin-eder-zeiss Can you test the current develop branch? I think I've fixed it but I'm somewhat worried I might have broken something... It seems to work flawlessly for me on a Windows machine though, but I might still be missing other test cases |
I am not familiar with Windows file locking mechanisms, but is it intended that this does not throw an AlreadyLocked exception?
How can I get a shared lock when there is already an exclusive one? |
I tested it like this, and that seems to work as expected: import portalocker
with portalocker.Lock('x', flags=portalocker.LOCK_EX):
print('lock A')
with portalocker.Lock('x', flags=portalocker.LOCK_SH | portalocker.LOCK_NB):
print('lock B') And I tried this as well: import portalocker
with open('x', 'w') as a:
portalocker.lock(a, portalocker.LOCK_EX)
print('lock A')
with open('x', 'w') as b:
portalocker.lock(b, portalocker.LOCK_SH | portalocker.LOCK_NB)
print('lock B') Could it be that you reused the same import portalocker
with open('x', 'w') as a:
portalocker.lock(a, portalocker.LOCK_EX)
print('lock A')
portalocker.lock(a, portalocker.LOCK_SH | portalocker.LOCK_NB)
print('lock B') |
yep, I reused the file handle. Nevermind, everything seems to work now. |
Hello @wolph. When the fix will be released? |
I've released version 2.5.0 about 3 weeks ago. Is it still broken for you? |
Sorry, I got confused in my venv-s. I see updates in the latest release. Thanks! |
In portalocker.py for SHARED locks the code has a sys.major version conditional that uses msvcrt flags on Python 3. The problem is that win32file.LockFileEx is being used so the flags don't match, and a non-blocking flag will block instead. Just remove the "if sys.version" and always use the win32con flags since msvcrt isn't used for SHARED.
The text was updated successfully, but these errors were encountered: