Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(21)

Unified Diff: sql/recovery.cc

Issue 2710823005: [sql] RecoverDatabase() deletes SQLITE_NOTADB databases. (Closed)
Patch Set: ref Windows delete in probe-after-delete comment. Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « sql/recovery.h ('k') | sql/recovery_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: sql/recovery.cc
diff --git a/sql/recovery.cc b/sql/recovery.cc
index 5d5700262c2b71354908c9c4a0861c3e85be4e11..e8dcd31997e3d1b036d60b1f7d2216146c48dec1 100644
--- a/sql/recovery.cc
+++ b/sql/recovery.cc
@@ -108,6 +108,21 @@ enum RecoveryEventType {
// Failed to recover triggers or views or virtual tables.
RECOVERY_FAILED_AUTORECOVERDB_AUX,
+ // After SQLITE_NOTADB failure setting up for recovery, Delete() failed.
+ RECOVERY_FAILED_AUTORECOVERDB_NOTADB_DELETE,
+
+ // After SQLITE_NOTADB failure setting up for recovery, Delete() succeeded
+ // then Open() failed.
+ RECOVERY_FAILED_AUTORECOVERDB_NOTADB_REOPEN,
+
+ // After SQLITE_NOTADB failure setting up for recovery, Delete() and Open()
+ // succeeded, then querying the database failed.
+ RECOVERY_FAILED_AUTORECOVERDB_NOTADB_QUERY,
+
+ // After SQLITE_NOTADB failure setting up for recovery, the database was
+ // successfully deleted.
+ RECOVERY_SUCCESS_AUTORECOVERDB_NOTADB_DELETE,
+
// Always keep this at the end.
RECOVERY_EVENT_MAX,
};
@@ -588,9 +603,50 @@ void Recovery::RecoverDatabase(Connection* db,
const base::FilePath& db_path) {
std::unique_ptr<sql::Recovery> recovery = sql::Recovery::Begin(db, db_path);
if (!recovery) {
- // TODO(shess): If recovery can't even get started, Raze() or Delete().
- RecordRecoveryEvent(RECOVERY_FAILED_AUTORECOVERDB_BEGIN);
+ // Close the underlying sqlite* handle. Windows does not allow deleting
+ // open files, and all platforms block opening a second sqlite3* handle
+ // against a database when exclusive locking is set.
db->Poison();
+
+ // Histograms from Recovery::Begin() show all current failures are in
+ // attaching the corrupt database, with 2/3 being SQLITE_NOTADB. Don't
+ // delete the database except for that specific failure case.
+ {
+ Connection probe_db;
+ if (!probe_db.OpenInMemory() ||
+ probe_db.AttachDatabase(db_path, "corrupt") ||
+ probe_db.GetErrorCode() != SQLITE_NOTADB) {
+ RecordRecoveryEvent(RECOVERY_FAILED_AUTORECOVERDB_BEGIN);
+ return;
+ }
+ }
+
+ // The database has invalid data in the SQLite header, so it is almost
+ // certainly not recoverable without manual intervention (and likely not
+ // recoverable _with_ manual intervention). Clear away the broken database.
+ if (!sql::Connection::Delete(db_path)) {
+ RecordRecoveryEvent(RECOVERY_FAILED_AUTORECOVERDB_NOTADB_DELETE);
+ return;
+ }
+
+ // Windows deletion is complicated by file scanners and malware - sometimes
+ // Delete() appears to succeed, even though the file remains. The following
+ // attempts to track if this happens often enough to cause concern.
+ {
+ Connection probe_db;
+ if (!probe_db.Open(db_path)) {
+ RecordRecoveryEvent(RECOVERY_FAILED_AUTORECOVERDB_NOTADB_REOPEN);
+ return;
+ }
+ if (!probe_db.Execute("PRAGMA auto_vacuum")) {
+ RecordRecoveryEvent(RECOVERY_FAILED_AUTORECOVERDB_NOTADB_QUERY);
+ return;
+ }
+ }
+
+ // The rest of the recovery code could be run on the re-opened database, but
+ // the database is empty, so there would be no point.
+ RecordRecoveryEvent(RECOVERY_SUCCESS_AUTORECOVERDB_NOTADB_DELETE);
return;
}
« no previous file with comments | « sql/recovery.h ('k') | sql/recovery_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698