diff options
author | shess <shess@chromium.org> | 2015-11-05 10:11:10 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-11-05 18:11:47 +0000 |
commit | 2f3a814be066d8c02ba8b82f003cd98799165882 (patch) | |
tree | f8a16012c8d406d9159161a0b1c61fcee2db4c50 /sql | |
parent | ea5fce4bbcba898f4c88eb5e5021ef5df4870120 (diff) | |
download | chromium_src-2f3a814be066d8c02ba8b82f003cd98799165882.zip chromium_src-2f3a814be066d8c02ba8b82f003cd98799165882.tar.gz chromium_src-2f3a814be066d8c02ba8b82f003cd98799165882.tar.bz2 |
[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}
Diffstat (limited to 'sql')
-rw-r--r-- | sql/connection.cc | 31 | ||||
-rw-r--r-- | sql/recovery.cc | 3 | ||||
-rw-r--r-- | sql/recovery_unittest.cc | 12 |
3 files changed, 34 insertions, 12 deletions
diff --git a/sql/connection.cc b/sql/connection.cc index a904632..79264916 100644 --- a/sql/connection.cc +++ b/sql/connection.cc @@ -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; } diff --git a/sql/recovery.cc b/sql/recovery.cc index 4308129..6691d07 100644 --- a/sql/recovery.cc +++ b/sql/recovery.cc @@ -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 diff --git a/sql/recovery_unittest.cc b/sql/recovery_unittest.cc index 1f930cb..bbddd97 100644 --- a/sql/recovery_unittest.cc +++ b/sql/recovery_unittest.cc @@ -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 |