-
Notifications
You must be signed in to change notification settings - Fork 29
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
Investigate: The lock waiting timeout (5 secs) is not observed #66
Comments
I spent a bit of time investigating this. The root cause of the issue seems to be Portalocker - it uses a completely different mechanism to acquire file locks, which is not compatible with .NET and Java implementations. Portalocker offers file locking semantics only to others apps using Portalocker. On Windows it seems to be using a Win32 mechanism called LockFileEx. Java and .NET are using a FileStream locking approach, which behaves differently. |
I managed to repro the issue using @abhidnya13 's test app cache_file_generator and I've created an indentical app on .NET side: https://github.com/AzureAD/microsoft-authentication-extensions-for-dotnet/blob/1a2de3a3ae272f288dbeda8fc242e7846892848f/tests/FileLockApp/Program.cs#L12 If you run the python app first (i.e. create the lock from py), the .NET app waits for the lock to go away. If you run the .NET app first however, the python app crashes because it cannot acquire the lock. |
Thanks Bogdan for the investigation! We originally chose portalocker in order to oursource such task to it, but we were not aware of there are more than one (incompatible?) way to lock. :-/ @abhidnya13 we will need to see whether it is feasible for us to switch to the .net and java way. I also started a conversation in Portalocker repo. By the way, @bgavrilMS , in #63, we followed MSAL EX Java to add an extra layer of file exclusive creation as a (weak) prerequisite of lock, which is likely a more universal approach. When/if MSAL EX .Net also adopts that, this issue would probably also go away. Will MSAL EX .Net adopt that anytime soon? |
Quoted from @bgavrilMS
CC @SomkaPe
The text was updated successfully, but these errors were encountered: