[sql] Fix crash in sqlite3_total_changes.

The mmap support makes direct SQLite calls against the database handle
in ReleaseCacheMemoryIfNeeded() (called after transactions are closed or
statements are executed outside of any explicit transaction).  Recovery
or raze during the error callback can close this handle during
otherwise-valid sequences of calls.  Check that the handle is still open
before clearing the cache.

BUG=589986

Review URL: https://codereview.chromium.org/1741553002

Cr-Commit-Position: refs/heads/master@{#377924}

2 files changed, 22 insertions, 2 deletions

diff --git a/sql/connection.cc b/sql/connection.cc
index f94e7ab..7787adf 100644
--- a/sql/connection.cc
+++ b/sql/connection.cc
@@ -595,7 +595,12 @@ void Connection::Preload() {
 // work.  There could be two prepared statements, one for cache_size=1 one for
 // cache_size=goal.
 void Connection::ReleaseCacheMemoryIfNeeded(bool implicit_change_performed) {
-  DCHECK(is_open());
+  // The database could have been closed during a transaction as part of error
+  // recovery.
+  if (!db_) {
+    DLOG_IF(FATAL, !poisoned_) << "Illegal use of connection without a db";
+    return;
+  }
 
   // If memory-mapping is not enabled, the page cache helps performance.
   if (!mmap_enabled_)
diff --git a/sql/connection_unittest.cc b/sql/connection_unittest.cc
index c6f80d8..d9dd9d9 100644
--- a/sql/connection_unittest.cc
+++ b/sql/connection_unittest.cc
@@ -247,7 +247,9 @@ class SQLConnectionTest : public sql::SQLTestBase {
 
   // Handle errors by blowing away the database.
   void RazeErrorCallback(int expected_error, int error, sql::Statement* stmt) {
-    EXPECT_EQ(expected_error, error);
+    // Nothing here needs extended errors at this time.
+    EXPECT_EQ(expected_error, expected_error&0xff);
+    EXPECT_EQ(expected_error, error&0xff);
     db().RazeAndClose();
   }
 };
@@ -928,6 +930,19 @@ TEST_F(SQLConnectionTest, Poison) {
   // The existing statement has become invalid.
   ASSERT_FALSE(valid_statement.is_valid());
   ASSERT_FALSE(valid_statement.Step());
+
+  // Test that poisoning the database during a transaction works (with errors).
+  // RazeErrorCallback() poisons the database, the extra COMMIT causes
+  // CommitTransaction() to throw an error while commiting.
+  db().set_error_callback(base::Bind(&SQLConnectionTest::RazeErrorCallback,
+                                     base::Unretained(this),
+                                     SQLITE_ERROR));
+  db().Close();
+  ASSERT_TRUE(db().Open(db_path()));
+  EXPECT_TRUE(db().BeginTransaction());
+  EXPECT_TRUE(db().Execute("INSERT INTO x VALUES ('x')"));
+  EXPECT_TRUE(db().Execute("COMMIT"));
+  EXPECT_FALSE(db().CommitTransaction());
 }
 
 // Test attaching and detaching databases from the connection.