summaryrefslogtreecommitdiffstats
path: root/sql
diff options
context:
space:
mode:
authorshess@chromium.org <shess@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-07-17 04:45:13 +0000
committershess@chromium.org <shess@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-07-17 04:45:13 +0000
commitfed734a145c6defbf1063b3b9a497aee88db720d (patch)
tree3f8fe3a0e49f57a42c1bb514e4f7c145c6c5195c /sql
parent5b7355bcdf73ef995001d0cf8fbf2385012a7a52 (diff)
downloadchromium_src-fed734a145c6defbf1063b3b9a497aee88db720d.zip
chromium_src-fed734a145c6defbf1063b3b9a497aee88db720d.tar.gz
chromium_src-fed734a145c6defbf1063b3b9a497aee88db720d.tar.bz2
[sql] Retry Open() if error handler fixed things.
Since the caller cannot have existing database-derived state before the database is opened, Open() can be safely retried after the error handler has razed the database. BUG=109482 Review URL: https://chromiumcodereview.appspot.com/18641004 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@211936 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'sql')
-rw-r--r--sql/connection.cc19
-rw-r--r--sql/connection.h9
-rw-r--r--sql/connection_unittest.cc31
3 files changed, 41 insertions, 18 deletions
diff --git a/sql/connection.cc b/sql/connection.cc
index 5f76a7f..95d09c2 100644
--- a/sql/connection.cc
+++ b/sql/connection.cc
@@ -194,15 +194,15 @@ bool Connection::Open(const base::FilePath& path) {
}
#if defined(OS_WIN)
- return OpenInternal(WideToUTF8(path.value()));
+ return OpenInternal(WideToUTF8(path.value()), RETRY_ON_POISON);
#elif defined(OS_POSIX)
- return OpenInternal(path.value());
+ return OpenInternal(path.value(), RETRY_ON_POISON);
#endif
}
bool Connection::OpenInMemory() {
in_memory_ = true;
- return OpenInternal(":memory:");
+ return OpenInternal(":memory:", NO_RETRY);
}
void Connection::CloseInternal(bool forced) {
@@ -238,8 +238,8 @@ void Connection::CloseInternal(bool forced) {
AssertIOAllowed();
// TODO(shess): Histogram for failure.
sqlite3_close(db_);
- db_ = NULL;
}
+ db_ = NULL;
}
void Connection::Close() {
@@ -699,7 +699,8 @@ const char* Connection::GetErrorMessage() const {
return sqlite3_errmsg(db_);
}
-bool Connection::OpenInternal(const std::string& file_name) {
+bool Connection::OpenInternal(const std::string& file_name,
+ Connection::Retry retry_flag) {
AssertIOAllowed();
if (db_) {
@@ -723,8 +724,11 @@ bool Connection::OpenInternal(const std::string& file_name) {
UMA_HISTOGRAM_ENUMERATION("Sqlite.OpenFailure", err & 0xff, 50);
OnSqliteError(err, NULL);
+ bool was_poisoned = poisoned_;
Close();
- db_ = NULL;
+
+ if (was_poisoned && retry_flag == RETRY_ON_POISON)
+ return OpenInternal(file_name, NO_RETRY);
return false;
}
@@ -806,7 +810,10 @@ bool Connection::OpenInternal(const std::string& file_name) {
}
if (!ExecuteWithTimeout("PRAGMA secure_delete=ON", kBusyTimeout)) {
+ bool was_poisoned = poisoned_;
Close();
+ if (was_poisoned && retry_flag == RETRY_ON_POISON)
+ return OpenInternal(file_name, NO_RETRY);
return false;
}
diff --git a/sql/connection.h b/sql/connection.h
index 8155a8f..1c4d72c 100644
--- a/sql/connection.h
+++ b/sql/connection.h
@@ -360,7 +360,14 @@ class SQL_EXPORT Connection {
// Internal initialize function used by both Init and InitInMemory. The file
// name is always 8 bits since we want to use the 8-bit version of
// sqlite3_open. The string can also be sqlite's special ":memory:" string.
- bool OpenInternal(const std::string& file_name);
+ //
+ // |retry_flag| controls retrying the open if the error callback
+ // addressed errors using RazeAndClose().
+ enum Retry {
+ NO_RETRY = 0,
+ RETRY_ON_POISON
+ };
+ bool OpenInternal(const std::string& file_name, Retry retry_flag);
// Internal close function used by Close() and RazeAndClose().
// |forced| indicates that orderly-shutdown checks should not apply.
diff --git a/sql/connection_unittest.cc b/sql/connection_unittest.cc
index fc430ac..8cf32fb 100644
--- a/sql/connection_unittest.cc
+++ b/sql/connection_unittest.cc
@@ -491,7 +491,8 @@ TEST_F(SQLConnectionTest, RazeNOTADB2) {
// Test that a callback from Open() can raze the database. This is
// essential for cases where the Open() can fail entirely, so the
-// Raze() cannot happen later.
+// Raze() cannot happen later. Additionally test that when the
+// callback does this during Open(), the open is retried and succeeds.
//
// Most corruptions seen in the wild seem to happen when two pages in
// the database were not written transactionally (the transaction
@@ -499,7 +500,7 @@ TEST_F(SQLConnectionTest, RazeNOTADB2) {
// A special case of that is when the header indicates that the
// database contains more pages than are in the file. This breaks
// things at a very basic level, verify that Raze() can handle it.
-TEST_F(SQLConnectionTest, RazeOpenCallback) {
+TEST_F(SQLConnectionTest, RazeCallbackReopen) {
const char* kCreateSql = "CREATE TABLE foo (id INTEGER PRIMARY KEY, value)";
ASSERT_TRUE(db().Execute(kCreateSql));
ASSERT_EQ(1, SqliteMasterCount(&db()));
@@ -519,19 +520,27 @@ TEST_F(SQLConnectionTest, RazeOpenCallback) {
ASSERT_TRUE(file_util::TruncateFile(file.get()));
}
+ // Open() will succeed, even though the PRAGMA calls within will
+ // fail with SQLITE_CORRUPT, as will this PRAGMA.
+ {
+ sql::ScopedErrorIgnorer ignore_errors;
+ ignore_errors.IgnoreError(SQLITE_CORRUPT);
+ ASSERT_TRUE(db().Open(db_path()));
+ ASSERT_FALSE(db().Execute("PRAGMA auto_vacuum"));
+ db().Close();
+ ASSERT_TRUE(ignore_errors.CheckIgnoredErrors());
+ }
+
db().set_error_callback(base::Bind(&SQLConnectionTest::RazeErrorCallback,
base::Unretained(this),
SQLITE_CORRUPT));
- // Open() will see SQLITE_CORRUPT due to size mismatch when
- // attempting to run a pragma, and the callback will RazeAndClose().
- // Later statements will fail, including the final secure_delete,
- // which will fail the Open() itself.
- ASSERT_FALSE(db().Open(db_path()));
- db().Close();
-
- // Now empty, the open should succeed with an empty database.
- EXPECT_TRUE(db().Open(db_path()));
+ // When the PRAGMA calls in Open() raise SQLITE_CORRUPT, the error
+ // callback will call RazeAndClose(). Open() will then fail and be
+ // retried. The second Open() on the empty database will succeed
+ // cleanly.
+ ASSERT_TRUE(db().Open(db_path()));
+ ASSERT_TRUE(db().Execute("PRAGMA auto_vacuum"));
EXPECT_EQ(0, SqliteMasterCount(&db()));
}