Several places in Chrome delete leveldb's with base::DeleteFile. |
||||
Issue descriptionDeleteFile() should be avoided because DestroyDB() will fail if it cannot acquire the database lock, but DeleteFile() does not make this check. There are a bunch of places in Chrome that attempt to open a leveldb via leveldb::OpenDB(), and if this call fails they will delete the database via DeleteFile. Most of these should be converted to DestroyDB(). A few other notes. 1. It is probably OK to use DeleteFile to delete a database if the open failed due to corruption. However, if there could ever be another thread/process trying to access that database DestroyDB should be used. 2. DestroyDB() will only delete leveldb files and will leave behind non-leveldb files if they somehow exist in that directory. If the function really wants to ensure that everything is deleted then please use DestroyDB, and if successful follow it by DeleteFile. Here are the locations where DeleteFile is used. 1. DeltaFileBackend::Init. 2. SessionStorageDatabase::LazyOpen. 3. DeltaFileBackend::Clear. 4. UsageReportsBufferBackend::Init. 5. UsageReportsBufferBackend::Clear. 6. MetadataDatabase::ClearDatabase. 7. DataStoreImpl::RecreateDB. 8. ResourceMetadataStorage::UpgradeOldDB (may be correct). 9. ResourceMetadataStorage::Initialize (may be correct). 10. ServiceWorkerDatabase::DestroyDatabase. (should first DestroyDB and then DeleteFile). 11. SandboxDirectoryDatabase::Init
,
May 22 2018
,
Jun 4 2018
SessionStorageDatabase (session_storage_database.cc) will soon be going away in favor of the mojo sessionStorage implementation. So won't be doing #2.
,
Jun 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/61befda0bc6d794fdbfdf038836d5cdca079bf5a commit 61befda0bc6d794fdbfdf038836d5cdca079bf5a Author: Chris Mumford <cmumford@chromium.org> Date: Tue Jun 19 22:27:44 2018 Switched from DeleteFile() to DestroyDB(). DeleteFile ignores db lock file so leveldb_chrome::DestroyDB is preferred. Bug: 802298 Change-Id: I15af9ae4c402f94d537a5b4495e8a60f6dd906fe Reviewed-on: https://chromium-review.googlesource.com/1069415 Commit-Queue: Chris Mumford <cmumford@chromium.org> Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org> Reviewed-by: Hidehiko Abe <hidehiko@chromium.org> Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org> Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org> Reviewed-by: Doug Arnett <dougarnett@chromium.org> Cr-Commit-Position: refs/heads/master@{#568620} [modify] https://crrev.com/61befda0bc6d794fdbfdf038836d5cdca079bf5a/chrome/browser/android/history_report/delta_file_backend_leveldb.cc [modify] https://crrev.com/61befda0bc6d794fdbfdf038836d5cdca079bf5a/chrome/browser/android/history_report/usage_reports_buffer_backend.cc [modify] https://crrev.com/61befda0bc6d794fdbfdf038836d5cdca079bf5a/chrome/browser/sync_file_system/drive_backend/metadata_database.cc [modify] https://crrev.com/61befda0bc6d794fdbfdf038836d5cdca079bf5a/chrome/browser/sync_file_system/drive_backend/metadata_database_unittest.cc [modify] https://crrev.com/61befda0bc6d794fdbfdf038836d5cdca079bf5a/components/data_reduction_proxy/core/browser/data_store_impl.cc [modify] https://crrev.com/61befda0bc6d794fdbfdf038836d5cdca079bf5a/components/drive/resource_metadata_storage.cc [modify] https://crrev.com/61befda0bc6d794fdbfdf038836d5cdca079bf5a/storage/browser/fileapi/sandbox_directory_database.cc
,
Jun 22 2018
Still outstanding: SessionStorageDatabase::LazyOpen SandboxOriginDatabase::Init SandboxOriginDatabase::RepairDatabase SandboxOriginDatabase::RemoveDatabase
,
Jun 25 2018
,
Jun 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c182c9896c420f4ee5b88f0b6694cef9ec7cfc28 commit c182c9896c420f4ee5b88f0b6694cef9ec7cfc28 Author: Chris Mumford <cmumford@chromium.org> Date: Tue Jun 26 15:47:50 2018 Using leveldb_chrome::DeleteDB in LevelDBSiteCharacteristicsDatabase. base::DeleteFile ignores database locks, leveldb_chrome::DeleteDB does not and is safer. Additionally did some minor cleanup: 1. Combined ReportRepairResult and RepairDatabase. 2. Removed unnecessary resets of unique_ptr: db_. Bug: 802298 Change-Id: I0c3c15e4a55bf27ede237353f55e79a41c796d59 Reviewed-on: https://chromium-review.googlesource.com/1113814 Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org> Reviewed-by: François Doray <fdoray@chromium.org> Commit-Queue: Chris Mumford <cmumford@chromium.org> Cr-Commit-Position: refs/heads/master@{#570415} [modify] https://crrev.com/c182c9896c420f4ee5b88f0b6694cef9ec7cfc28/chrome/browser/resource_coordinator/leveldb_site_characteristics_database.cc
,
Jul 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2947750744a16354dbf9329bed030b3fa9c900cd commit 2947750744a16354dbf9329bed030b3fa9c900cd Author: Chris Mumford <cmumford@chromium.org> Date: Thu Jul 26 19:51:30 2018 SessionStorage: Using DeleteDB to delete database. base::DeleteFile ignores database locks, leveldb_chrome::DeleteDB does not and is safer. Bug: 802298 Change-Id: I6b30fb6826216c9e54a16569a133e1fbddfad98f Reviewed-on: https://chromium-review.googlesource.com/1151543 Commit-Queue: Chris Mumford <cmumford@chromium.org> Commit-Queue: Daniel Murphy <dmurph@chromium.org> Reviewed-by: Daniel Murphy <dmurph@chromium.org> Cr-Commit-Position: refs/heads/master@{#578406} [modify] https://crrev.com/2947750744a16354dbf9329bed030b3fa9c900cd/content/browser/dom_storage/session_storage_database.cc
,
Jul 26
|
||||
►
Sign in to add a comment |
||||
Comment 1 by jsb...@chromium.org
, Jan 18 2018