From 74cdedef60df3df4a2045e5ce542f142c0fd1e4b Mon Sep 17 00:00:00 2001 From: "shess@chromium.org" Date: Wed, 25 Sep 2013 05:39:57 +0000 Subject: Recover corrupt Favicon databases. Add sql::Recovery support to ThumbnailDatabase. Rearrange the schema setup to allow sharing with the recovery code. Unit test a few corruption cases. Add sql::Recovery::Rollback() to allow transitioning code to monitor failures for diagnostic purposes. It is possible/likely that that function can be removed if failure cases are infrequent enough (in which case it with Unrecoverable()). Expose sql::Connection::ShouldIgnore() to allow the error callback to support sql::ScopedErrorIgnorer for testing. BUG=240396 Review URL: https://chromiumcodereview.appspot.com/23533038 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@225144 0039d316-1c4b-4281-b951-d872f2087c98 --- sql/connection.cc | 4 ++-- sql/connection.h | 7 ++++++- sql/recovery.cc | 19 +++++++++++++++++++ sql/recovery.h | 10 +++++++++- 4 files changed, 36 insertions(+), 4 deletions(-) (limited to 'sql') diff --git a/sql/connection.cc b/sql/connection.cc index 097edd7..0b49786 100644 --- a/sql/connection.cc +++ b/sql/connection.cc @@ -133,7 +133,7 @@ namespace sql { Connection::ErrorIgnorerCallback* Connection::current_ignorer_cb_ = NULL; // static -bool Connection::ShouldIgnore(int error) { +bool Connection::ShouldIgnoreSqliteError(int error) { if (!current_ignorer_cb_) return false; return current_ignorer_cb_->Run(error); @@ -1040,7 +1040,7 @@ int Connection::OnSqliteError(int err, sql::Statement *stmt) { } // The default handling is to assert on debug and to ignore on release. - if (!ShouldIgnore(err)) + if (!ShouldIgnoreSqliteError(err)) DLOG(FATAL) << GetErrorMessage(); return err; } diff --git a/sql/connection.h b/sql/connection.h index 7938606..5475a84 100644 --- a/sql/connection.h +++ b/sql/connection.h @@ -394,6 +394,12 @@ class SQL_EXPORT Connection { // SELECT type, name, tbl_name, sql FROM sqlite_master ORDER BY 1, 2, 3, 4; std::string GetSchema() const; + // Clients which provide an error_callback don't see the + // error-handling at the end of OnSqliteError(). Expose to allow + // those clients to work appropriately with ScopedErrorIgnorer in + // tests. + static bool ShouldIgnoreSqliteError(int error); + private: // For recovery module. friend class Recovery; @@ -436,7 +442,6 @@ class SQL_EXPORT Connection { // See test/scoped_error_ignorer.h. typedef base::Callback ErrorIgnorerCallback; static ErrorIgnorerCallback* current_ignorer_cb_; - static bool ShouldIgnore(int error); static void SetErrorIgnorer(ErrorIgnorerCallback* ignorer); static void ResetErrorIgnorer(); diff --git a/sql/recovery.cc b/sql/recovery.cc index 9016f8a..fc67661 100644 --- a/sql/recovery.cc +++ b/sql/recovery.cc @@ -37,6 +37,13 @@ void Recovery::Unrecoverable(scoped_ptr r) { // ~Recovery() will RAZE_AND_POISON. } +// static +void Recovery::Rollback(scoped_ptr r) { + // TODO(shess): HISTOGRAM to track? Or just have people crash out? + // Crash and dump? + r->Shutdown(POISON); +} + Recovery::Recovery(Connection* connection) : db_(connection), recover_db_() { @@ -69,6 +76,18 @@ bool Recovery::Init(const base::FilePath& db_path) { // more complicated. db_->RollbackAllTransactions(); + // Disable exclusive locking mode so that the attached database can + // access things. The locking_mode change is not active until the + // next database access, so immediately force an access. Enabling + // writable_schema allows processing through certain kinds of + // corruption. + // TODO(shess): It would be better to just close the handle, but it + // is necessary for the final backup which rewrites things. It + // might be reasonable to close then re-open the handle. + ignore_result(db_->Execute("PRAGMA writable_schema=1")); + ignore_result(db_->Execute("PRAGMA locking_mode=NORMAL")); + ignore_result(db_->Execute("SELECT COUNT(*) FROM sqlite_master")); + if (!recover_db_.OpenTemporary()) return false; diff --git a/sql/recovery.h b/sql/recovery.h index c0bb6da..e832da6 100644 --- a/sql/recovery.h +++ b/sql/recovery.h @@ -64,7 +64,7 @@ class SQL_EXPORT Recovery { // If Recovered() is not called, the destructor will call // Unrecoverable(). // - // TODO(shess): At this time, this function an fail while leaving + // TODO(shess): At this time, this function can fail while leaving // the original database intact. Figure out which failure cases // should go to RazeAndClose() instead. static bool Recovered(scoped_ptr r) WARN_UNUSED_RESULT; @@ -73,6 +73,14 @@ class SQL_EXPORT Recovery { // database is razed, and the handle poisoned. static void Unrecoverable(scoped_ptr r); + // When initially developing recovery code, sometimes the possible + // database states are not well-understood without further + // diagnostics. Abandon recovery but do not raze the original + // database. + // NOTE(shess): Only call this when adding recovery support. In the + // steady state, all databases should progress to recovered or razed. + static void Rollback(scoped_ptr r); + // Handle to the temporary recovery database. sql::Connection* db() { return &recover_db_; } -- cgit v1.1