The cache lock cannot be removed on Windows 10 if placed inside the user home directory.

[olegvereshchynsky]

I'm trying to create a TokenCache on Windows 10 and store a cache at

~/.myapp/token.cache

As a result the lockfile is created at:

~/.myapp/token.cache.lockfile

However this lockfile is not removed in CrossPlatLock.__exit__ and it is completely blocked by the operationg system (no visible process) untill the further reboot. It looks like the file is not yet unlocked when being removed (I may be wrong) creating some kind of a hanging file handle.

Here is a simple way to reproduce:

path = os.path.expanduser('~/.myapp/test.lockfile')
with CrossPlatLock(path):
    print('hi')
# One more time
with CrossPlatLock(path):
    print('hi')
    
<ipython-input-117-002d28c082a3> in <module>                                           
----> 1 with CrossPlatLock(path):                                                      
      2     print('hi')                                                                
      3                                                                                
                                                                                       
<ipython-input-116-89428a9dd969> in __enter__(self)                                    
     11         pid = os.getpid()                                                      
     12                                                                                
---> 13         self._fh = open(self._lockpath, 'wb+', buffering=0)                    
     14         portalocker.lock(self._fh, portalocker.LOCK_EX)                        
     15         self._fh.write('{} {}'.format(pid, sys.argv[0]).encode('utf-8'))       
                                                                                       
PermissionError: [Errno 13] Permission denied: 'C:\\Users\\XXXXYYY/.myapp/test.lockfile'

Explicitly unlocking the file before closing it solves the issue:

portalocker.unlock(self._fh)

I know that portalocker claims that the explicit unlock is not necessary, but would it be possible to add it to prevent the problem?

[rayluo]

@olegVer Thanks for bringing this to our attention. We will need to investigate this before we can make a comment on this.

Meanwhile, a couple quick notes to you:

• If you believe this is a discrepancy between the actual behavior and portalocker's claims, have you tried open an issue there and let's hear that package author's input?
• You are probably already aware, this extension module relies on those locking mechanism to orchestrate token cache file sharing across multiple processes. But if your application happens to be the only process maintaining its own token cache file, and does NOT intend to share it with other processes, you do NOT have to deal with these locking in the first place. You can use a naive cache mechanism, described here.

[olegvereshchynsky]

@rayluo Thanks!
Answering your questions:

• I've submitted an issue on portalocker as well.
• Well, it may be the case that the cache is shared (I've noticed the problem first time with the default location at ~/AppData/Local/.IdentityService). Also it provides the encrypted versions of the cache for Windows and macOS, which I need.
• I've also noticed, that if I remove writing to the file from __enter__ function of the CrossPlatLock - it works as well.
• For now I'll switch to naive cache indeed.

[wolph]

• If you believe this is a discrepancy between the actual behavior and portalocker's claims, have you tried open an issue there and let's hear that package author's input?

I'm guessing that part of the normal unlocking mechanism relies on the Python garbage collector a bit. The code the library uses for the same method internally looks like this: https://github.com/WoLpH/portalocker/blob/e5e7d760488539857fdeff65231d40c2a1dcdd88/portalocker/utils.py#L165-L169

The only notable difference is that the file handle gets cleared:

>>> import portalocker
>>> file = open('somefile', 'r+')
>>> portalocker.lock(file, portalocker.LOCK_EX)
>>> file.seek(12)
>>> file.write('foo')
>>> file.close()
# The big difference:
>>> file = None

It's very well possible that this part of the documentation is simply in error. The library has mostly been tested on Linux/Unix systems where locks are advisory by default. Windows forces locking by default so it behaves different.

[rayluo]

Thank you both for the valuable input!

@wolph So you hinted that we add an explicit file = None? @olegVer Have you tested that to see whether that also works for you?

@wolph What if we add portalocker.unlock(self._fh)? @olegVer already tested that one and said it worked. I know this approach slightly goes against your doc in portalock, but, it seems an explicit symmetric lock-and-then-unlock pattern sounds better than relying on implicit GC.

PS: @wolph , I'll give portalocker.Lock more thoughts. Previously we did not use it directly, partially because we would need to write some extra content to that locked file.

[wolph]

@wolph So you hinted that we add an explicit file = None? @olegVer Have you tested that to see whether that also works for you?

Honestly, I haven't tested it myself personally. But this has been the state of the library for quite a while and it's a library that's used quite a bit so I'm simply guessing it's not an issue.

@wolph What if we add portalocker.unlock(self._fh)? @olegVer already tested that one and said it worked. I know this approach slightly goes against your doc in portalock, but, it seems an explicit symmetric lock-and-then-unlock pattern sounds better than relying on implicit GC.

There's nothing wrong with explicitly unlocking so that's definitely a good option.
Seeing that this bug report exists would suggest the doc is at least wrong for Windows.

PS: @wolph , I'll give portalocker.Lock more thoughts. Previously we did not use it directly, partially because we would need to write some extra content to that locked file.

It's definitely not a necessity of course but if more issues similar to this one pop-up for some reason, there's a change that it would be fixed transparently for you if you would inherit the class :)

[olegvereshchynsky]

I've performed few more tests - it appears that it's actually writing to the file that triggers the problem - if removed - it works.

So, here is the summary:

Solution	Solves the issue
Code as is	No
Explicit portalocker.unlock	Yes
Explicit self._fh = None	No
Removing writing to self._fh	Yes
Using portalocker.Lock (no writing during aquisition)	Yes

[rayluo]

Thanks @wolph for the insight, and thanks @olegVer for such a detailed test result!

Then we will go with "explicit portalocker.unlock, because we would have to write something in that file due to some other requirement. We'll send out a PR for this.

[wolph]

The new release of portalocker has the explicit unlock in portalocker.Lock