[sql] Disable memory-mapping under sql::Recovery.
Recovery may be used to recover files with I/O errors, which would not interact well with memory-mapped I/O. [AFAICT, SQLite doesn't use memory-mapped I/O on temporary files, but as of yet I don't see why this might be.] BUG=551110 Review URL: https://codereview.chromium.org/1414563010 Cr-Commit-Position: refs/heads/master@{#358083}
This commit is contained in:
@ -1628,18 +1628,6 @@ bool Connection::OpenInternal(const std::string& file_name,
|
||||
// secure_delete.
|
||||
ignore_result(Execute("PRAGMA journal_mode = TRUNCATE"));
|
||||
|
||||
// Enable memory-mapped access. This value will be capped by
|
||||
// SQLITE_MAX_MMAP_SIZE, which could be different between 32-bit and 64-bit
|
||||
// platforms.
|
||||
mmap_enabled_ = false;
|
||||
if (!mmap_disabled_)
|
||||
ignore_result(Execute("PRAGMA mmap_size = 268435456")); // 256MB.
|
||||
{
|
||||
Statement s(GetUniqueStatement("PRAGMA mmap_size"));
|
||||
if (s.Step() && s.ColumnInt64(0) > 0)
|
||||
mmap_enabled_ = true;
|
||||
}
|
||||
|
||||
const base::TimeDelta kBusyTimeout =
|
||||
base::TimeDelta::FromSeconds(kBusyTimeoutSeconds);
|
||||
|
||||
@ -1668,6 +1656,25 @@ bool Connection::OpenInternal(const std::string& file_name,
|
||||
return false;
|
||||
}
|
||||
|
||||
// Enable memory-mapped access. The explicit-disable case is because SQLite
|
||||
// can be built to default-enable mmap. This value will be capped by
|
||||
// SQLITE_MAX_MMAP_SIZE, which could be different between 32-bit and 64-bit
|
||||
// platforms.
|
||||
if (mmap_disabled_) {
|
||||
ignore_result(Execute("PRAGMA mmap_size = 0"));
|
||||
} else {
|
||||
ignore_result(Execute("PRAGMA mmap_size = 268435456")); // 256MB.
|
||||
}
|
||||
|
||||
// Determine if memory-mapping has actually been enabled. The Execute() above
|
||||
// can succeed without changing the amount mapped.
|
||||
mmap_enabled_ = false;
|
||||
{
|
||||
Statement s(GetUniqueStatement("PRAGMA mmap_size"));
|
||||
if (s.Step() && s.ColumnInt64(0) > 0)
|
||||
mmap_enabled_ = true;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
|
@ -141,6 +141,9 @@ Recovery::Recovery(Connection* connection)
|
||||
if (db_->page_size_)
|
||||
recover_db_.set_page_size(db_->page_size_);
|
||||
|
||||
// Files with I/O errors cannot be safely memory-mapped.
|
||||
recover_db_.set_mmap_disabled();
|
||||
|
||||
// TODO(shess): This may not handle cases where the default page
|
||||
// size is used, but the default has changed. I do not think this
|
||||
// has ever happened. This could be handled by using "PRAGMA
|
||||
|
@ -692,4 +692,16 @@ TEST_F(SQLRecoveryTest, Bug387868) {
|
||||
}
|
||||
#endif // !defined(USE_SYSTEM_SQLITE)
|
||||
|
||||
// Memory-mapped I/O interacts poorly with I/O errors. Make sure the recovery
|
||||
// database doesn't accidentally enable it.
|
||||
TEST_F(SQLRecoveryTest, NoMmap) {
|
||||
scoped_ptr<sql::Recovery> recovery = sql::Recovery::Begin(&db(), db_path());
|
||||
ASSERT_TRUE(recovery.get());
|
||||
|
||||
// In the current implementation, the PRAGMA successfully runs with no result
|
||||
// rows. Running with a single result of |0| is also acceptable.
|
||||
sql::Statement s(recovery->db()->GetUniqueStatement("PRAGMA mmap_size"));
|
||||
EXPECT_TRUE(!s.Step() || !s.ColumnInt64(0));
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
Reference in New Issue
Block a user