[sql] Remove mmap before Raze().

sql/connection.cc

Index: sql/connection.cc
diff --git a/sql/connection.cc b/sql/connection.cc
index 0511e11b5c5b22361bbdf24cc1230203749f272b..1ae09b5196824e9a92bc50ce72ed93a6f1e1cbfb 100644
--- a/sql/connection.cc
+++ b/sql/connection.cc
@@ -1112,6 +1112,12 @@ bool Connection::Raze() {
   // page_size" can be used to query such a database.
   ScopedWritableSchema writable_schema(db_);
 
+  // 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.

[pwnall, 2017/04/21 11:00:14]

Is there any reference for the Windows issue? I saw it mentioned in
https://sqlite.org/mmap.html but I couldn't find a different source. It'd be
nice to have something that allows us to track whether this is still an issue or
not on newer Windows versions.

Would it make sense to try to save the current mmap_size value and reload it
after the truncate?

Also, would it make sense to only have this workaround applied to Windows? #if
defined(OS_WINDOWS)?

[Scott Hess - ex-Googler, 2017/04/21 14:18:47]

I haven't seen anything better than what SQLite says.  But experimentally it
does seem to happen.
The primary use for Raze() is as part of "I give up" error handling.  In that
case, the next step will be to poison the database handle.  So it's hard to find
a plausible advantage to additional code.

[It may make sense to inline RazeAndClose(), but historically this developed as
"Here's a tool to help clear the database" and later "Here's a better way to
handle errors in that case".]
I can't think of any reason against an #if, since other platforms don't have
this truncate issue.  I think that having allowed the truncate to proceed,
security demands that they do the right thing with previously-mapped blocks past
the end of the truncated file, and Linux and OSX are both generally trustworthy
in this area.  Presumably failing the truncate is how Windows handles the
security issue.

Umm ... robost_ftruncate() has a not about Android only respecting 32-bit
offsets, so truncates beyond 2GB are dropped.  I don't think that applies
specifically to this (it's not mmap-specific), so ignoring.

[pwnall, 2017/04/21 19:00:18]

Oh, I was under the impression that the same connection might be reused after
the database is razed, dooming every platform to slower speeds because of
Windows' incompetence. If that's not the case, please discard my comment about
restoring mmap_size.

I would prefer that the workaround is Windows-only. No need to increase code
size and reason about more things on other platforms. If the test starts failing
(thank you for adding it!), we can revise this decision.

[Scott Hess - ex-Googler, 2017/04/23 03:36:08]

I'm not aware of Raze() cases where the connection continues to be used, but it
should work, so it's possible I'm just not aware of them.  But the mmap settings
are not persisted, so any side effects of turning it off would only apply until
next restart.
OK, I think the workaround should be Windows-only, but I'll leave the test
cross-platform.

+  ignore_result(Execute("PRAGMA mmap_size = 0"));
+
   const char* kMain = "main";
   int rc = BackupDatabase(null_db.db_, db_, kMain);
   UMA_HISTOGRAM_SPARSE_SLOWLY("Sqlite.RazeDatabase",rc);