[sql] Retry post-poison open as soon as possible.

sql/connection.cc

Index: sql/connection.cc
diff --git a/sql/connection.cc b/sql/connection.cc
index 9c7cb7d4099a811bc78b314070ab2fd1eebb1eb4..0370faa806e8e09dbaa5099bd9d7609ca1b95d30 100644
--- a/sql/connection.cc
+++ b/sql/connection.cc
@@ -1693,15 +1693,28 @@ bool Connection::OpenInternal(const std::string& file_name,
   err = sqlite3_extended_result_codes(db_, 1);
   DCHECK_EQ(err, SQLITE_OK) << "Could not enable extended result codes";
 
-  // sqlite3_open() does not actually read the database file (unless a
-  // hot journal is found).  Successfully executing this pragma on an
-  // existing database requires a valid header on page 1.
+  // sqlite3_open() does not actually read the database file (unless a hot
+  // journal is found).  Successfully executing this pragma on an existing
+  // database requires a valid header on page 1.  ExecuteAndReturnErrorCode() to
+  // get the error code before error callback (potentially) overwrites.
   // TODO(shess): For now, just probing to see what the lay of the
   // land is.  If it's mostly SQLITE_NOTADB, then the database should
   // be razed.
   err = ExecuteAndReturnErrorCode("PRAGMA auto_vacuum");

[afakhry, 2016/08/18 21:32:29]

Shouldn't we use Execute() instead so that the error callback is invoked, in
order to give a chance to clients to Raze() the DB on a catastrophic error?

[Scott Hess - ex-Googler, 2016/08/18 22:06:53]

Sqlite.OpenProbeFailure tracks the specific SQLite error.  Execute() followed by
GetErrorCode() could work, but if the error callback does RazeAndClose() or
calls recovery code, GetErrorCode() will return the last error from _that_ (kind
of like how errno needs to be pinned in error-handling code if you want to log
it).  OnSqliteError() will call into the regular error-handling, including
callback.

[afakhry, 2016/08/19 15:25:09]

Oh yes, sorry, I didn't notice the OnSqliteError() even though I wrote something
similar in one of the patches in my other CL. :D

-  if (err != SQLITE_OK)
+  if (err != SQLITE_OK) {
     UMA_HISTOGRAM_SPARSE_SLOWLY("Sqlite.OpenProbeFailure", err);
+    OnSqliteError(err, NULL, "PRAGMA auto_vacuum");

[afakhry, 2016/08/19 15:25:09]

Nit: NULL --> nullptr.

[Scott Hess - ex-Googler, 2016/08/19 21:51:03]

Done.

+
+    // Retry or bail out if the error handler poisoned the handle.
+    // TODO(shess): Move this handling to one place (see also sqlite3_open and
+    // secure_delete).  Possibly a wrapper function?
+    if (poisoned_) {
+      Close();
+      if (retry_flag == RETRY_ON_POISON)
+        return OpenInternal(file_name, NO_RETRY);
+      return false;
+    }
+  }
 
 #if defined(OS_IOS) && defined(USE_SYSTEM_SQLITE)
   // The version of SQLite shipped with iOS doesn't enable ICU, which includes