Skip to content

Commit

Permalink
[sql] Remove mmap before Raze().
Browse files Browse the repository at this point in the history
On Windows, file truncation silently fails when the file is memory
mapped.  Modify Raze() to disable memory mapping before truncating.

BUG=none
R=pwnall@chromium.org

Review-Url: https://codereview.chromium.org/2830263002
Cr-Commit-Position: refs/heads/master@{#466555}
  • Loading branch information
dshess authored and Commit bot committed Apr 23, 2017
1 parent 8aa7a4e commit 92a6fb2
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 0 deletions.
9 changes: 9 additions & 0 deletions sql/connection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1112,6 +1112,15 @@ bool Connection::Raze() {
// page_size" can be used to query such a database.
ScopedWritableSchema writable_schema(db_);

#if defined(OS_WIN)
// On Windows, truncate silently fails when applied to memory-mapped files.
// Disable memory-mapping so that the truncate succeeds. Note that other
// connections may have memory-mapped the file, so this may not entirely
// prevent the problem.
// [Source: <https://sqlite.org/mmap.html> plus experiments.]
ignore_result(Execute("PRAGMA mmap_size = 0"));
#endif

const char* kMain = "main";
int rc = BackupDatabase(null_db.db_, db_, kMain);
UMA_HISTOGRAM_SPARSE_SLOWLY("Sqlite.RazeDatabase",rc);
Expand Down
36 changes: 36 additions & 0 deletions sql/connection_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -854,6 +854,42 @@ TEST_F(SQLConnectionTest, RazeAndCloseDiagnostics) {
// closely match real life. That would also allow testing
// RazeWithTimeout().

// On Windows, truncate silently fails against a memory-mapped file. One goal
// of Raze() is to truncate the file to remove blocks which generate I/O errors.
// Test that Raze() turns off memory mapping so that the file is truncated.
// [This would not cover the case of multiple connections where one of the other
// connections is memory-mapped. That is infrequent in Chromium.]
TEST_F(SQLConnectionTest, RazeTruncate) {
// The empty database has 0 or 1 pages. Raze() should leave it with exactly 1
// page. Not checking directly because auto_vacuum on Android adds a freelist
// page.
ASSERT_TRUE(db().Raze());
int64_t expected_size;
ASSERT_TRUE(base::GetFileSize(db_path(), &expected_size));
ASSERT_GT(expected_size, 0);

// Cause the database to take a few pages.
const char* kCreateSql = "CREATE TABLE foo (id INTEGER PRIMARY KEY, value)";
ASSERT_TRUE(db().Execute(kCreateSql));
for (size_t i = 0; i < 24; ++i) {
ASSERT_TRUE(
db().Execute("INSERT INTO foo (value) VALUES (randomblob(1024))"));
}
int64_t db_size;
ASSERT_TRUE(base::GetFileSize(db_path(), &db_size));
ASSERT_GT(db_size, expected_size);

// Make a query covering most of the database file to make sure that the
// blocks are actually mapped into memory. Empirically, the truncate problem
// doesn't seem to happen if no blocks are mapped.
EXPECT_EQ("24576",
ExecuteWithResult(&db(), "SELECT SUM(LENGTH(value)) FROM foo"));

ASSERT_TRUE(db().Raze());
ASSERT_TRUE(base::GetFileSize(db_path(), &db_size));
ASSERT_EQ(expected_size, db_size);
}

#if defined(OS_ANDROID)
TEST_F(SQLConnectionTest, SetTempDirForSQL) {

Expand Down

0 comments on commit 92a6fb2

Please sign in to comment.