DCHECK followe by error handling in SQLite DB corruption case
Reported by
tsniatow...@opera.com,
Oct 12 2017
|
||||||
Issue descriptionhttps://cs.chromium.org/chromium/src/net/extras/sqlite/sqlite_persistent_cookie_store.cc?q=sqlite_persistent_cookie_store.cc&sq=package:chromium&dr&l=661 if (!db_->Open(path_)) { NOTREACHED() << "Unable to open cookie DB."; if (corruption_detected_) db_->Raze(); ... This is a DCHECK/NOTREACHED followe by error handling / recovery code. It's agains the style guide and moreover, it can actually be hit in real life because files on disk do get corrupted sometimes. Crashy exits tend to sometimes make the next run rigger this NOTREACHED unless user data is cleaned, which is not helpful. We've hit this downstream a couple times, not very often but it was an annoying obstacle hindering some other investigation. I'd prefer if these were error logs instead.
,
Oct 13 2017
Yeah having a test hitting that code path would be ideal. I'll see if I can figure something out.
,
Oct 13 2017
tsniatowski: Did you mean to make this owned by your vewd account, rather than your opera one? Just mention it because of the red "hasn't visited this site in 15 days" thing under your vewd account.
,
Oct 13 2017
mmenke: Thanks for the heads-up, yes, I also meant to report this from the vewd one, oh well. A confusing time for me.
,
Oct 19 2017
I can trigger both "NOTREACHED" in SQLitePersistentCookieStore::Backend::InitializeDatabase during sqlite database opening. One can trigger if the sqlite db contains junk, another if it we fail opening the file (because it's been chmod-000). Will try to come up with proper testcases for these "soon". I'm a bit confused by https://bugs.chromium.org/p/chromium/issues/detail?id=727566 having disabled most of the cookie storage tests a couple months ago though.
,
Feb 6 2018
,
Apr 6 2018
We made some progress on not doing NOTREACHED all over the place in master, though a few remain (and fixes for some things going wrong in Commit are sketched), but it sounds like the database-containing-junk in particular case probably wants some sort of database recovery thing to happen, too, as to avoid losing cookies on exit every single time for someone who is in that state.
,
May 3 2018
In general: I suspect is due to the bad example set by //sql. Issue 839186 describes the problem there and what we need to do to fix it.
,
May 3 2018
,
Aug 3
This bug has an owner, thus, it's been triaged. Changing status to "assigned". |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by ricea@chromium.org
, Oct 13 2017