Repository navigation
clear_download_cache() can cause problems if other jobs are running #9977
Description
Description
By design, clear_download_cache() completely removes the cache, including any lock that may be held. Unfortunately, if the cache is recreated, the new lock file has the same name as the old, and so processes that think they have the lock from before the cache was cleared can remove it while processes started after the clearing hold it, breaking their lock.
Expected behavior
Unclear what good behaviour in this setting would look like.
Actual behavior
clear_download_cache() while other processes are running can cause cache problems.
Steps to Reproduce
- Process A acquires the lock.
- Process B runs
clear_download_cache() - Process B acquires a lock on the new cache.
- Process A releases the lock, now on the new cache.
Now Process B thinks it has the lock but doesn't. Double unlocks are usually harmless, but other problems can occur.
Suggested mitigation: do not use no-argument clear_download_cache() (or delete the Astropy cache) while jobs are running.
Activity
aarchiba commentedon Feb 22, 2020
On Unix machines the situation can be improved by holding a file handle to the cache directory then removing the lock by using
os.rmdir('lock', dir_fd=fd). Windows does not support thedir_fdargument but we could fall back to the current method there. I imagine the nastiest cache contention cases are NFS mounts on Unix clusters.pllim commentedon Feb 24, 2020
Would it make sense to include PID in the lock name? 🤷♀
Even with arguments, if two processes happen upon the same cached URL, won't the same problem still happen?
aarchiba commentedon Feb 25, 2020
Unfortunately different processes need to hit the same lock.
No, there's just one lock for the whole cache, and clear_download_cache(url) operates normally, acquiring the lock, removing the url from the shelve (and if appropriate the file from the disk), and then releasing the lock.
You can get a different problem if someone is using a file and you clear it out from under them, but that doesn't interact with the locking in any way.
aarchiba commentedon Mar 4, 2020
One could reduce the likelihood of this causing problems by having no-argument clear_download_cache try to acquire the lock before deleting things. But, if we hit the timeout, and optionally behind a boolean flag
break_lock, we can forcibly break the lock on the assumption that something has gone wrong and we need to clean up for the user.aarchiba commentedon Mar 4, 2020
Turns out it already gives the lock a chance to time out before breaking it.
aarchiba commentedon Aug 10, 2020
I believe #10437 has essentially solved this - deleting the cache can affect processes that are depending on its contents, and on some systems this can prevent deleting or renaming files, but otherwise no concurrency surprises arise any more.