Bug 506805 - Remove locking in AsyncExecuteStatements
authorShawn Wilsher <sdwilsh@shawnwilsher.com>
Wed, 29 Jul 2009 17:24:50 -0700
changeset 30938 7f4a5f6f26f174b947be9c68b2e925b9201318c3
parent 30937 f9364f04f9addfb2f2f05d9d5ca400438034c936
child 30939 5acfa69898f268987e31d677413782ba191606bb
push id8297
push usersdwilsh@shawnwilsher.com
push dateThu, 30 Jul 2009 17:34:32 +0000
treeherdermozilla-central@8cff4bd2121a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs506805
milestone1.9.2a1pre
Bug 506805 - Remove locking in AsyncExecuteStatements This removes the use of the shared mutex in AsyncExecuteStatements. We now rely on PR_AtomicSet and the volatile keyword. This results in zero lock contention between the calling thread and the background thread if cancel is ever called. r=asuth r=bent sr=vlad
storage/public/mozIStoragePendingStatement.idl
storage/src/mozStorageAsyncStatementExecution.cpp
storage/src/mozStorageAsyncStatementExecution.h
storage/src/mozStorageConnection.cpp
storage/src/mozStorageConnection.h
storage/test/unit/test_statement_executeAsync.js
--- a/storage/public/mozIStoragePendingStatement.idl
+++ b/storage/public/mozIStoragePendingStatement.idl
@@ -34,22 +34,20 @@
  * and other provisions required by the GPL or the LGPL. If you do not delete
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 #include "nsISupports.idl"
 
-[scriptable, uuid(fc3c5fdc-9a87-4757-b01f-4ace2670a3a0)]
+[scriptable, uuid(00da7d20-3768-4398-bedc-e310c324b3f0)]
 interface mozIStoragePendingStatement : nsISupports {
 
   /**
    * Cancels a pending statement, if possible.  This will only fail if you try
    * cancel more than once.
    *
    * @note For read statements (such as SELECT), you will no longer receive any
    *       notifications about results once cancel is called.
-   *
-   * @returns true if canceled successfully, false otherwise.
    */
-   boolean cancel();
+   void cancel();
 };
--- a/storage/src/mozStorageAsyncStatementExecution.cpp
+++ b/storage/src/mozStorageAsyncStatementExecution.cpp
@@ -200,62 +200,53 @@ AsyncExecuteStatements::AsyncExecuteStat
                                                mozIStorageStatementCallback *aCallback)
 : mConnection(aConnection)
 , mTransactionManager(nsnull)
 , mCallback(aCallback)
 , mCallingThread(::do_GetCurrentThread())
 , mMaxWait(TimeDuration::FromMilliseconds(MAX_MILLISECONDS_BETWEEN_RESULTS))
 , mIntervalStart(TimeStamp::Now())
 , mState(PENDING)
-, mCancelRequested(PR_FALSE)
-, mMutex(aConnection->sharedAsyncExecutionMutex)
+, mCancelRequested(0)
 {
   (void)mStatements.SwapElements(aStatements);
   NS_ASSERTION(mStatements.Length(), "We weren't given any statements!");
   NS_IF_ADDREF(mCallback);
 }
 
 bool
 AsyncExecuteStatements::shouldNotify()
 {
 #ifdef DEBUG
   PRBool onCallingThread = PR_FALSE;
   (void)mCallingThread->IsOnCurrentThread(&onCallingThread);
   NS_ASSERTION(onCallingThread, "runEvent not running on the calling thread!");
 #endif
 
-  // We do not need to acquire mMutex here because it can only ever be written
-  // to on the calling thread, and the only thread that can call us is the
-  // calling thread, so we know that our access is serialized.
   return !mCancelRequested;
 }
 
 bool
 AsyncExecuteStatements::bindExecuteAndProcessStatement(StatementData &aData,
                                                        bool aLastStatement)
 {
-  mMutex.AssertNotCurrentThreadOwns();
-
   sqlite3_stmt *stmt(aData);
   BindingParamsArray *paramsArray(aData);
 
   // Iterate through all of our parameters, bind them, and execute.
   bool continueProcessing = true;
   BindingParamsArray::iterator itr = paramsArray->begin();
   BindingParamsArray::iterator end = paramsArray->end();
   while (itr != end && continueProcessing) {
     // Bind the data to our statement.
     nsCOMPtr<mozIStorageError> error;
     error = (*itr)->bind(stmt);
     if (error) {
       // Set our error state.
-      {
-        MutexAutoLock mutex(mMutex);
-        mState = ERROR;
-      }
+      (void)::PR_AtomicSet(const_cast<PRInt32 *>(&mState), ERROR);
 
       // And notify.
       (void)notifyError(error);
       return false;
     }
 
     // Advance our iterator, execute, and then process the statement.
     itr++;
@@ -268,124 +259,94 @@ AsyncExecuteStatements::bindExecuteAndPr
 
   return continueProcessing;
 }
 
 bool
 AsyncExecuteStatements::executeAndProcessStatement(sqlite3_stmt *aStatement,
                                                    bool aLastStatement)
 {
-  mMutex.AssertNotCurrentThreadOwns();
-
-  // We need to hold the mutex for statement execution so we can properly
-  // reflect state in case we are canceled.  We release the mutex in a few areas
-  // in order to allow for cancelation to occur.
-  MutexAutoLock lockedScope(mMutex);
-
   // Execute our statement
   bool hasResults;
   do {
     hasResults = executeStatement(aStatement);
 
     // If we had an error, bail.
     if (mState == ERROR)
       return false;
 
     // If we have been canceled, there is no point in going on...
     if (mCancelRequested) {
-      mState = CANCELED;
+      (void)::PR_AtomicSet(const_cast<PRInt32 *>(&mState), CANCELED);
       return false;
     }
 
     // Build our result set and notify if we got anything back and have a
     // callback to notify.
     if (mCallback && hasResults &&
         NS_FAILED(buildAndNotifyResults(aStatement))) {
       // We had an error notifying, so we notify on error and stop processing.
-      mState = ERROR;
+      (void)::PR_AtomicSet(const_cast<PRInt32 *>(&mState), ERROR);
 
-      {
-        // Drop our mutex because notifyError doesn't want it held.
-        MutexAutoUnlock unlockedScope(mMutex);
-
-        // Notify, and stop processing statements.
-        (void)notifyError(mozIStorageError::ERROR,
-                          "An error occurred while notifying about results");
-      }
+      // Notify, and stop processing statements.
+      (void)notifyError(mozIStorageError::ERROR,
+                        "An error occurred while notifying about results");
 
       return false;
     }
   } while (hasResults);
 
 #ifdef DEBUG
   // Check to make sure that this statement was smart about what it did.
   checkAndLogStatementPerformance(aStatement);
 #endif
 
-  // If we are done, we need to set our state accordingly while we still hold
-  // our mutex.  We would have already returned if we were canceled or had
-  // an error at this point.
+  // If we are done, we need to set our state accordingly.
   if (aLastStatement)
-    mState = COMPLETED;
+    (void)::PR_AtomicSet(const_cast<PRInt32 *>(&mState), COMPLETED);
 
   return true;
 }
 
 bool
 AsyncExecuteStatements::executeStatement(sqlite3_stmt *aStatement)
 {
-  mMutex.AssertCurrentThreadOwns();
-
   while (true) {
     int rc = ::sqlite3_step(aStatement);
     // Stop if we have no more results.
     if (rc == SQLITE_DONE)
       return false;
 
     // If we got results, we can return now.
     if (rc == SQLITE_ROW)
       return true;
 
     // Some errors are not fatal, and we can handle them and continue.
     if (rc == SQLITE_BUSY) {
-      // We do not want to hold our mutex while we yield.
-      MutexAutoUnlock cancelationScope(mMutex);
-
       // Yield, and try again
       (void)::PR_Sleep(PR_INTERVAL_NO_WAIT);
       continue;
     }
 
     // Set an error state.
-    mState = ERROR;
+    (void)::PR_AtomicSet(const_cast<PRInt32 *>(&mState), ERROR);
 
-    {
-      // Drop our mutex because notifyError doesn't want it held.
-      MutexAutoUnlock unlockedScope(mMutex);
-
-      // And notify.
-      sqlite3 *db = ::sqlite3_db_handle(aStatement);
-      (void)notifyError(rc, ::sqlite3_errmsg(db));
-    }
+    // And notify.
+    sqlite3 *db = ::sqlite3_db_handle(aStatement);
+    (void)notifyError(rc, ::sqlite3_errmsg(db));
 
     // Finally, indicate that we should stop processing.
     return false;
   }
 }
 
 nsresult
 AsyncExecuteStatements::buildAndNotifyResults(sqlite3_stmt *aStatement)
 {
   NS_ASSERTION(mCallback, "Trying to dispatch results without a callback!");
-  mMutex.AssertCurrentThreadOwns();
-
-  // At this point, it is safe to not hold the mutex and allow for cancelation.
-  // We may add an event to the calling thread, but that thread will not end
-  // up running when it checks back with us to see if it should run.
-  MutexAutoUnlock cancelationScope(mMutex);
 
   // Build result object if we need it.
   if (!mResultSet)
     mResultSet = new ResultSet();
   NS_ENSURE_TRUE(mResultSet, NS_ERROR_OUT_OF_MEMORY);
 
   nsRefPtr<Row> row(new Row());
   NS_ENSURE_TRUE(row, NS_ERROR_OUT_OF_MEMORY);
@@ -412,92 +373,86 @@ AsyncExecuteStatements::buildAndNotifyRe
   }
 
   return NS_OK;
 }
 
 nsresult
 AsyncExecuteStatements::notifyComplete()
 {
-  mMutex.AssertNotCurrentThreadOwns();
   NS_ASSERTION(mState != PENDING,
                "Still in a pending state when calling Complete!");
 
   // Finalize our statements before we try to commit or rollback.  If we are
   // canceling and have statements that think they have pending work, the
   // rollback will fail.
   for (PRUint32 i = 0; i < mStatements.Length(); i++)
     mStatements[i].finalize();
 
   // Handle our transaction, if we have one
   if (mTransactionManager) {
     if (mState == COMPLETED) {
       nsresult rv = mTransactionManager->Commit();
       if (NS_FAILED(rv)) {
-        mState = ERROR;
+        (void)::PR_AtomicSet(const_cast<PRInt32 *>(&mState), ERROR);
         (void)notifyError(mozIStorageError::ERROR,
                           "Transaction failed to commit");
       }
     }
     else {
       (void)mTransactionManager->Rollback();
     }
     delete mTransactionManager;
     mTransactionManager = nsnull;
   }
 
   // Notify about completion iff we have a callback.
   if (mCallback) {
     nsRefPtr<CompletionNotifier> completionEvent =
-      new CompletionNotifier(mCallback, mState);
+      new CompletionNotifier(mCallback, static_cast<ExecutionState>(mState));
     NS_ENSURE_TRUE(completionEvent, NS_ERROR_OUT_OF_MEMORY);
 
     // We no longer own mCallback (the CompletionNotifier takes ownership).
     mCallback = nsnull;
 
     (void)mCallingThread->Dispatch(completionEvent, NS_DISPATCH_NORMAL);
   }
 
   return NS_OK;
 }
 
 nsresult
 AsyncExecuteStatements::notifyError(PRInt32 aErrorCode,
                                     const char *aMessage)
 {
-  mMutex.AssertNotCurrentThreadOwns();
-
   if (!mCallback)
     return NS_OK;
 
   nsCOMPtr<mozIStorageError> errorObj(new Error(aErrorCode, aMessage));
   NS_ENSURE_TRUE(errorObj, NS_ERROR_OUT_OF_MEMORY);
 
   return notifyError(errorObj);
 }
 
 nsresult
 AsyncExecuteStatements::notifyError(mozIStorageError *aError)
 {
-  mMutex.AssertNotCurrentThreadOwns();
-
   if (!mCallback)
     return NS_OK;
 
   nsRefPtr<ErrorNotifier> notifier =
     new ErrorNotifier(mCallback, aError, this);
   NS_ENSURE_TRUE(notifier, NS_ERROR_OUT_OF_MEMORY);
 
   return mCallingThread->Dispatch(notifier, NS_DISPATCH_NORMAL);
 }
 
 nsresult
 AsyncExecuteStatements::notifyResults()
 {
-  mMutex.AssertNotCurrentThreadOwns();
   NS_ASSERTION(mCallback, "notifyResults called without a callback!");
 
   nsRefPtr<CallbackResultNotifier> notifier =
     new CallbackResultNotifier(mCallback, mResultSet, this);
   NS_ENSURE_TRUE(notifier, NS_ERROR_OUT_OF_MEMORY);
 
   nsresult rv = mCallingThread->Dispatch(notifier, NS_DISPATCH_NORMAL);
   if (NS_SUCCEEDED(rv))
@@ -510,63 +465,52 @@ NS_IMPL_THREADSAFE_ISUPPORTS2(
   nsIRunnable,
   mozIStoragePendingStatement
 )
 
 ////////////////////////////////////////////////////////////////////////////////
 //// mozIStoragePendingStatement
 
 NS_IMETHODIMP
-AsyncExecuteStatements::Cancel(PRBool *_successful)
+AsyncExecuteStatements::Cancel()
 {
 #ifdef DEBUG
   PRBool onCallingThread = PR_FALSE;
   (void)mCallingThread->IsOnCurrentThread(&onCallingThread);
   NS_ASSERTION(onCallingThread, "Not canceling from the calling thread!");
 #endif
 
   // If we have already canceled, we have an error, but always indicate that
   // we are trying to cancel.
   NS_ENSURE_FALSE(mCancelRequested, NS_ERROR_UNEXPECTED);
 
-  {
-    MutexAutoLock lockedScope(mMutex);
-
-    // We need to indicate that we want to try and cancel now.
-    mCancelRequested = true;
+  // Indicate that we want to try and cancel at the next cancelation point.
+  (void)::PR_AtomicSet(const_cast<PRInt32 *>(&mCancelRequested), 1);
 
-    // Establish if we can cancel
-    *_successful = (mState == PENDING);
-  }
-
-  // Note, it is possible for us to return false here, and end up canceling
+  // Note:  While we are requesting to cancel here, it is possible that we will
+  // not actually be able to.  However, we will still suppress handleResult
   // events that have been dispatched to the calling thread.  This is OK,
   // however, because only read statements (such as SELECT) are going to be
   // posting events to the calling thread that actually check if they should
   // run or not.
 
   return NS_OK;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 //// nsIRunnable
 
 NS_IMETHODIMP
 AsyncExecuteStatements::Run()
 {
   // Do not run if we have been canceled.
-  bool cancelRequested;
-  {
-    MutexAutoLock lockedScope(mMutex);
-    cancelRequested = mCancelRequested;
-    if (cancelRequested)
-      mState = CANCELED;
+  if (mCancelRequested) {
+    (void)::PR_AtomicSet(const_cast<PRInt32 *>(&mState), CANCELED);
+    return notifyComplete();
   }
-  if (cancelRequested)
-    return notifyComplete();
 
   // If there is more than one statement, run it in a transaction.  We assume
   // that we have been given write statements since getting a batch of read
   // statements doesn't make a whole lot of sense.
   // Additionally, if we have only one statement and it needs a transaction, we
   // will wrap it in one.
   if (mStatements.Length() > 1 || mStatements[0].needsTransaction()) {
     // We don't error if this failed because it's not terrible if it does.
--- a/storage/src/mozStorageAsyncStatementExecution.h
+++ b/storage/src/mozStorageAsyncStatementExecution.h
@@ -39,17 +39,16 @@
 
 #ifndef _mozStorageAsyncStatementExecution_h_
 #define _mozStorageAsyncStatementExecution_h_
 
 #include "nscore.h"
 #include "nsTArray.h"
 #include "nsAutoPtr.h"
 #include "nsThreadUtils.h"
-#include "mozilla/Mutex.h"
 #include "mozilla/TimeStamp.h"
 
 #include "mozIStoragePendingStatement.h"
 #include "mozIStorageStatementCallback.h"
 
 struct sqlite3_stmt;
 class mozStorageTransaction;
 
@@ -113,92 +112,78 @@ private:
                          Connection *aConnection,
                          mozIStorageStatementCallback *aCallback);
 
   /**
    * 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
-   *
    * @param aData
    *        The StatementData to bind, execute, and then process.
    * @param aLastStatement
    *        Indicates if this is the last statement or not.  If it is, we have
    *        to set the proper state.
    * @returns true if we should continue to process statements, false otherwise.
    */
   bool bindExecuteAndProcessStatement(StatementData &aData,
                                       bool aLastStatement);
 
   /**
    * 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
-   *
    * @param aStatement
    *        The statement to execute and then process.
    * @param aLastStatement
    *        Indicates if this is the last statement or not.  If it is, we have
    *        to set the proper state.
    * @returns true if we should continue to process statements, false otherwise.
    */
   bool executeAndProcessStatement(sqlite3_stmt *aStatement,
                                   bool aLastStatement);
 
   /**
    * Executes a statement to completion, properly handling any error conditions.
    *
-   * @pre mMutex is held
-   *
    * @param aStatement
    *        The statement to execute to completion.
    * @returns true if results were obtained, false otherwise.
    */
   bool executeStatement(sqlite3_stmt *aStatement);
 
   /**
    * Builds a result set up with a row from a given statement.  If we meet the
    * right criteria, go ahead and notify about this results too.
    *
-   * @pre mMutex is held
-   *
    * @param aStatement
    *        The statement to get the row data from.
    */
   nsresult buildAndNotifyResults(sqlite3_stmt *aStatement);
 
   /**
    * Notifies callback about completion, and does any necessary cleanup.
-   *
-   * @pre mMutex is not held
    */
   nsresult notifyComplete();
 
   /**
    * Notifies callback about an error.
    *
-   * @pre mMutex is not held
-   *
    * @param aErrorCode
    *        The error code defined in mozIStorageError for the error.
    * @param aMessage
    *        The error string, if any.
    * @param aError
    *        The error object to notify the caller with.
    */
   nsresult notifyError(PRInt32 aErrorCode, const char *aMessage);
   nsresult notifyError(mozIStorageError *aError);
 
   /**
    * Notifies the callback about a result set.
-   *
-   * @pre mMutex is not held
    */
   nsresult notifyResults();
 
   StatementDataArray mStatements;
   nsRefPtr<Connection> mConnection;
   mozStorageTransaction *mTransactionManager;
   mozIStorageStatementCallback *mCallback;
   nsCOMPtr<nsIThread> mCallingThread;
@@ -211,32 +196,28 @@ private:
   const TimeDuration mMaxWait;
 
   /**
    * The start time since our last set of results.
    */
   TimeStamp mIntervalStart;
 
   /**
-   * Indicates our state of execution.
+   * Indicates our state of execution.  This is one of the values in the
+   * ExecutionState enum.
+   *
+   * @note This should only be set with PR_Atomic* functions.
    */
-  ExecutionState mState;
+  volatile PRInt32 mState;
 
   /**
-   * Indicates if we should try to cancel at a cancelation point.
+   * Indicates if we should try to cancel at a cancelation point or not.  This
+   * is evaluated as a boolean.
+   *
+   * @note This should only be set with PR_Atomic* functions.
    */
-  bool mCancelRequested;
-
-  /**
-   * This is the mutex tat protects our state from changing between threads.
-   * This includes the following variables:
-   *   - mState
-   *   - mCancelRequested is only set on the calling thread while the lock is
-   *     held.  It is always read from within the lock on the background thread,
-   *     but not on the calling thread (see shouldNotify for why).
-   */
-  Mutex &mMutex;
+  volatile PRInt32 mCancelRequested;
 };
 
 } // namespace storage
 } // namespace mozilla
 
 #endif // _mozStorageAsyncStatementExecution_h_
--- a/storage/src/mozStorageConnection.cpp
+++ b/storage/src/mozStorageConnection.cpp
@@ -240,18 +240,17 @@ aggregateFunctionFinalHelper(sqlite3_con
 
 
 } // anonymous namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 //// Connection
 
 Connection::Connection(Service *aService)
-: sharedAsyncExecutionMutex("Connection::sharedAsyncExecutionMutex")
-, mDBConn(nsnull)
+: mDBConn(nsnull)
 , mAsyncExecutionMutex(nsAutoLock::NewLock("AsyncExecutionMutex"))
 , mAsyncExecutionThreadShuttingDown(PR_FALSE)
 , mTransactionMutex(nsAutoLock::NewLock("TransactionMutex"))
 , mTransactionInProgress(PR_FALSE)
 , mFunctionsMutex(nsAutoLock::NewLock("FunctionsMutex"))
 , mProgressHandlerMutex(nsAutoLock::NewLock("ProgressHandlerMutex"))
 , mProgressHandler(nsnull)
 , mStorageService(aService)
--- a/storage/src/mozStorageConnection.h
+++ b/storage/src/mozStorageConnection.h
@@ -38,17 +38,16 @@
  *
  * ***** END LICENSE BLOCK ***** */
 
 #ifndef _MOZSTORAGECONNECTION_H_
 #define _MOZSTORAGECONNECTION_H_
 
 #include "nsAutoPtr.h"
 #include "nsCOMPtr.h"
-#include "mozilla/Mutex.h"
 
 #include "nsString.h"
 #include "nsInterfaceHashtable.h"
 #include "mozIStorageProgressHandler.h"
 #include "mozIStorageConnection.h"
 #include "mozStorageService.h"
 
 #include "nsIMutableArray.h"
@@ -88,23 +87,16 @@ public:
    * Lazily creates and returns a background execution thread.  In the future,
    * the thread may be re-claimed if left idle, so you should call this
    * method just before you dispatch and not save the reference.
    *
    * @returns an event target suitable for asynchronous statement execution.
    */
   already_AddRefed<nsIEventTarget> getAsyncExecutionTarget();
 
-  /**
-   * Mutex used by asynchronous statements to protect state.  The mutex is
-   * declared on the connection object because there is no contention between
-   * asynchronous statements (they are serialized on mAsyncExecutionThread).
-   */
-  Mutex sharedAsyncExecutionMutex;
-
 private:
   ~Connection();
 
   /**
    * Describes a certain primitive type in the database.
    *
    * Possible Values Are:
    *  INDEX - To check for the existence of an index
--- a/storage/test/unit/test_statement_executeAsync.js
+++ b/storage/test/unit/test_statement_executeAsync.js
@@ -368,82 +368,78 @@ function test_partial_listener_works()
 }
 
 function test_immediate_cancellation()
 {
   var stmt = getOpenedDatabase().createStatement(
     "DELETE FROM test WHERE id = ?"
   );
   stmt.bindInt32Parameter(0, 0);
-  let reason = Ci.mozIStorageStatementCallback.REASON_CANCELED;
   var pendingStatement = stmt.executeAsync({
     handleResult: function(aResultSet)
     {
       do_throw("unexpected result!");
     },
     handleError: function(aError)
     {
       print("Error code " + aError.result + " with message '" +
             aError.message + "' returned.");
       do_throw("unexpected error!");
     },
     handleCompletion: function(aReason)
     {
       print("handleCompletion(" + aReason +
             ") for test_immediate_cancellation");
-      do_check_eq(reason, aReason);
+      // It is possible that we finished before we canceled.
+      do_check_true(aReason == Ci.mozIStorageStatementCallback.REASON_FINISHED ||
+                    aReason == Ci.mozIStorageStatementCallback.REASON_CANCELED);
 
       // Run the next test.
       run_next_test();
     }
   });
 
   // Cancel immediately
-  if (!pendingStatement.cancel()) {
-    // It is possible that we finished before we canceled
-    reason = Ci.mozIStorageStatementCallback.REASON_FINISHED;
-  }
+  pendingStatement.cancel()
 
   stmt.finalize();
 }
 
 function test_double_cancellation()
 {
   var stmt = getOpenedDatabase().createStatement(
     "DELETE FROM test WHERE id = ?"
   );
   stmt.bindInt32Parameter(0, 0);
-  let reason = Ci.mozIStorageStatementCallback.REASON_CANCELED;
   var pendingStatement = stmt.executeAsync({
     handleResult: function(aResultSet)
     {
       do_throw("unexpected result!");
     },
     handleError: function(aError)
     {
       print("Error code " + aError.result + " with message '" +
             aError.message + "' returned.");
       do_throw("unexpected error!");
     },
     handleCompletion: function(aReason)
     {
       print("handleCompletion(" + aReason +
             ") for test_double_cancellation");
-      do_check_eq(reason, aReason);
+      // It is possible that we finished before we canceled.
+      do_check_true(aReason == Ci.mozIStorageStatementCallback.REASON_FINISHED ||
+                    aReason == Ci.mozIStorageStatementCallback.REASON_CANCELED);
 
       // Run the next test.
       run_next_test();
     }
   });
 
   // Cancel immediately
-  if (!pendingStatement.cancel()) {
-    // It is possible that we finished before we canceled
-    reason = Ci.mozIStorageStatementCallback.REASON_FINISHED;
-  }
+  pendingStatement.cancel()
 
   // And cancel again - expect an exception
   try {
     pendingStatement.cancel();
     do_throw("function call should have thrown!");
   }
   catch (e) {
     do_check_eq(Cr.NS_ERROR_UNEXPECTED, e.result);