[sql] Check for open db before releasing cache.

sql/connection.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "sql/connection.h"
 
 #include <string.h>
 
 #include "base/bind.h"
 #include "base/files/file_path.h"
@@ skipping 494 matching lines @@
 // would keep the zero page around, plus some pre-initialized pages, and SQLite
 // can manage things.  The downside is that updates larger than the cache would
 // spill to the journal.  That could be compensated by setting cache_spill to
 // false.  The downside then is that it allows open-ended use of memory for
 // large transactions.
 //
 // TODO(shess): The TrimMemory() trick of bouncing the cache size would also
 // 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());
+
   // If memory-mapping is not enabled, the page cache helps performance.
   if (!mmap_enabled_)
     return;
 
   // On caller request, force the change comparison to fail.  Done before the
   // transaction-nesting test so that the signal can carry to transaction
   // commit.
   if (implicit_change_performed)
     --total_changes_at_last_release_;
 
   // Cached pages may be re-used within the same transaction.
   if (transaction_nesting())
     return;
 

[Scott Hess - ex-Googler, 2015/09/30 17:25:04]

I didn't put:
  if (!is_open())
    return;
here because it felt like unwarranted overkill.  Looking at the callers,
Execute* explicitly checks db status, commit is probably an error to call with
closed db, and close clears the statement's connection reference.

That feels like a series of terrible justifications, but this is probably not
the time to re-architect everything.

   // If no changes have been made, skip flushing.  This allows the first page of
   // the database to remain in cache across multiple reads.
   const int total_changes = sqlite3_total_changes(db_);
   if (total_changes == total_changes_at_last_release_)
     return;
 
   total_changes_at_last_release_ = total_changes;
   sqlite3_db_release_memory(db_);
 }
 
@@ skipping 818 matching lines @@
 
   // Collect the rollback time manually, sql::Statement would register it as
   // query time only.
   const base::TimeTicks before = Now();
   rollback.RunWithoutTimers();
   const base::TimeDelta delta = Now() - before;
 
   RecordUpdateTime(delta);
   RecordOneEvent(EVENT_ROLLBACK);
 
-  // The cache may have been accumulating dirty pages for commit.
-  ReleaseCacheMemoryIfNeeded(false);
+  // The cache may have been accumulating dirty pages for commit.  Note that in
+  // some cases sql::Transaction can fire rollback after a database is closed.
+  if (is_open())
+    ReleaseCacheMemoryIfNeeded(false);
 
   needs_rollback_ = false;
 }
 
 void Connection::StatementRefCreated(StatementRef* ref) {
   DCHECK(open_statements_.find(ref) == open_statements_.end());
   open_statements_.insert(ref);
 }
 
 void Connection::StatementRefDeleted(StatementRef* ref) {
@@ skipping 100 matching lines @@
   ignore_result(Execute(kNoWritableSchema));
 
   return ret;
 }
 
 base::TimeTicks TimeSource::Now() {
   return base::TimeTicks::Now();
 }
 
 }  // namespace sql