New issue
Advanced search Search tips

Issue 712399 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 586194
issue 732863



Sign in to add a comment

Better deal with database errors reading/writing to localstorage leveldb backend

Project Member Reported by mek@chromium.org, Apr 17 2017

Issue description

Several places in the new mojo/leveldb localstorage backend don't currently have any way to recover from errors:

- Errors loading data in LocalStorageContextMojo::LevelDBWrapperHolder::OnMapLoaded currently just result in the error being UMA logged, but nothing else is done.

- Errors on committing data in LocalStorageContextMojo::LevelDBWrapperHolder::DidCommit similarly currently only result in UMA logging, but no attempt to recover.

- And finally invalid meta-data in LocalStorageContextMojo::OnGotMetaData (or any other error reading this data from the database) is simply ignored completely.

We'll have to decide what to do in these cases, in particular since even the user clearing storage might not actually delete the leveldb database itself (it'll just try to empty it), thereby potentially not helping you recover from corruption.

If corruption is detected when initially connecting to leveldb the code already potentially throws away the entire database and tries to recreate it, so doing something similar on any of these other errors might be sensible (just a bit more complicated).
 

Comment 1 by mek@chromium.org, Jun 13 2017

Blocking: 732863

Comment 2 by mek@chromium.org, Jun 13 2017

Labels: -Pri-3 M-61 Pri-1
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b8c4af16dd831b76ee2c9f0b711a1d5947e772cb

commit b8c4af16dd831b76ee2c9f0b711a1d5947e772cb
Author: Marijn Kruisselbrink <mek@chromium.org>
Date: Thu Jul 13 00:23:34 2017

Try to recover from persistent localstorage commit errors by deleting the db.

Certain errors can put leveldb in a mode where all future writes will
automatically fail. If we detect such a situation (because too many consecutive
commits failed) clear out the entire database and start fresh.

Existing renderers that were connected to a LevelDBWrapper already will be
disconnected, resulting in localstorage in those renderers (for origins that
were opened) to now be disconnected from eachother and from potential future
renderers/origins. Connections to localstorage from new renderers or for
new origins in existing renderers will end up going through the database,
and should work normally.

Bug:  712399 
Change-Id: Ie38020a33ce383fd43513d1cca22afbe7180fecf
Reviewed-on: https://chromium-review.googlesource.com/533720
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Michael Nordman <michaeln@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486182}
[modify] https://crrev.com/b8c4af16dd831b76ee2c9f0b711a1d5947e772cb/content/browser/dom_storage/local_storage_context_mojo.cc
[modify] https://crrev.com/b8c4af16dd831b76ee2c9f0b711a1d5947e772cb/content/browser/dom_storage/local_storage_context_mojo.h
[modify] https://crrev.com/b8c4af16dd831b76ee2c9f0b711a1d5947e772cb/content/browser/dom_storage/local_storage_context_mojo_unittest.cc
[modify] https://crrev.com/b8c4af16dd831b76ee2c9f0b711a1d5947e772cb/content/renderer/dom_storage/local_storage_cached_area_unittest.cc
[modify] https://crrev.com/b8c4af16dd831b76ee2c9f0b711a1d5947e772cb/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/b8c4af16dd831b76ee2c9f0b711a1d5947e772cb/tools/metrics/histograms/histograms.xml

Comment 4 by mek@chromium.org, Jul 24 2017

Status: Fixed (was: Available)

Sign in to add a comment