Chrome should now crash when failing to open leveldb |
|||
Issue descriptionIdentified via code inspection. There are three places in Chromium where a database (leveldb) is opened, and a CHECK() is used to verify that the database was successfully opened. These CHECK's are in non-critical areas of Chrome, and crashing the browser process should not be the correct behavior in this situation. The three places are: 1. chrome/browser/android/history_report/delta_file_backend_leveldb.cc: DeltaFileBackend::Init 2. chrome/browser/android/history_report/usage_reports_buffer_backend.cc: UsageReportsBufferBackend::Init 3. third_party/leveldatabase/env_chromium.cc: DBTracker::OpenDatabase
,
Feb 12 2018
In DBTracker::OpenDatabase the assert is: CHECK((status.ok() && db) || (!status.ok() && !db)); ... which seems legit; i.e. if an error was reported db should be null; if no error was reported, db should not be null. Is that assertion about the leveldb API invalid?
,
Feb 12 2018
Similarly, the two android/history_report cases have:
if (status.ok()) {
CHECK(db_);
return true;
}
Is it ever expected that leveldb would return successful status but no database?
,
Feb 12 2018
jsbell@: Adding you to CC so you see my reply. The CHECKs should not crash. I thought we added the DBTracker check in a code review that I was involved in. I might have suggested the current version -- sorry it's confusing! Do we have any crash/ data suggesting this is a problem? If not, I'd close the bug. |
|||
►
Sign in to add a comment |
|||
Comment 1 by dtrainor@chromium.org
, Feb 8 2018