Bug 1694231 - Add support for nested mozStorageTransaction using savepoints; r=dom-storage-reviewers,sg
authorJan Varga <jvarga@mozilla.com>
Wed, 03 Mar 2021 18:53:14 +0000
changeset 569518 316fdd5817ae62591d1c63f983ab608765096ce7
parent 569517 38aa4daa99a6d1f6f095c782e22233de0cf7e4e0
child 569519 4de89f9a02771d23230b0a98f436d3cd7395323a
push id38261
push userabutkovits@mozilla.com
push dateThu, 04 Mar 2021 04:07:40 +0000
treeherdermozilla-central@eee3ec3004e4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdom-storage-reviewers, sg
bugs1694231
milestone88.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 1694231 - Add support for nested mozStorageTransaction using savepoints; r=dom-storage-reviewers,sg The nesting level is tracked on the storage connection. The thread safety is ensured by holding a lock while a transaction is being started/commited/rolled back. For these purposes, the sharedDBMutex has been exposed on mozIStorageConnection interface and additional helper methods have been added to the interface as well. Differential Revision: https://phabricator.services.mozilla.com/D106019
dom/cache/Connection.cpp
storage/moz.build
storage/mozIStorageConnection.idl
storage/mozStorageConnection.cpp
storage/mozStorageConnection.h
storage/mozStorageHelper.h
storage/test/gtest/test_transaction_helper.cpp
--- a/dom/cache/Connection.cpp
+++ b/dom/cache/Connection.cpp
@@ -246,9 +246,28 @@ Connection::EnableModule(const nsACStrin
 }
 
 NS_IMETHODIMP
 Connection::GetQuotaObjects(QuotaObject** aDatabaseQuotaObject,
                             QuotaObject** aJournalQuotaObject) {
   return mBase->GetQuotaObjects(aDatabaseQuotaObject, aJournalQuotaObject);
 }
 
+mozilla::storage::SQLiteMutex& Connection::GetSharedDBMutex() {
+  return mBase->GetSharedDBMutex();
+}
+
+uint32_t Connection::GetTransactionNestingLevel(
+    const mozilla::storage::SQLiteMutexAutoLock& aProofOfLock) {
+  return mBase->GetTransactionNestingLevel(aProofOfLock);
+}
+
+uint32_t Connection::IncreaseTransactionNestingLevel(
+    const mozilla::storage::SQLiteMutexAutoLock& aProofOfLock) {
+  return mBase->IncreaseTransactionNestingLevel(aProofOfLock);
+}
+
+uint32_t Connection::DecreaseTransactionNestingLevel(
+    const mozilla::storage::SQLiteMutexAutoLock& aProofOfLock) {
+  return mBase->DecreaseTransactionNestingLevel(aProofOfLock);
+}
+
 }  // namespace mozilla::dom::cache
--- a/storage/moz.build
+++ b/storage/moz.build
@@ -43,16 +43,17 @@ EXPORTS.mozilla += [
 ]
 
 # NOTE When adding something to this list, you probably need to add it to the
 #      storage.h file too.
 EXPORTS.mozilla.storage += [
     "mozStorageAsyncStatementParams.h",
     "mozStorageStatementParams.h",
     "mozStorageStatementRow.h",
+    "SQLiteMutex.h",
     "StatementCache.h",
     "Variant.h",
     "Variant_inl.h",
 ]
 # SEE ABOVE NOTE!
 
 UNIFIED_SOURCES += [
     "FileSystemModule.cpp",
--- a/storage/mozIStorageConnection.idl
+++ b/storage/mozIStorageConnection.idl
@@ -2,27 +2,30 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsISupports.idl"
 #include "mozIStorageAsyncConnection.idl"
 
 %{C++
-namespace mozilla {
-namespace dom {
-namespace quota {
+namespace mozilla::dom::quota {
 class QuotaObject;
 }
-}
+
+namespace mozilla::storage {
+class SQLiteMutex;
+class SQLiteMutexAutoLock;
 }
 
 %}
 
 [ptr] native QuotaObject(mozilla::dom::quota::QuotaObject);
+native SQLiteMutex(mozilla::storage::SQLiteMutex&);
+native SQLiteMutexAutoLock(const mozilla::storage::SQLiteMutexAutoLock&);
 
 interface mozIStorageAggregateFunction;
 interface mozIStorageCompletionCallback;
 interface mozIStorageFunction;
 interface mozIStorageProgressHandler;
 interface mozIStorageBaseStatement;
 interface mozIStorageStatement;
 interface mozIStorageAsyncStatement;
@@ -36,17 +39,17 @@ interface nsIFile;
  * primary interface for interacting with a database, including
  * creating prepared statements, executing SQL, and examining database
  * errors.
  *
  * @note From the main thread, you should rather use mozIStorageAsyncConnection.
  *
  * @threadsafe
  */
-[scriptable, uuid(4aa2ac47-8d24-4004-9b31-ec0bd85f0cc3)]
+[scriptable, builtinclass, uuid(4aa2ac47-8d24-4004-9b31-ec0bd85f0cc3)]
 interface mozIStorageConnection : mozIStorageAsyncConnection {
   /**
    * Closes a database connection.  Callers must finalize all statements created
    * for this connection prior to calling this method.  It is illegal to use
    * call this method if any asynchronous statements have been executed on this
    * connection.
    *
    * @throws NS_ERROR_UNEXPECTED
@@ -251,9 +254,30 @@ interface mozIStorageConnection : mozISt
    *             The QuotaObject associated with the database file.
    * @param[out] aJournalQuotaObject
    *             The QuotaObject associated with the journal file.
    *
    * @throws NS_ERROR_NOT_INITIALIZED.
    */
   [noscript] void getQuotaObjects(out QuotaObject aDatabaseQuotaObject,
                                   out QuotaObject aJournalQuotaObject);
+
+  /**
+   * The mutex used for protection of operations (BEGIN/COMMIT/ROLLBACK) in
+   * mozStorageTransaction. The lock must be held in a way that spans whole
+   * operation, not just when accessing the nesting level.
+   */
+  [notxpcom, nostdcall] readonly attribute SQLiteMutex sharedDBMutex;
+
+  /**
+   * Helper methods for managing the transaction nesting level. The methods
+   * must be called with a proof of lock. Currently only used by
+   * mozStorageTransaction.
+   */
+  [notxpcom, nostdcall] unsigned long getTransactionNestingLevel(
+      in SQLiteMutexAutoLock aProofOfLock);
+
+  [notxpcom, nostdcall] unsigned long increaseTransactionNestingLevel(
+      in SQLiteMutexAutoLock aProofOfLock);
+
+  [notxpcom, nostdcall] unsigned long decreaseTransactionNestingLevel(
+      in SQLiteMutexAutoLock aProofOfLock);
 };
--- a/storage/mozStorageConnection.cpp
+++ b/storage/mozStorageConnection.cpp
@@ -439,17 +439,18 @@ Connection::Connection(Service* aService
       mAsyncExecutionThreadShuttingDown(false),
       mConnectionClosed(false),
       mDefaultTransactionType(mozIStorageConnection::TRANSACTION_DEFERRED),
       mDestroying(false),
       mProgressHandler(nullptr),
       mFlags(aFlags),
       mIgnoreLockingMode(aIgnoreLockingMode),
       mStorageService(aService),
-      mSupportedOperations(aSupportedOperations) {
+      mSupportedOperations(aSupportedOperations),
+      mTransactionNestingLevel(0) {
   MOZ_ASSERT(!mIgnoreLockingMode || mFlags & SQLITE_OPEN_READONLY,
              "Can't ignore locking for a non-readonly connection!");
   mStorageService->registerConnection(this);
 }
 
 Connection::~Connection() {
   // Failsafe Close() occurs in our custom Release method because of
   // complications related to Close() potentially invoking AsyncClose() which
@@ -2339,9 +2340,26 @@ Connection::GetQuotaObjects(QuotaObject*
     return NS_ERROR_FAILURE;
   }
 
   databaseQuotaObject.forget(aDatabaseQuotaObject);
   journalQuotaObject.forget(aJournalQuotaObject);
   return NS_OK;
 }
 
+SQLiteMutex& Connection::GetSharedDBMutex() { return sharedDBMutex; }
+
+uint32_t Connection::GetTransactionNestingLevel(
+    const mozilla::storage::SQLiteMutexAutoLock& aProofOfLock) {
+  return mTransactionNestingLevel;
+}
+
+uint32_t Connection::IncreaseTransactionNestingLevel(
+    const mozilla::storage::SQLiteMutexAutoLock& aProofOfLock) {
+  return ++mTransactionNestingLevel;
+}
+
+uint32_t Connection::DecreaseTransactionNestingLevel(
+    const mozilla::storage::SQLiteMutexAutoLock& aProofOfLock) {
+  return --mTransactionNestingLevel;
+}
+
 }  // namespace mozilla::storage
--- a/storage/mozStorageConnection.h
+++ b/storage/mozStorageConnection.h
@@ -467,16 +467,18 @@ class Connection final : public mozIStor
   RefPtr<Service> mStorageService;
 
   /**
    * Indicates which operations are supported on this connection.
    */
   const ConnectionOperation mSupportedOperations;
 
   nsresult synchronousClose();
+
+  uint32_t mTransactionNestingLevel;
 };
 
 /**
  * A Runnable designed to call a mozIStorageCompletionCallback on
  * the appropriate thread.
  */
 class CallbackComplete final : public Runnable {
  public:
--- a/storage/mozStorageHelper.h
+++ b/storage/mozStorageHelper.h
@@ -5,34 +5,36 @@
 
 #ifndef MOZSTORAGEHELPER_H
 #define MOZSTORAGEHELPER_H
 
 #include "nsCOMPtr.h"
 #include "nsString.h"
 #include "mozilla/DebugOnly.h"
 
+#include "mozilla/storage/SQLiteMutex.h"
 #include "mozIStorageConnection.h"
 #include "mozIStorageStatement.h"
 #include "mozIStoragePendingStatement.h"
 #include "nsError.h"
 
 /**
  * This class wraps a transaction inside a given C++ scope, guaranteeing that
  * the transaction will be completed even if you have an exception or
  * return early.
  *
  * A common use is to create an instance with aCommitOnComplete = false
  * (rollback), then call Commit() on this object manually when your function
  * completes successfully.
  *
- * @note nested transactions are not supported by Sqlite, so if a transaction
- * is already in progress, this object does nothing.  Note that in this case,
- * you may not get the transaction type you asked for, and you won't be able
- * to rollback.
+ * @note nested transactions are not supported by Sqlite, only nested
+ * savepoints, so if a transaction is already in progress, this object creates
+ * a nested savepoint to the existing transaction which is considered as
+ * anonymous savepoint itself. However, aType and aAsyncCommit are ignored
+ * in the case of nested savepoints.
  *
  * @param aConnection
  *        The connection to create the transaction on.
  * @param aCommitOnComplete
  *        Controls whether the transaction is committed or rolled back when
  *        this object goes out of scope.
  * @param aType [optional]
  *        The transaction type, as defined in mozIStorageConnection. Uses the
@@ -50,48 +52,67 @@
  *        Finally, if the database is using WAL journaling mode, other
  *        connections won't see the changes done in async committed transactions
  *        until commit is complete.
  *
  *        For all of the above reasons, this should only be used as an interim
  *        solution and avoided completely if possible.
  */
 class mozStorageTransaction {
+  using SQLiteMutexAutoLock = mozilla::storage::SQLiteMutexAutoLock;
+
  public:
   mozStorageTransaction(
       mozIStorageConnection* aConnection, bool aCommitOnComplete,
       int32_t aType = mozIStorageConnection::TRANSACTION_DEFAULT,
       bool aAsyncCommit = false)
       : mConnection(aConnection),
+        mNestingLevel(0),
         mHasTransaction(false),
         mCommitOnComplete(aCommitOnComplete),
         mCompleted(false),
         mAsyncCommit(aAsyncCommit) {
     if (mConnection) {
-      nsAutoCString query("BEGIN");
-      int32_t type = aType;
-      if (type == mozIStorageConnection::TRANSACTION_DEFAULT) {
-        MOZ_ALWAYS_SUCCEEDS(mConnection->GetDefaultTransactionType(&type));
+      SQLiteMutexAutoLock lock(mConnection->GetSharedDBMutex());
+
+      // We nee to speculatively set the nesting level to be able to decide
+      // if this is a top level transaction and to be able to generate the
+      // savepoint name.
+      TransactionStarted(lock);
+
+      nsAutoCString query;
+
+      if (TopLevelTransaction(lock)) {
+        query.Assign("BEGIN");
+        int32_t type = aType;
+        if (type == mozIStorageConnection::TRANSACTION_DEFAULT) {
+          MOZ_ALWAYS_SUCCEEDS(mConnection->GetDefaultTransactionType(&type));
+        }
+        switch (type) {
+          case mozIStorageConnection::TRANSACTION_IMMEDIATE:
+            query.AppendLiteral(" IMMEDIATE");
+            break;
+          case mozIStorageConnection::TRANSACTION_EXCLUSIVE:
+            query.AppendLiteral(" EXCLUSIVE");
+            break;
+          case mozIStorageConnection::TRANSACTION_DEFERRED:
+            query.AppendLiteral(" DEFERRED");
+            break;
+          default:
+            MOZ_ASSERT(false, "Unknown transaction type");
+        }
+      } else {
+        query.Assign("SAVEPOINT sp"_ns + IntToCString(mNestingLevel));
       }
-      switch (type) {
-        case mozIStorageConnection::TRANSACTION_IMMEDIATE:
-          query.AppendLiteral(" IMMEDIATE");
-          break;
-        case mozIStorageConnection::TRANSACTION_EXCLUSIVE:
-          query.AppendLiteral(" EXCLUSIVE");
-          break;
-        case mozIStorageConnection::TRANSACTION_DEFERRED:
-          query.AppendLiteral(" DEFERRED");
-          break;
-        default:
-          MOZ_ASSERT(false, "Unknown transaction type");
+
+      // If the query fails to execute we need to revert the speculatively set
+      // nesting level on the connection.
+      if (NS_FAILED(mConnection->ExecuteSimpleSQL(query))) {
+        TransactionFinished(lock);
       }
-      // If a transaction is already in progress, this will fail, since Sqlite
-      // doesn't support nested transactions.
-      mHasTransaction = NS_SUCCEEDED(mConnection->ExecuteSimpleSQL(query));
     }
   }
 
   ~mozStorageTransaction() {
     if (mConnection && mHasTransaction && !mCompleted) {
       if (mCommitOnComplete) {
         mozilla::DebugOnly<nsresult> rv = Commit();
         NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
@@ -106,58 +127,133 @@ class mozStorageTransaction {
 
   /**
    * Commits the transaction if one is in progress. If one is not in progress,
    * this is a NOP since the actual owner of the transaction outside of our
    * scope is in charge of finally committing or rolling back the transaction.
    */
   nsresult Commit() {
     if (!mConnection || mCompleted || !mHasTransaction) return NS_OK;
+
+    SQLiteMutexAutoLock lock(mConnection->GetSharedDBMutex());
+
+#ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
+    MOZ_DIAGNOSTIC_ASSERT(CurrentTransaction(lock));
+#else
+    if (!CurrentTransaction(lock)) {
+      return NS_ERROR_NOT_AVAILABLE;
+    }
+#endif
+
     mCompleted = true;
 
-    // TODO (bug 559659): this might fail with SQLITE_BUSY, but we don't handle
-    // it, thus the transaction might stay open until the next COMMIT.
     nsresult rv;
-    if (mAsyncCommit) {
-      nsCOMPtr<mozIStoragePendingStatement> ps;
-      rv = mConnection->ExecuteSimpleSQLAsync("COMMIT"_ns, nullptr,
-                                              getter_AddRefs(ps));
+
+    if (TopLevelTransaction(lock)) {
+      // TODO (bug 559659): this might fail with SQLITE_BUSY, but we don't
+      // handle it, thus the transaction might stay open until the next COMMIT.
+      if (mAsyncCommit) {
+        nsCOMPtr<mozIStoragePendingStatement> ps;
+        rv = mConnection->ExecuteSimpleSQLAsync("COMMIT"_ns, nullptr,
+                                                getter_AddRefs(ps));
+      } else {
+        rv = mConnection->ExecuteSimpleSQL("COMMIT"_ns);
+      }
     } else {
-      rv = mConnection->ExecuteSimpleSQL("COMMIT"_ns);
+      rv = mConnection->ExecuteSimpleSQL("RELEASE sp"_ns +
+                                         IntToCString(mNestingLevel));
     }
 
-    if (NS_SUCCEEDED(rv)) mHasTransaction = false;
+    if (NS_SUCCEEDED(rv)) {
+      TransactionFinished(lock);
+    }
 
     return rv;
   }
 
   /**
    * Rolls back the transaction if one is in progress. If one is not in
    * progress, this is a NOP since the actual owner of the transaction outside
    * of our scope is in charge of finally rolling back the transaction.
    */
   nsresult Rollback() {
     if (!mConnection || mCompleted || !mHasTransaction) return NS_OK;
+
+    SQLiteMutexAutoLock lock(mConnection->GetSharedDBMutex());
+
+#ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
+    MOZ_DIAGNOSTIC_ASSERT(CurrentTransaction(lock));
+#else
+    if (!CurrentTransaction(lock)) {
+      return NS_ERROR_NOT_AVAILABLE;
+    }
+#endif
+
     mCompleted = true;
 
-    // TODO (bug 1062823): from Sqlite 3.7.11 on, rollback won't ever return
-    // a busy error, so this handling can be removed.
     nsresult rv;
-    do {
-      rv = mConnection->ExecuteSimpleSQL("ROLLBACK"_ns);
-      if (rv == NS_ERROR_STORAGE_BUSY) (void)PR_Sleep(PR_INTERVAL_NO_WAIT);
-    } while (rv == NS_ERROR_STORAGE_BUSY);
 
-    if (NS_SUCCEEDED(rv)) mHasTransaction = false;
+    if (TopLevelTransaction(lock)) {
+      // TODO (bug 1062823): from Sqlite 3.7.11 on, rollback won't ever return
+      // a busy error, so this handling can be removed.
+      do {
+        rv = mConnection->ExecuteSimpleSQL("ROLLBACK"_ns);
+        if (rv == NS_ERROR_STORAGE_BUSY) (void)PR_Sleep(PR_INTERVAL_NO_WAIT);
+      } while (rv == NS_ERROR_STORAGE_BUSY);
+    } else {
+      const auto nestingLevelCString = IntToCString(mNestingLevel);
+      rv = mConnection->ExecuteSimpleSQL(
+          "ROLLBACK TO sp"_ns + nestingLevelCString + "; RELEASE sp"_ns +
+          nestingLevelCString);
+    }
+
+    if (NS_SUCCEEDED(rv)) {
+      TransactionFinished(lock);
+    }
 
     return rv;
   }
 
  protected:
+  void TransactionStarted(const SQLiteMutexAutoLock& aProofOfLock) {
+    MOZ_ASSERT(mConnection);
+    MOZ_ASSERT(!mHasTransaction);
+    MOZ_ASSERT(mNestingLevel == 0);
+    mHasTransaction = true;
+    mNestingLevel = mConnection->IncreaseTransactionNestingLevel(aProofOfLock);
+  }
+
+  bool CurrentTransaction(const SQLiteMutexAutoLock& aProofOfLock) const {
+    MOZ_ASSERT(mConnection);
+    MOZ_ASSERT(mHasTransaction);
+    MOZ_ASSERT(mNestingLevel > 0);
+    return mNestingLevel ==
+           mConnection->GetTransactionNestingLevel(aProofOfLock);
+  }
+
+  bool TopLevelTransaction(const SQLiteMutexAutoLock& aProofOfLock) const {
+    MOZ_ASSERT(mConnection);
+    MOZ_ASSERT(mHasTransaction);
+    MOZ_ASSERT(mNestingLevel > 0);
+    MOZ_ASSERT(CurrentTransaction(aProofOfLock));
+    return mNestingLevel == 1;
+  }
+
+  void TransactionFinished(const SQLiteMutexAutoLock& aProofOfLock) {
+    MOZ_ASSERT(mConnection);
+    MOZ_ASSERT(mHasTransaction);
+    MOZ_ASSERT(mNestingLevel > 0);
+    MOZ_ASSERT(CurrentTransaction(aProofOfLock));
+    mConnection->DecreaseTransactionNestingLevel(aProofOfLock);
+    mNestingLevel = 0;
+    mHasTransaction = false;
+  }
+
   nsCOMPtr<mozIStorageConnection> mConnection;
+  uint32_t mNestingLevel;
   bool mHasTransaction;
   bool mCommitOnComplete;
   bool mCompleted;
   bool mAsyncCommit;
 };
 
 /**
  * This class wraps a statement so that it is guaraneed to be reset when
--- a/storage/test/gtest/test_transaction_helper.cpp
+++ b/storage/test/gtest/test_transaction_helper.cpp
@@ -134,8 +134,43 @@ TEST(storage_transaction_helper, async_C
   stmt->Finalize();
   do_check_false(has_transaction(db));
   bool exists = false;
   (void)db->TableExists("test"_ns, &exists);
   do_check_true(exists);
 
   blocking_async_close(db);
 }
+
+TEST(storage_transaction_helper, Nesting)
+{
+  nsCOMPtr<mozIStorageConnection> db(getMemoryDatabase());
+
+  {
+    mozStorageTransaction transaction(db, false);
+    do_check_true(has_transaction(db));
+    do_check_success(
+        db->ExecuteSimpleSQL("CREATE TABLE test (id INTEGER PRIMARY KEY)"_ns));
+
+    {
+      mozStorageTransaction nestedTransaction(db, false);
+      do_check_true(has_transaction(db));
+      do_check_success(db->ExecuteSimpleSQL(
+          "CREATE TABLE nested_test (id INTEGER PRIMARY KEY)"_ns));
+
+#ifndef MOZ_DIAGNOSTIC_ASSERT_ENABLED
+      do_check_true(transaction.Commit() == NS_ERROR_NOT_AVAILABLE);
+      do_check_true(transaction.Rollback() == NS_ERROR_NOT_AVAILABLE);
+#endif
+    }
+
+    bool exists = false;
+    do_check_success(db->TableExists("nested_test"_ns, &exists));
+    do_check_false(exists);
+
+    (void)transaction.Commit();
+  }
+  do_check_false(has_transaction(db));
+
+  bool exists = false;
+  do_check_success(db->TableExists("test"_ns, &exists));
+  do_check_true(exists);
+}