Skip to content

Commit

Permalink
Make sql::Connection::Raze() more robust against corruptions.
Browse files Browse the repository at this point in the history
Corruptions resulting from the first page showing a different database
size than the filesystem shows cause very basic functions to fail.
This does fewer queries against the corrupt database, and also enables
a magic pragma to let it make better progress.

BUG=159490


Review URL: https://chromiumcodereview.appspot.com/11369126

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@167019 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
shess@chromium.org committed Nov 10, 2012
1 parent 1c3988f commit 6d42f15
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 21 deletions.
69 changes: 49 additions & 20 deletions sql/connection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,26 @@ class ScopedBusyTimeout {
sqlite3* db_;
};

// Helper to "safely" enable writable_schema. No error checking
// because it is reasonable to just forge ahead in case of an error.
// If turning it on fails, then most likely nothing will work, whereas
// if turning it off fails, it only matters if some code attempts to
// continue working with the database and tries to modify the
// sqlite_master table (none of our code does this).
class ScopedWritableSchema {
public:
explicit ScopedWritableSchema(sqlite3* db)
: db_(db) {
sqlite3_exec(db_, "PRAGMA writable_schema=1", NULL, NULL, NULL);
}
~ScopedWritableSchema() {
sqlite3_exec(db_, "PRAGMA writable_schema=0", NULL, NULL, NULL);
}

private:
sqlite3* db_;
};

} // namespace

namespace sql {
Expand Down Expand Up @@ -196,29 +216,27 @@ bool Connection::Raze() {
return false;
}

// Get the page size from the current connection, then propagate it
// to the null database.
{
Statement s(GetUniqueStatement("PRAGMA page_size"));
if (!s.Step())
return false;
const std::string sql = StringPrintf("PRAGMA page_size=%d",
s.ColumnInt(0));
if (page_size_) {
// Enforce SQLite restrictions on |page_size_|.
DCHECK(!(page_size_ & (page_size_ - 1)))
<< " page_size_ " << page_size_ << " is not a power of two.";
const int kSqliteMaxPageSize = 32768; // from sqliteLimit.h
DCHECK_LE(page_size_, kSqliteMaxPageSize);
const std::string sql = StringPrintf("PRAGMA page_size=%d", page_size_);
if (!null_db.Execute(sql.c_str()))
return false;
}

// Get the value of auto_vacuum from the current connection, then propagate it
// to the null database.
{
Statement s(GetUniqueStatement("PRAGMA auto_vacuum"));
if (!s.Step())
return false;
const std::string sql = StringPrintf("PRAGMA auto_vacuum=%d",
s.ColumnInt(0));
if (!null_db.Execute(sql.c_str()))
return false;
}
#if defined(OS_ANDROID)
// Android compiles with SQLITE_DEFAULT_AUTOVACUUM. Unfortunately,
// in-memory databases do not respect this define.
// TODO(shess): Figure out a way to set this without using platform
// specific code. AFAICT from sqlite3.c, the only way to do it
// would be to create an actual filesystem database, which is
// unfortunate.
if (!null_db.Execute("PRAGMA auto_vacuum = 1"))
return false;
#endif

// The page size doesn't take effect until a database has pages, and
// at this point the null database has none. Changing the schema
Expand All @@ -230,6 +248,17 @@ bool Connection::Raze() {
if (!null_db.Execute("PRAGMA schema_version = 1"))
return false;

// SQLite tracks the expected number of database pages in the first
// page, and if it does not match the total retrieved from a
// filesystem call, treats the database as corrupt. This situation
// breaks almost all SQLite calls. "PRAGMA writable_schema" can be
// used to hint to SQLite to soldier on in that case, specifically
// for purposes of recovery. [See SQLITE_CORRUPT_BKPT case in
// sqlite3.c lockBtree().]
// TODO(shess): With this, "PRAGMA auto_vacuum" and "PRAGMA
// page_size" can be used to query such a database.
ScopedWritableSchema writable_schema(db_);

sqlite3_backup* backup = sqlite3_backup_init(db_, "main",
null_db.db_, "main");
if (!backup) {
Expand Down Expand Up @@ -549,7 +578,7 @@ bool Connection::OpenInternal(const std::string& file_name) {
// Enforce SQLite restrictions on |page_size_|.
DCHECK(!(page_size_ & (page_size_ - 1)))
<< " page_size_ " << page_size_ << " is not a power of two.";
static const int kSqliteMaxPageSize = 32768; // from sqliteLimit.h
const int kSqliteMaxPageSize = 32768; // from sqliteLimit.h
DCHECK_LE(page_size_, kSqliteMaxPageSize);
const std::string sql = StringPrintf("PRAGMA page_size=%d", page_size_);
if (!ExecuteWithTimeout(sql.c_str(), kBusyTimeout))
Expand Down
13 changes: 13 additions & 0 deletions sql/connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,19 @@ class SQL_EXPORT Connection {
// Since Raze() is expected to be called in unexpected situations,
// these all return false, since it is unlikely that the caller
// could fix them.
//
// The database's page size is taken from |page_size_|. The
// existing database's |auto_vacuum| setting is lost (the
// possibility of corruption makes it unreliable to pull it from the
// existing database). To re-enable on the empty database requires
// running "PRAGMA auto_vacuum = 1;" then "VACUUM".
//
// NOTE(shess): For Android, SQLITE_DEFAULT_AUTOVACUUM is set to 1,
// so Raze() sets auto_vacuum to 1.
//
// TODO(shess): Raze() needs a connection so cannot clear SQLITE_NOTADB.
// TODO(shess): Bake auto_vacuum into Connection's API so it can
// just pick up the default.
bool Raze();
bool RazeWithTimout(base::TimeDelta timeout);

Expand Down
2 changes: 1 addition & 1 deletion sql/connection_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ TEST_F(SQLConnectionTest, Raze) {
{
sql::Statement s(db().GetUniqueStatement("PRAGMA auto_vacuum"));
ASSERT_TRUE(s.Step());
// auto_vacuum must be preserved across a Raze.
// The new database has the same auto_vacuum as a fresh database.
EXPECT_EQ(pragma_auto_vacuum, s.ColumnInt(0));
}
}
Expand Down

0 comments on commit 6d42f15

Please sign in to comment.