Bug 1285041 - ignore locking when trying to read chrome DB file, r=mak draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Mon, 18 Jul 2016 16:46:45 +0100
changeset 414404 53efa0f0258728df54fc17a773cbf74c712b51e4
parent 414157 2840a2d57a59c054bfa9694e7c8c9bfddc71dc28
child 531433 f5d1cb76b0f8211ae422a69d45471294419f7e27
push id29659
push usergijskruitbosch@gmail.com
push dateFri, 16 Sep 2016 08:54:47 +0000
reviewersmak
bugs1285041
milestone51.0a1
Bug 1285041 - ignore locking when trying to read chrome DB file, r=mak MozReview-Commit-ID: 89f0YCxxgC8
browser/components/migration/ChromeProfileMigrator.js
storage/mozStorageConnection.cpp
storage/mozStorageConnection.h
storage/mozStorageService.cpp
storage/test/unit/test_storage_connection.js
toolkit/modules/Sqlite.jsm
--- a/browser/components/migration/ChromeProfileMigrator.js
+++ b/browser/components/migration/ChromeProfileMigrator.js
@@ -306,29 +306,69 @@ function GetBookmarksResource(aProfileFo
 }
 
 function GetHistoryResource(aProfileFolder) {
   let historyFile = aProfileFolder.clone();
   historyFile.append("History");
   if (!historyFile.exists())
     return null;
 
+  function getRows(dbOptions) {
+    const RETRYLIMIT = 10;
+    const RETRYINTERVAL = 100;
+    return Task.spawn(function* innerGetRows() {
+      let rows = null;
+      for (let retryCount = RETRYLIMIT; retryCount && !rows; retryCount--) {
+        // Attempt to get the rows. If this succeeds, we will bail out of the loop,
+        // close the database in a failsafe way, and pass the rows back.
+        // If fetching the rows throws, we will wait RETRYINTERVAL ms
+        // and try again. This will repeat a maximum of RETRYLIMIT times.
+        let db;
+        let didOpen = false;
+        let exceptionSeen;
+        try {
+          db = yield Sqlite.openConnection(dbOptions);
+          didOpen = true;
+          rows = yield db.execute(`SELECT url, title, last_visit_time, typed_count
+                                   FROM urls WHERE hidden = 0`);
+        } catch (ex) {
+          if (!exceptionSeen) {
+            Cu.reportError(ex);
+          }
+          exceptionSeen = ex;
+        } finally {
+          try {
+            if (didOpen) {
+              yield db.close();
+            }
+          } catch (ex) {}
+        }
+        if (exceptionSeen) {
+          yield new Promise(resolve => setTimeout(resolve, RETRYINTERVAL));
+        }
+      }
+      if (!rows) {
+        throw new Error("Couldn't get rows from the Chrome history database.");
+      }
+      return rows;
+    });
+  }
+
   return {
     type: MigrationUtils.resourceTypes.HISTORY,
 
     migrate(aCallback) {
       Task.spawn(function* () {
-        let db = yield Sqlite.openConnection({
+        let dbOptions = {
+          readOnly: true,
+          ignoreLockingMode: true,
           path: historyFile.path
-        });
+        };
 
-        let rows = yield db.execute(`SELECT url, title, last_visit_time, typed_count
-                                     FROM urls WHERE hidden = 0`);
-        yield db.close();
-
+        let rows = yield getRows(dbOptions);
         let places = [];
         for (let row of rows) {
           try {
             // if having typed_count, we changes transition type to typed.
             let transType = PlacesUtils.history.TRANSITION_LINK;
             if (row.getResultByName("typed_count") > 0)
               transType = PlacesUtils.history.TRANSITION_TYPED;
 
--- a/storage/mozStorageConnection.cpp
+++ b/storage/mozStorageConnection.cpp
@@ -467,32 +467,36 @@ private:
 
 } // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 //// Connection
 
 Connection::Connection(Service *aService,
                        int aFlags,
-                       bool aAsyncOnly)
+                       bool aAsyncOnly,
+                       bool aIgnoreLockingMode)
 : sharedAsyncExecutionMutex("Connection::sharedAsyncExecutionMutex")
 , sharedDBMutex("Connection::sharedDBMutex")
 , threadOpenedOn(do_GetCurrentThread())
 , mDBConn(nullptr)
 , mAsyncExecutionThreadShuttingDown(false)
 #ifdef DEBUG
 , mAsyncExecutionThreadIsAlive(false)
 #endif
 , mConnectionClosed(false)
 , mTransactionInProgress(false)
 , mProgressHandler(nullptr)
 , mFlags(aFlags)
+, mIgnoreLockingMode(aIgnoreLockingMode)
 , mStorageService(aService)
 , mAsyncOnly(aAsyncOnly)
 {
+  MOZ_ASSERT(!mIgnoreLockingMode || mFlags & SQLITE_OPEN_READONLY,
+             "Can't ignore locking for a non-readonly connection!");
   mStorageService->registerConnection(this);
 }
 
 Connection::~Connection()
 {
   (void)Close();
 
   MOZ_ASSERT(!mAsyncExecutionThread,
@@ -572,16 +576,17 @@ Connection::getAsyncExecutionTarget()
 
   return mAsyncExecutionThread;
 }
 
 nsresult
 Connection::initialize()
 {
   NS_ASSERTION (!mDBConn, "Initialize called on already opened database!");
+  MOZ_ASSERT(!mIgnoreLockingMode, "Can't ignore locking on an in-memory db.");
   PROFILER_LABEL("mozStorageConnection", "initialize",
     js::ProfileEntry::Category::STORAGE);
 
   // in memory database requested, sqlite uses a magic file name
   int srv = ::sqlite3_open_v2(":memory:", &mDBConn, mFlags, nullptr);
   if (srv != SQLITE_OK) {
     mDBConn = nullptr;
     return convertResultCode(srv);
@@ -605,18 +610,25 @@ Connection::initialize(nsIFile *aDatabas
     js::ProfileEntry::Category::STORAGE);
 
   mDatabaseFile = aDatabaseFile;
 
   nsAutoString path;
   nsresult rv = aDatabaseFile->GetPath(path);
   NS_ENSURE_SUCCESS(rv, rv);
 
+#ifdef XP_WIN
+  static const char* sIgnoreLockingVFS = "win32-none";
+#else
+  static const char* sIgnoreLockingVFS = "unix-none";
+#endif
+  const char* vfs = mIgnoreLockingMode ? sIgnoreLockingVFS : nullptr;
+
   int srv = ::sqlite3_open_v2(NS_ConvertUTF16toUTF8(path).get(), &mDBConn,
-                              mFlags, nullptr);
+                              mFlags, vfs);
   if (srv != SQLITE_OK) {
     mDBConn = nullptr;
     return convertResultCode(srv);
   }
 
   // Do not set mFileURL here since this is database does not have an associated
   // URL.
   mDatabaseFile = aDatabaseFile;
--- a/storage/mozStorageConnection.h
+++ b/storage/mozStorageConnection.h
@@ -64,18 +64,26 @@ public:
    *        connection.
    * @param aFlags
    *        The flags to pass to sqlite3_open_v2.
    * @param aAsyncOnly
    *        If |true|, the Connection only implements asynchronous interface:
    *        - |mozIStorageAsyncConnection|;
    *        If |false|, the result also implements synchronous interface:
    *        - |mozIStorageConnection|.
+   * @param aIgnoreLockingMode
+   *        If |true|, ignore locks in force on the file. Only usable with
+   *        read-only connections. Defaults to false.
+   *        Use with extreme caution. If sqlite ignores locks, reads may fail
+   *        indicating database corruption (the database won't actually be
+   *        corrupt) or produce wrong results without any indication that has
+   *        happened.
    */
-  Connection(Service *aService, int aFlags, bool aAsyncOnly);
+  Connection(Service *aService, int aFlags, bool aAsyncOnly,
+             bool aIgnoreLockingMode = false);
 
   /**
    * Creates the connection to an in-memory database.
    */
   nsresult initialize();
 
   /**
    * Creates the connection to the database.
@@ -351,16 +359,21 @@ private:
    */
   nsCOMPtr<mozIStorageProgressHandler> mProgressHandler;
 
   /**
    * Stores the flags we passed to sqlite3_open_v2.
    */
   const int mFlags;
 
+  /**
+   * Stores whether we should ask sqlite3_open_v2 to ignore locking.
+   */
+  const bool mIgnoreLockingMode;
+
   // This is here for two reasons: 1) It's used to make sure that the
   // connections do not outlive the service.  2) Our custom collating functions
   // call its localeCompareStrings() method.
   RefPtr<Service> mStorageService;
 
   /**
    * If |false|, this instance supports synchronous operations
    * and it can be cast to |mozIStorageConnection|.
--- a/storage/mozStorageService.cpp
+++ b/storage/mozStorageService.cpp
@@ -743,66 +743,89 @@ Service::OpenAsyncDatabase(nsIVariant *a
                            mozIStorageCompletionCallback *aCallback)
 {
   if (!NS_IsMainThread()) {
     return NS_ERROR_NOT_SAME_THREAD;
   }
   NS_ENSURE_ARG(aDatabaseStore);
   NS_ENSURE_ARG(aCallback);
 
-  nsCOMPtr<nsIFile> storageFile;
-  int flags = SQLITE_OPEN_READWRITE;
+  nsresult rv;
+  bool shared = false;
+  bool readOnly = false;
+  bool ignoreLockingMode = false;
+  int32_t growthIncrement = -1;
+
+#define FAIL_IF_SET_BUT_INVALID(rv)\
+  if (NS_FAILED(rv) && rv != NS_ERROR_NOT_AVAILABLE) { \
+    return NS_ERROR_INVALID_ARG; \
+  }
+
+  // Deal with options first:
+  if (aOptions) {
+    rv = aOptions->GetPropertyAsBool(NS_LITERAL_STRING("readOnly"), &readOnly);
+    FAIL_IF_SET_BUT_INVALID(rv);
 
+    rv = aOptions->GetPropertyAsBool(NS_LITERAL_STRING("ignoreLockingMode"),
+                                     &ignoreLockingMode);
+    FAIL_IF_SET_BUT_INVALID(rv);
+    // Specifying ignoreLockingMode will force use of the readOnly flag:
+    if (ignoreLockingMode) {
+      readOnly = true;
+    }
+
+    rv = aOptions->GetPropertyAsBool(NS_LITERAL_STRING("shared"), &shared);
+    FAIL_IF_SET_BUT_INVALID(rv);
+
+    // NB: we re-set to -1 if we don't have a storage file later on.
+    rv = aOptions->GetPropertyAsInt32(NS_LITERAL_STRING("growthIncrement"),
+                                      &growthIncrement);
+    FAIL_IF_SET_BUT_INVALID(rv);
+  }
+  int flags = readOnly ? SQLITE_OPEN_READONLY : SQLITE_OPEN_READWRITE;
+
+  nsCOMPtr<nsIFile> storageFile;
   nsCOMPtr<nsISupports> dbStore;
-  nsresult rv = aDatabaseStore->GetAsISupports(getter_AddRefs(dbStore));
+  rv = aDatabaseStore->GetAsISupports(getter_AddRefs(dbStore));
   if (NS_SUCCEEDED(rv)) {
     // Generally, aDatabaseStore holds the database nsIFile.
     storageFile = do_QueryInterface(dbStore, &rv);
     if (NS_FAILED(rv)) {
       return NS_ERROR_INVALID_ARG;
     }
 
     rv = storageFile->Clone(getter_AddRefs(storageFile));
     MOZ_ASSERT(NS_SUCCEEDED(rv));
 
-    // Ensure that SQLITE_OPEN_CREATE is passed in for compatibility reasons.
-    flags |= SQLITE_OPEN_CREATE;
+    if (!readOnly) {
+      // Ensure that SQLITE_OPEN_CREATE is passed in for compatibility reasons.
+      flags |= SQLITE_OPEN_CREATE;
+    }
 
-    // Extract and apply the shared-cache option.
-    bool shared = false;
-    if (aOptions) {
-      rv = aOptions->GetPropertyAsBool(NS_LITERAL_STRING("shared"), &shared);
-      if (NS_FAILED(rv) && rv != NS_ERROR_NOT_AVAILABLE) {
-        return NS_ERROR_INVALID_ARG;
-      }
-    }
+    // Apply the shared-cache option.
     flags |= shared ? SQLITE_OPEN_SHAREDCACHE : SQLITE_OPEN_PRIVATECACHE;
   } else {
     // Sometimes, however, it's a special database name.
     nsAutoCString keyString;
     rv = aDatabaseStore->GetAsACString(keyString);
     if (NS_FAILED(rv) || !keyString.EqualsLiteral("memory")) {
       return NS_ERROR_INVALID_ARG;
     }
 
     // Just fall through with nullptr storageFile, this will cause the storage
     // connection to use a memory DB.
   }
 
-  int32_t growthIncrement = -1;
-  if (aOptions && storageFile) {
-    rv = aOptions->GetPropertyAsInt32(NS_LITERAL_STRING("growthIncrement"),
-                                      &growthIncrement);
-    if (NS_FAILED(rv) && rv != NS_ERROR_NOT_AVAILABLE) {
-      return NS_ERROR_INVALID_ARG;
-    }
+  if (!storageFile && growthIncrement >= 0) {
+    return NS_ERROR_INVALID_ARG;
   }
 
   // Create connection on this thread, but initialize it on its helper thread.
-  RefPtr<Connection> msc = new Connection(this, flags, true);
+  RefPtr<Connection> msc = new Connection(this, flags, true,
+                                          ignoreLockingMode);
   nsCOMPtr<nsIEventTarget> target = msc->getAsyncExecutionTarget();
   MOZ_ASSERT(target, "Cannot initialize a connection that has been closed already");
 
   RefPtr<AsyncInitDatabase> asyncInit =
     new AsyncInitDatabase(msc,
                           storageFile,
                           growthIncrement,
                           aCallback);
--- a/storage/test/unit/test_storage_connection.js
+++ b/storage/test/unit/test_storage_connection.js
@@ -350,22 +350,37 @@ function* standardAsyncTest(promisedDB, 
 add_task(function* test_open_async() {
   yield standardAsyncTest(openAsyncDatabase(getTestDB(), null), "default");
   yield standardAsyncTest(openAsyncDatabase(getTestDB()), "no optional arg");
   yield standardAsyncTest(openAsyncDatabase(getTestDB(),
     {shared: false, growthIncrement: 54}), "non-default options");
   yield standardAsyncTest(openAsyncDatabase("memory"),
     "in-memory database", true);
   yield standardAsyncTest(openAsyncDatabase("memory",
-    {shared: false, growthIncrement: 54}),
+    {shared: false}),
     "in-memory database and options", true);
 
-  do_print("Testing async opening with bogus options 1");
+  do_print("Testing async opening with bogus options 0");
   let raised = false;
   let adb = null;
+
+  try {
+    adb = yield openAsyncDatabase("memory", {shared: false, growthIncrement: 54});
+  } catch (ex) {
+    raised = true;
+  } finally {
+    if (adb) {
+      yield asyncClose(adb);
+    }
+  }
+  do_check_true(raised);
+
+  do_print("Testing async opening with bogus options 1");
+  raised = false;
+  adb = null;
   try {
     adb = yield openAsyncDatabase(getTestDB(), {shared: "forty-two"});
   } catch (ex) {
     raised = true;
   } finally {
     if (adb) {
       yield asyncClose(adb);
     }
--- a/toolkit/modules/Sqlite.jsm
+++ b/toolkit/modules/Sqlite.jsm
@@ -863,16 +863,25 @@ ConnectionData.prototype = Object.freeze
  *   shrinkMemoryOnConnectionIdleMS -- (integer) If defined, the connection
  *       will attempt to minimize its memory usage after this many
  *       milliseconds of connection idle. The connection is idle when no
  *       statements are executing. There is no default value which means no
  *       automatic memory minimization will occur. Please note that this is
  *       *not* a timer on the idle service and this could fire while the
  *       application is active.
  *
+ *   readOnly -- (bool) Whether to open the database with SQLITE_OPEN_READONLY
+ *       set. If used, writing to the database will fail. Defaults to false.
+ *
+ *   ignoreLockingMode -- (bool) Whether to ignore locks on the database held
+ *       by other connections. If used, implies readOnly. Defaults to false.
+ *       USE WITH EXTREME CAUTION. This mode WILL produce incorrect results or
+ *       return "false positive" corruption errors if other connections write
+ *       to the DB at the same time.
+ *
  * FUTURE options to control:
  *
  *   special named databases
  *   pragma TEMP STORE = MEMORY
  *   TRUNCATE JOURNAL
  *   SYNCHRONOUS = full
  *
  * @param options
@@ -910,22 +919,31 @@ function openConnection(options) {
   }
 
   let file = FileUtils.File(path);
   let identifier = getIdentifierByPath(path);
 
   log.info("Opening database: " + path + " (" + identifier + ")");
 
   return new Promise((resolve, reject) => {
-    let dbOptions = null;
+    let dbOptions = Cc["@mozilla.org/hash-property-bag;1"].
+                    createInstance(Ci.nsIWritablePropertyBag);
     if (!sharedMemoryCache) {
-      dbOptions = Cc["@mozilla.org/hash-property-bag;1"].
-        createInstance(Ci.nsIWritablePropertyBag);
       dbOptions.setProperty("shared", false);
     }
+    if (options.readOnly) {
+      dbOptions.setProperty("readOnly", true);
+    }
+    if (options.ignoreLockingMode) {
+      dbOptions.setProperty("ignoreLockingMode", true);
+      dbOptions.setProperty("readOnly", true);
+    }
+
+    dbOptions = dbOptions.enumerator.hasMoreElements() ? dbOptions : null;
+
     Services.storage.openAsyncDatabase(file, dbOptions, (status, connection) => {
       if (!connection) {
         log.warn(`Could not open connection to ${path}: ${status}`);
         reject(new Error(`Could not open connection to ${path}: ${status}`));
         return;
       }
       log.info("Connection opened");
       try {