Fix caching issue with directory listing. #38
Conversation
* Translated using Weblate (Korean) Currently translated at 100.0% (492 of 492 strings) Translation: Files/Files Translate-URL: https://l10n.elementary.io/projects/files/files/ko/ * Update link in message to report bugs. (#14) * Translated using Weblate (Korean) Currently translated at 100.0% (492 of 492 strings) Translation: Files/Files Translate-URL: https://l10n.elementary.io/projects/files/files/ko/
* Translated using Weblate (Korean) Currently translated at 100.0% (492 of 492 strings) Translation: Files/Files Translate-URL: https://l10n.elementary.io/projects/files/files/ko/ * Update link in message to report bugs. (#14) * Translated using Weblate (Korean) Currently translated at 100.0% (492 of 492 strings) Translation: Files/Files Translate-URL: https://l10n.elementary.io/projects/files/files/ko/ * Update URLs (#15) * Translated using Weblate (Indonesian) Currently translated at 100.0% (492 of 492 strings) Translation: Files/Files Translate-URL: https://l10n.elementary.io/projects/files/files/id/ * Translated using Weblate (Indonesian) Currently translated at 100.0% (492 of 492 strings) Translation: Files/Files Translate-URL: https://l10n.elementary.io/projects/files/files/id/ * Replace .bzrignore with .gitignore (#30) * Replace .bzrignore with .gitignore * Ignore files in build directory. * Remove .bzrignore * Add repology to README.md (#32) * Review messages generated by gof-directory-async. (#31) * Review messages generated by gof-directory-async. * Make expected messages more specific and mark as important for ctest in code. * Reduce many warning messages to debug where so as not to require taking into account in ctest. * Make messages indicating unexpected program events critical. Trailing space stripped. * Constant expected test messages in shared namespace. * Both Async directory class and its tests now share the namespace "GOF.Directory". * Expected test messages are constant strings in namespace "GOF.Directory.TestMessages". * Residual commented out code removed. * Fix code style. * Insert required spaces. * Translated using Weblate (Catalan) Currently translated at 100.0% (492 of 492 strings) Translation: Files/Files Translate-URL: https://l10n.elementary.io/projects/files/files/ca/ * Fix async test 2 (#35) * Remove deprecated testing for log messages. * Extra assertions for non-existent-local-test * Ensure state is NOT_LOADED after trying to load non-existent folder. * Check other file flags appropriate. * Extra assertions for load_empty_local_test. * Check state and is LOADED * Check other file flags appropriate. * Add loaded_from_cache property for testing purposes * Replace expected_message testing with a boolean property. * Translated using Weblate (Catalan) Currently translated at 100.0% (492 of 492 strings) Translation: Files/Files Translate-URL: https://l10n.elementary.io/projects/files/files/ca/ * Remove unused debugging code and correct some signatures and formatting (#34) * Remove unused debugging code; Correct some signatures; Code format. * Function for printing out the contents of the icon caches removed, as not useful for CI. * Remove message in test not useful for CI. (to be replaced with test) * Correct signatures of some functions returning Pixbufs. * Insert some missing spaces. * Remove remaining commented out code.
|
Was able to reproduce as described in #36 (comment) and with this code change the issue appears to be resolved. |
|
Thanks for the proposed fix, Vishal. I would prefer to understand a bit more why this is (sometimes) happening, although I have not been able to reproduce it yet. Then a more general solution may be possible rather than fixing this particular case. In particular, the directory should be automatically removed from the cache when the reference count drops to 1 (the toggle ref), so it sounds like there is a reference counting problem somewhere. If you could describe exactly how you reproduced this problem it would help. |
|
Looks like the directory representing the mounted USB is not necessarily cleared from the cache if it is unmounted externally to Files. Not sure your fix covers that case. Probably need to use purge_dir_from_cache () anyway when a device is unmounted. |
|
Thanks for looking at this Jeremy. In reply to you most recent two comments, the steps to reproduce this works for me as described here: #36 (comment) Also I just now checked with two windows of Files open and am able to repro on either one. Plus, the patch resolves (covers the case) of unmounting externally to Files - if I unmount from one of the Files windows the other detects it or if I do a |
|
Jeremy, maybe you are closing the Files window while trying to reproduce, hence unable to see the issue? Closing the window, of course, clears the directory cache anyway (I don't believe it's being persisted anywhere, is it?). I leave the Files window(s) open while reproducing the issue here. Also, to your point about understanding the general case, I believe Files directory cache is updated fine if the folder contents are modified (either in another Files window or in the terminal) while mounted, but when unmounted and changed on another machine, there is no way for the VolumeMonitor on the first machine to know. Plus, when plugging the USB stick back in, it mounts to the same mount point (location) which is they key on which the directory is cached, as I understand it. |
There was a problem hiding this comment.
I have not fully tested this, but I am not sure this always works. The directory cache might not contain the root location but might contain subfolders on the mount, which would not get purged.
As this precise issue does not occur in the latest version of Loki/Files anyway, the scope of this issue should be widened to a review of purging mounts of any kind and their subdirectories when unmounted by any means (not just the sidebar), to ensure they are refreshed on remounting. Although that would not fix any caching issue external to Files.
vjr
changed the title
vjr
added
the
Status: Incomplete
|
This appears to be abandoned so closing for now. |
Fix for issue #36 - caching issue with directory listing.