Bug 1285041 - ignore locking when trying to read chrome DB file, r=mak
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Mon, 18 Jul 2016 16:46:45 +0100
changeset 355498 6267fc17f8c02ce375589be5e9ac7acdf6bad126
parent 355497 9f345e640bd78c21ac2bef12ca6f329e546abc85
child 355499 054e4e24a0c1a90db80c8ba33418bb0ab3be5672
push id6570
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:26:13 +0000
treeherdermozilla-beta@f455459b2ae5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1285041
milestone51.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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 {