Delete does not invalidate cache |
|||||||||||||||
Issue description- Create an empty directory - Copy then paste a file into that directory - Copy and paste a second file to the at directory - Delete the first file - Copy the first file again The file will get a (1) suffix as if the file already existed. The cache was populated via read dir on the second copy, but not invalidated by the delete.
,
Oct 15
Also the same applies for the source path for move.
,
Oct 15
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/eeb9f6ef4d44c4ed3649d1ebe3ecaad166f878ae commit eeb9f6ef4d44c4ed3649d1ebe3ecaad166f878ae Author: Zentaro Kavanagh <zentaro@chromium.org> Date: Mon Oct 15 23:01:55 2018 smbprovider: Invalidate cache on delete - Remove paths from cache when we attempt to delete them - We always remove from the cache regardless of whether delete is successful, since this is always safe and will fall back like any other cache miss BUG= chromium:894471 TEST=unittests Change-Id: I71da322d9a87d03f85d20876b32ddfa8678be046 Reviewed-on: https://chromium-review.googlesource.com/1277902 Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org> Tested-by: Zentaro Kavanagh <zentaro@chromium.org> Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org> Reviewed-by: Bailey Berro <baileyberro@chromium.org> [modify] https://crrev.com/eeb9f6ef4d44c4ed3649d1ebe3ecaad166f878ae/smbprovider/smbprovider.cc [modify] https://crrev.com/eeb9f6ef4d44c4ed3649d1ebe3ecaad166f878ae/smbprovider/metadata_cache.h [modify] https://crrev.com/eeb9f6ef4d44c4ed3649d1ebe3ecaad166f878ae/smbprovider/smbprovider_test.cc [modify] https://crrev.com/eeb9f6ef4d44c4ed3649d1ebe3ecaad166f878ae/smbprovider/smbprovider.h
,
Oct 18
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/4173abbf149557e6a5732617c93e9dea6d80eaab commit 4173abbf149557e6a5732617c93e9dea6d80eaab Author: Zentaro Kavanagh <zentaro@chromium.org> Date: Thu Oct 18 17:55:37 2018 smbprovider: Invalidate cache during move - Invalidate the source path when moving - Adds test that fails without fix BUG= chromium:894471 TEST=unittests Change-Id: I262b9681cb1fcdc5a45f01b100779e59f1958690 Reviewed-on: https://chromium-review.googlesource.com/1280963 Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org> Tested-by: Zentaro Kavanagh <zentaro@chromium.org> Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org> Reviewed-by: jimmy gong <jimmyxgong@chromium.org> Reviewed-by: Bailey Berro <baileyberro@chromium.org> [modify] https://crrev.com/4173abbf149557e6a5732617c93e9dea6d80eaab/smbprovider/smbprovider.cc [modify] https://crrev.com/4173abbf149557e6a5732617c93e9dea6d80eaab/smbprovider/smbprovider_test.cc
,
Oct 22
,
Oct 23
This fixes a bug where moving or deleting then trying to reuse the filename causes any attempt to reuse the old filename to append (1).
,
Oct 24
Your change meets the bar and is auto-approved for M71. Please go ahead and merge the CL to branch 3578 manually. Please contact milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 26
,
Oct 26
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/a7052b78a380c11ba0cc8170c9071a24901eb5d7 commit a7052b78a380c11ba0cc8170c9071a24901eb5d7 Author: Zentaro Kavanagh <zentaro@chromium.org> Date: Fri Oct 26 21:21:16 2018 smbprovider: Invalidate cache on delete - Remove paths from cache when we attempt to delete them - We always remove from the cache regardless of whether delete is successful, since this is always safe and will fall back like any other cache miss BUG= chromium:894471 TEST=unittests Change-Id: I71da322d9a87d03f85d20876b32ddfa8678be046 Reviewed-on: https://chromium-review.googlesource.com/1277902 Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org> Tested-by: Zentaro Kavanagh <zentaro@chromium.org> Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org> Reviewed-by: Bailey Berro <baileyberro@chromium.org> (cherry picked from commit eeb9f6ef4d44c4ed3649d1ebe3ecaad166f878ae) Reviewed-on: https://chromium-review.googlesource.com/c/1303114 Commit-Queue: Zentaro Kavanagh <zentaro@chromium.org> [modify] https://crrev.com/a7052b78a380c11ba0cc8170c9071a24901eb5d7/smbprovider/smbprovider.cc [modify] https://crrev.com/a7052b78a380c11ba0cc8170c9071a24901eb5d7/smbprovider/metadata_cache.h [modify] https://crrev.com/a7052b78a380c11ba0cc8170c9071a24901eb5d7/smbprovider/smbprovider_test.cc [modify] https://crrev.com/a7052b78a380c11ba0cc8170c9071a24901eb5d7/smbprovider/smbprovider.h
,
Oct 26
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/c487ad43fbe62c0962d9b3c5bcc2b8fc2d90a884 commit c487ad43fbe62c0962d9b3c5bcc2b8fc2d90a884 Author: Zentaro Kavanagh <zentaro@chromium.org> Date: Fri Oct 26 21:29:14 2018 smbprovider: Invalidate cache during move - Invalidate the source path when moving - Adds test that fails without fix BUG= chromium:894471 TEST=unittests Change-Id: I262b9681cb1fcdc5a45f01b100779e59f1958690 Reviewed-on: https://chromium-review.googlesource.com/1280963 Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org> Tested-by: Zentaro Kavanagh <zentaro@chromium.org> Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org> Reviewed-by: jimmy gong <jimmyxgong@chromium.org> Reviewed-by: Bailey Berro <baileyberro@chromium.org> (cherry picked from commit 4173abbf149557e6a5732617c93e9dea6d80eaab) Reviewed-on: https://chromium-review.googlesource.com/c/1303116 Commit-Queue: Zentaro Kavanagh <zentaro@chromium.org> [modify] https://crrev.com/c487ad43fbe62c0962d9b3c5bcc2b8fc2d90a884/smbprovider/smbprovider.cc [modify] https://crrev.com/c487ad43fbe62c0962d9b3c5bcc2b8fc2d90a884/smbprovider/smbprovider_test.cc
,
Oct 29
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 29
Not sure why the tags didn't remove. Everything is merged here. I removed the approved tags.
,
Oct 29
,
Nov 5
,
Nov 5
This looks like it might be a different issue. Can you tell me the repro steps?
,
Nov 5
I just followed the steps that you provided: - Create an empty directory - Copy then paste a file into that directory - Copy and paste a second file to the at directory - Delete the first file I wasn't able to delete the first file (the second file was deleted successfully).
,
Nov 7
,
Nov 8
,
Nov 12
jimmyxgong@ - I'm pretty sure this is not the same underlying cause as the original bug. When you find the issue please create a new bug and put a link to it here and cc ibezmenov@ on the new bug.
,
Nov 19
Hi ibezmenov@- could you try reproducing again and let us know what happens after clicking the refresh icon in the upper right hand corner. Also, if you could attach the logs of a repro from /var/log/messages and /var/log/ui that would be awesome
,
Nov 20
,
Nov 29
,
Nov 29
I was able to repro this again in M-72: - Create an empty directory - Copy then paste a file into that directory - Copy and paste a second file to the at directory - Delete the first file The first file was not deleted. Screenshot and debug-logs attached. Chrome OS: 11307.0.0, Chrome: 72.0.3623.3, Device: Nautilus
,
Nov 29
This is a different bug. It's unrelated to the caching issue (it's a file descriptor leak that happens on the first file operation). It just happens to have a similar repro steps ( though different symptoms) See https://bugs.chromium.org/p/chromium/issues/detail?id=909974 We have the fix already for that bug, it just hasn't landed yet.
,
Nov 29
Ok, when the fix for crbug.com/909974 will be landed, can we verify both bugs with the same steps?
,
Nov 29
Yes. But the two bugs are different symptoms. - This bug the problem is that the new file erroneously gets (1) appended to it - The other bug is the the very first file copied to the device can not be deleted
,
Nov 29
,
Dec 20
Verified fixed in M-73 with the following steps: - Create an empty directory - Copy then paste a file into that directory - Copy and paste a second file to the at directory - Delete the first file - Copy the first file again The first file doesn't get a (1) suffix. Chrome OS: 11435.0.0, Chrome: 73.0.3644.0, device: Nautilus |
|||||||||||||||
►
Sign in to add a comment |
|||||||||||||||
Comment 1 by tbuckley@google.com
, Oct 12