Bug 914070 - Part 3 - Make async transaction management work with a null mDBConn. r=asuth
authorMarco Bonardo <mbonardo@mozilla.com>
Thu, 24 Apr 2014 11:54:15 +0200
changeset 198492 dbf72e1458ffbd7f690459da3f95a77a14d6e49c
parent 198491 528a58952f8c2389ec7d59bf31ad15a4eb420f13
child 198493 b9660304f8ef367e44d56ecf63d41263459d3baa
push id3624
push userasasaki@mozilla.com
push dateMon, 09 Jun 2014 21:49:01 +0000
treeherdermozilla-beta@b1a5da15899a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs914070
milestone31.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 914070 - Part 3 - Make async transaction management work with a null mDBConn. r=asuth
storage/src/mozStorageAsyncStatementExecution.cpp
storage/src/mozStorageAsyncStatementExecution.h
storage/src/mozStorageConnection.cpp
storage/src/mozStorageConnection.h
--- a/storage/src/mozStorageAsyncStatementExecution.cpp
+++ b/storage/src/mozStorageAsyncStatementExecution.cpp
@@ -195,32 +195,37 @@ AsyncExecuteStatements::execute(Statemen
 }
 
 AsyncExecuteStatements::AsyncExecuteStatements(StatementDataArray &aStatements,
                                                Connection *aConnection,
                                                sqlite3 *aNativeConnection,
                                                mozIStorageStatementCallback *aCallback)
 : mConnection(aConnection)
 , mNativeConnection(aNativeConnection)
-, mTransactionManager(nullptr)
+, mHasTransaction(false)
 , mCallback(aCallback)
 , mCallingThread(::do_GetCurrentThread())
 , mMaxWait(TimeDuration::FromMilliseconds(MAX_MILLISECONDS_BETWEEN_RESULTS))
 , mIntervalStart(TimeStamp::Now())
 , mState(PENDING)
 , mCancelRequested(false)
 , mMutex(aConnection->sharedAsyncExecutionMutex)
 , mDBMutex(aConnection->sharedDBMutex)
   , mRequestStartDate(TimeStamp::Now())
 {
   (void)mStatements.SwapElements(aStatements);
   NS_ASSERTION(mStatements.Length(), "We weren't given any statements!");
   NS_IF_ADDREF(mCallback);
 }
 
+AsyncExecuteStatements::~AsyncExecuteStatements()
+{
+  MOZ_ASSERT(!mHasTransaction, "There should be no transaction at this point");
+}
+
 bool
 AsyncExecuteStatements::shouldNotify()
 {
 #ifdef DEBUG
   mMutex.AssertNotCurrentThreadOwns();
 
   bool onCallingThread = false;
   (void)mCallingThread->IsOnCurrentThread(&onCallingThread);
@@ -433,30 +438,29 @@ AsyncExecuteStatements::notifyComplete()
 
   // Release references to the statement data as soon as possible. If this
   // is the last reference, statements will be finalized immediately on the
   // async thread, hence avoiding several bounces between threads and possible
   // race conditions with AsyncClose().
   mStatements.Clear();
 
   // Handle our transaction, if we have one
-  if (mTransactionManager) {
+  if (mHasTransaction) {
     if (mState == COMPLETED) {
-      nsresult rv = mTransactionManager->Commit();
+      nsresult rv = mConnection->commitTransactionInternal(mNativeConnection);
       if (NS_FAILED(rv)) {
         mState = ERROR;
         (void)notifyError(mozIStorageError::ERROR,
                           "Transaction failed to commit");
       }
     }
     else {
-      (void)mTransactionManager->Rollback();
+      NS_WARN_IF(NS_FAILED(mConnection->rollbackTransactionInternal(mNativeConnection)));
     }
-    delete mTransactionManager;
-    mTransactionManager = nullptr;
+    mHasTransaction = false;
   }
 
   // Always generate a completion notification; it is what guarantees that our
   // destruction does not happen here on the async thread.
   nsRefPtr<CompletionNotifier> completionEvent =
     new CompletionNotifier(mCallback, mState);
 
   // We no longer own mCallback (the CompletionNotifier takes ownership).
@@ -576,18 +580,25 @@ AsyncExecuteStatements::Run()
     if (mCancelRequested)
       mState = CANCELED;
   }
   if (mState == CANCELED)
     return notifyComplete();
 
   if (statementsNeedTransaction()) {
     Connection* rawConnection = static_cast<Connection*>(mConnection.get());
-    mTransactionManager = new mozStorageAsyncTransaction(rawConnection, false,
-                                                         mozIStorageConnection::TRANSACTION_IMMEDIATE);
+    if (NS_SUCCEEDED(mConnection->beginTransactionInternal(mNativeConnection,
+                                                           mozIStorageConnection::TRANSACTION_IMMEDIATE))) {
+      mHasTransaction = true;
+    }
+#ifdef DEBUG
+    else {
+      NS_WARNING("Unable to create a transaction for async execution.");
+    }
+#endif
   }
 
   // Execute each statement, giving the callback results if it returns any.
   for (uint32_t i = 0; i < mStatements.Length(); i++) {
     bool finished = (i == (mStatements.Length() - 1));
 
     sqlite3_stmt *stmt;
     { // lock the sqlite mutex so sqlite3_errmsg cannot change
--- a/storage/src/mozStorageAsyncStatementExecution.h
+++ b/storage/src/mozStorageAsyncStatementExecution.h
@@ -24,24 +24,16 @@ struct sqlite3_stmt;
 
 namespace mozilla {
 namespace storage {
 
 class Connection;
 class ResultSet;
 class StatementData;
 
-/**
- * An instance of the mozStorageTransaction<> family dedicated
- * to concrete class |Connection|.
- */
-typedef mozStorageTransactionBase<mozilla::storage::Connection,
-                                  nsRefPtr<mozilla::storage::Connection> >
-    mozStorageAsyncTransaction;
-
 class AsyncExecuteStatements MOZ_FINAL : public nsIRunnable
                                        , public mozIStoragePendingStatement
 {
 public:
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSIRUNNABLE
   NS_DECL_MOZISTORAGEPENDINGSTATEMENT
 
@@ -90,16 +82,17 @@ public:
    */
   bool shouldNotify();
 
 private:
   AsyncExecuteStatements(StatementDataArray &aStatements,
                          Connection *aConnection,
                          sqlite3 *aNativeConnection,
                          mozIStorageStatementCallback *aCallback);
+  ~AsyncExecuteStatements();
 
   /**
    * Binds and then executes a given statement until completion, an error
    * occurs, or we are canceled.  If aLastStatement is true, we should set
    * mState accordingly.
    *
    * @pre mMutex is not held
    *
@@ -187,17 +180,17 @@ private:
    *
    * @return true if an explicit transaction is needed, false otherwise.
    */
   bool statementsNeedTransaction();
 
   StatementDataArray mStatements;
   nsRefPtr<Connection> mConnection;
   sqlite3 *mNativeConnection;
-  mozStorageAsyncTransaction *mTransactionManager;
+  bool mHasTransaction;
   mozIStorageStatementCallback *mCallback;
   nsCOMPtr<nsIThread> mCallingThread;
   nsRefPtr<ResultSet> mResultSet;
 
   /**
    * The maximum amount of time we want to wait between results.  Defined by
    * MAX_MILLISECONDS_BETWEEN_RESULTS and set at construction.
    */
--- a/storage/src/mozStorageConnection.cpp
+++ b/storage/src/mozStorageConnection.cpp
@@ -665,17 +665,17 @@ Connection::initializeInternal(nsIFile* 
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Setting the cache_size forces the database open, verifying if it is valid
   // or corrupt.  So this is executed regardless it being actually needed.
   // The cache_size is calculated from the actual page_size, to save memory.
   nsAutoCString cacheSizeQuery(MOZ_STORAGE_UNIQUIFY_QUERY_STR
                                "PRAGMA cache_size = ");
   cacheSizeQuery.AppendInt(-MAX_CACHE_SIZE_KIBIBYTES);
-  int srv = executeSql(cacheSizeQuery.get());
+  int srv = executeSql(mDBConn, cacheSizeQuery.get());
   if (srv != SQLITE_OK) {
     ::sqlite3_close(mDBConn);
     mDBConn = nullptr;
     return convertResultCode(srv);
   }
 
   // Register our built-in SQL functions.
   srv = registerFunctions(mDBConn);
@@ -1051,23 +1051,24 @@ Connection::prepareStatement(sqlite3 *aN
     return SQLITE_MISUSE;
   }
 
   return rc;
 }
 
 
 int
-Connection::executeSql(const char *aSqlString)
+Connection::executeSql(sqlite3 *aNativeConnection, const char *aSqlString)
 {
-  if (!mDBConn)
+  if (isClosed())
     return SQLITE_MISUSE;
 
   TimeStamp startTime = TimeStamp::Now();
-  int srv = ::sqlite3_exec(mDBConn, aSqlString, nullptr, nullptr, nullptr);
+  int srv = ::sqlite3_exec(aNativeConnection, aSqlString, nullptr, nullptr,
+                           nullptr);
 
   // Report very slow SQL statements to Telemetry
   TimeDuration duration = TimeStamp::Now() - startTime;
   const uint32_t threshold =
     NS_IsMainThread() ? Telemetry::kSlowSQLThresholdForMainThread
                       : Telemetry::kSlowSQLThresholdForHelperThreads;
   if (duration.ToMilliseconds() >= threshold) {
     nsDependentCString statementString(aSqlString);
@@ -1406,17 +1407,17 @@ Connection::CreateAsyncStatement(const n
   return NS_OK;
 }
 
 NS_IMETHODIMP
 Connection::ExecuteSimpleSQL(const nsACString &aSQLStatement)
 {
   if (!mDBConn) return NS_ERROR_NOT_INITIALIZED;
 
-  int srv = executeSql(PromiseFlatCString(aSQLStatement).get());
+  int srv = executeSql(mDBConn, PromiseFlatCString(aSQLStatement).get());
   return convertResultCode(srv);
 }
 
 NS_IMETHODIMP
 Connection::ExecuteAsync(mozIStorageBaseStatement **aStatements,
                          uint32_t aNumStatements,
                          mozIStorageStatementCallback *aCallback,
                          mozIStoragePendingStatement **_handle)
@@ -1498,81 +1499,101 @@ Connection::BeginTransaction()
   return BeginTransactionAs(mozIStorageConnection::TRANSACTION_DEFERRED);
 }
 
 NS_IMETHODIMP
 Connection::BeginTransactionAs(int32_t aTransactionType)
 {
   if (!mDBConn) return NS_ERROR_NOT_INITIALIZED;
 
+  return beginTransactionInternal(mDBConn, aTransactionType);
+}
+
+nsresult
+Connection::beginTransactionInternal(sqlite3 *aNativeConnection,
+                                     int32_t aTransactionType)
+{
   SQLiteMutexAutoLock lockedScope(sharedDBMutex);
   if (mTransactionInProgress)
     return NS_ERROR_FAILURE;
   nsresult rv;
   switch(aTransactionType) {
     case TRANSACTION_DEFERRED:
-      rv = ExecuteSimpleSQL(NS_LITERAL_CSTRING("BEGIN DEFERRED"));
+      rv = convertResultCode(executeSql(aNativeConnection, "BEGIN DEFERRED"));
       break;
     case TRANSACTION_IMMEDIATE:
-      rv = ExecuteSimpleSQL(NS_LITERAL_CSTRING("BEGIN IMMEDIATE"));
+      rv = convertResultCode(executeSql(aNativeConnection, "BEGIN IMMEDIATE"));
       break;
     case TRANSACTION_EXCLUSIVE:
-      rv = ExecuteSimpleSQL(NS_LITERAL_CSTRING("BEGIN EXCLUSIVE"));
+      rv = convertResultCode(executeSql(aNativeConnection, "BEGIN EXCLUSIVE"));
       break;
     default:
       return NS_ERROR_ILLEGAL_VALUE;
   }
   if (NS_SUCCEEDED(rv))
     mTransactionInProgress = true;
   return rv;
 }
 
 NS_IMETHODIMP
 Connection::CommitTransaction()
 {
   if (!mDBConn)
     return NS_ERROR_NOT_INITIALIZED;
 
+  return commitTransactionInternal(mDBConn);
+}
+
+nsresult
+Connection::commitTransactionInternal(sqlite3 *aNativeConnection)
+{
   SQLiteMutexAutoLock lockedScope(sharedDBMutex);
   if (!mTransactionInProgress)
     return NS_ERROR_UNEXPECTED;
-
-  nsresult rv = ExecuteSimpleSQL(NS_LITERAL_CSTRING("COMMIT TRANSACTION"));
+  nsresult rv =
+    convertResultCode(executeSql(aNativeConnection, "COMMIT TRANSACTION"));
   if (NS_SUCCEEDED(rv))
     mTransactionInProgress = false;
   return rv;
 }
 
 NS_IMETHODIMP
 Connection::RollbackTransaction()
 {
   if (!mDBConn)
     return NS_ERROR_NOT_INITIALIZED;
 
+  return rollbackTransactionInternal(mDBConn);
+}
+
+nsresult
+Connection::rollbackTransactionInternal(sqlite3 *aNativeConnection)
+{
   SQLiteMutexAutoLock lockedScope(sharedDBMutex);
   if (!mTransactionInProgress)
     return NS_ERROR_UNEXPECTED;
 
-  nsresult rv = ExecuteSimpleSQL(NS_LITERAL_CSTRING("ROLLBACK TRANSACTION"));
+  nsresult rv =
+    convertResultCode(executeSql(aNativeConnection, "ROLLBACK TRANSACTION"));
   if (NS_SUCCEEDED(rv))
     mTransactionInProgress = false;
   return rv;
 }
 
 NS_IMETHODIMP
 Connection::CreateTable(const char *aTableName,
                         const char *aTableSchema)
 {
   if (!mDBConn) return NS_ERROR_NOT_INITIALIZED;
 
   char *buf = ::PR_smprintf("CREATE TABLE %s (%s)", aTableName, aTableSchema);
   if (!buf)
     return NS_ERROR_OUT_OF_MEMORY;
 
-  int srv = executeSql(buf);
+  int srv = executeSql(mDBConn, buf);
   ::PR_smprintf_free(buf);
 
   return convertResultCode(srv);
 }
 
 NS_IMETHODIMP
 Connection::CreateFunction(const nsACString &aFunctionName,
                            int32_t aNumArguments,
--- a/storage/src/mozStorageConnection.h
+++ b/storage/src/mozStorageConnection.h
@@ -186,16 +186,26 @@ public:
    * @param aNativeConnection
    *        The underlying Sqlite connection to step the statement with.
    * @param aStatement
    *        A pointer to a sqlite3_stmt object.
    * @return the result from sqlite3_step.
    */
   int stepStatement(sqlite3* aNativeConnection, sqlite3_stmt* aStatement);
 
+  /**
+   * Raw connection transaction management.
+   *
+   * @see BeginTransactionAs, CommitTransaction, RollbackTransaction.
+   */
+  nsresult beginTransactionInternal(sqlite3 *aNativeConnection,
+                                    int32_t aTransactionType=TRANSACTION_DEFERRED);
+  nsresult commitTransactionInternal(sqlite3 *aNativeConnection);
+  nsresult rollbackTransactionInternal(sqlite3 *aNativeConnection);
+
   bool connectionReady();
 
   /**
    * True if this connection is shutting down but not yet closed.
    */
   bool isClosing();
 
   /**
@@ -217,21 +227,23 @@ private:
    *
    * @note mDBConn is set to nullptr in this method.
    */
   nsresult setClosedState();
 
   /**
    * Helper for calls to sqlite3_exec. Reports long delays to Telemetry.
    *
+   * @param aNativeConnection
+   *        The underlying Sqlite connection to execute the query with.
    * @param aSqlString
    *        SQL string to execute
    * @return the result from sqlite3_exec.
    */
-  int executeSql(const char *aSqlString);
+  int executeSql(sqlite3 *aNativeConnection, const char *aSqlString);
 
   /**
    * Describes a certain primitive type in the database.
    *
    * Possible Values Are:
    *  INDEX - To check for the existence of an index
    *  TABLE - To check for the existence of a table
    */