Bug 506022 - Avoid obtaining the database mutex at all costs in Connection::ExecuteAsync
authorShawn Wilsher <sdwilsh@shawnwilsher.com>
Tue, 28 Jul 2009 10:21:03 -0700
changeset 30834 cae7b01ca38f9f79b45b7836232cb8c432fd4269
parent 30833 44fab2f1ab6464c31f431e17e2b524d7767f2f72
child 30835 3b1ef8e6425fbf0b5336d80a4f8198ab7098e0b7
push id8272
push usersdwilsh@shawnwilsher.com
push dateWed, 29 Jul 2009 18:27:55 +0000
treeherdermozilla-central@3b1ef8e6425f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs506022
milestone1.9.2a1pre
Bug 506022 - Avoid obtaining the database mutex at all costs in Connection::ExecuteAsync Stop calling any SQLite functions that would acquire the database mutex inside Connection::ExecuteAsync. Additionally, do not actually bind parameters when the binding functions are called, but rather when we execute (so for async statements, that will happen on the background thread creating no mutex contention with the main thread). r=asuth
storage/src/Variant.h
storage/src/mozStorageAsyncStatementExecution.cpp
storage/src/mozStorageBindingParams.cpp
storage/src/mozStorageBindingParams.h
storage/src/mozStorageBindingParamsArray.h
storage/src/mozStorageConnection.cpp
storage/src/mozStorageStatement.cpp
storage/src/mozStorageStatement.h
storage/src/mozStorageStatementData.h
storage/test/unit/test_statement_executeAsync.js
storage/test/unit/test_storage_connection.js
--- a/storage/src/Variant.h
+++ b/storage/src/Variant.h
@@ -284,17 +284,25 @@ struct variant_storage_traits<PRUint8[]>
 template < >
 struct variant_blob_traits<PRUint8[]>
 {
   static inline nsresult asArray(nsTArray<PRUint8> &aData,
                                  PRUint16 *_type,
                                  PRUint32 *_size,
                                  void **_result)
   {
-    // Copy the array
+    // For empty blobs, we return nsnull.
+    if (aData.Length() == 0) {
+      *_result = nsnull;
+      *_type = nsIDataType::VTYPE_UINT8;
+      *_size = 0;
+      return NS_OK;
+    }
+
+    // Otherwise, we copy the array.
     *_result = nsMemory::Clone(aData.Elements(), aData.Length() * sizeof(PRUint8));
     NS_ENSURE_TRUE(*_result, NS_ERROR_OUT_OF_MEMORY);
 
     // Set type and size
     *_type = nsIDataType::VTYPE_UINT8;
     *_size = aData.Length();
     return NS_OK;
   }
--- a/storage/src/mozStorageAsyncStatementExecution.cpp
+++ b/storage/src/mozStorageAsyncStatementExecution.cpp
@@ -561,19 +561,19 @@ AsyncExecuteStatements::Run()
       mState = CANCELED;
   }
   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 has parameters to be
-  // bound, we assume that the consumer would want a transaction as well.
-  if (mStatements.Length() > 1 || mStatements[0].hasParametersToBeBound()) {
+  // 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.
     mTransactionManager = new mozStorageTransaction(mConnection, PR_FALSE,
                                                     mozIStorageConnection::TRANSACTION_IMMEDIATE);
   }
 
   // Execute each statement, giving the callback results if it returns any.
   for (PRUint32 i = 0; i < mStatements.Length(); i++) {
     bool finished = (i == (mStatements.Length() - 1));
--- a/storage/src/mozStorageBindingParams.cpp
+++ b/storage/src/mozStorageBindingParams.cpp
@@ -131,31 +131,39 @@ sqlite3_T_blob(BindingColumnData aData,
 
 BindingParams::BindingParams(BindingParamsArray *aOwningArray,
                              Statement *aOwningStatement)
 : mOwningArray(aOwningArray)
 , mOwningStatement(aOwningStatement)
 , mLocked(false)
 {
   (void)mOwningStatement->GetParameterCount(&mParamCount);
+  (void)mParameters.SetCapacity(mParamCount);
 }
 
 void
 BindingParams::lock()
 {
   NS_ASSERTION(mLocked == false, "Parameters have already been locked!");
   mLocked = true;
 
   // We no longer need to hold a reference to our statement or our owning array.
   // The array owns us at this point, and it will own a reference to the
   // statement.
   mOwningStatement = nsnull;
   mOwningArray = nsnull;
 }
 
+void
+BindingParams::unlock()
+{
+  NS_ASSERTION(mLocked == true, "Parameters were not yet locked!");
+  mLocked = false;
+}
+
 const BindingParamsArray *
 BindingParams::getOwner() const
 {
   return mOwningArray;
 }
 
 already_AddRefed<mozIStorageError>
 BindingParams::bind(sqlite3_stmt *aStatement)
@@ -280,17 +288,17 @@ BindingParams::BindBlobByName(const nsAC
 NS_IMETHODIMP
 BindingParams::BindByIndex(PRUint32 aIndex,
                            nsIVariant *aValue)
 {
   NS_ENSURE_FALSE(mLocked, NS_ERROR_UNEXPECTED);
   ENSURE_INDEX_VALUE(aIndex, mParamCount);
 
   // Store the variant for later use.
-  NS_ENSURE_TRUE(mParameters.InsertObjectAt(aValue, aIndex),
+  NS_ENSURE_TRUE(mParameters.ReplaceObjectAt(aValue, aIndex),
                  NS_ERROR_OUT_OF_MEMORY);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BindingParams::BindUTF8StringByIndex(PRUint32 aIndex,
                                      const nsACString &aValue)
 {
--- a/storage/src/mozStorageBindingParams.h
+++ b/storage/src/mozStorageBindingParams.h
@@ -62,16 +62,21 @@ public:
 
   /**
    * Locks the parameters and prevents further modification to it (such as
    * binding more elements to it).
    */
   void lock();
 
   /**
+   * Unlocks the parameters and allows modification to it again.
+   */
+  void unlock();
+
+  /**
    * @returns the pointer to the owning BindingParamsArray.
    */
   const BindingParamsArray *getOwner() const;
 
   /**
    * Binds our stored data to the statement.
    *
    * @param aStatement
--- a/storage/src/mozStorageBindingParamsArray.h
+++ b/storage/src/mozStorageBindingParamsArray.h
@@ -54,27 +54,34 @@ class Statement;
 class BindingParamsArray : public mozIStorageBindingParamsArray
 {
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_MOZISTORAGEBINDINGPARAMSARRAY
 
   BindingParamsArray(Statement *aOwningStatement);
 
+  typedef nsTArray_base::size_type size_type;
+
   /**
    * Locks the array and prevents further modification to it (such as adding
    * more elements to it).
    */
   void lock();
 
   /**
-   * @returns the pointer to the owning BindingParamsArray.
+   * @return the pointer to the owning BindingParamsArray.
    */
   const Statement *getOwner() const;
 
+  /**
+   * @return the number of elemets the array contains.
+   */
+  const size_type length() const { return mArray.Length(); }
+
   class iterator {
   public:
     iterator(BindingParamsArray *aArray,
              PRUint32 aIndex)
     : mArray(aArray)
     , mIndex(aIndex)
     {
     }
@@ -90,42 +97,44 @@ public:
       return mIndex == aOther.mIndex;
     }
     bool operator!=(const iterator &aOther) const
     {
       return !(*this == aOther);
     }
     BindingParams *operator*()
     {
-      NS_ASSERTION(mIndex < mArray->mArray.Length(),
+      NS_ASSERTION(mIndex < mArray->length(),
                    "Dereferenceing an invalid value!");
       return mArray->mArray[mIndex].get();
     }
   private:
     void operator--() { }
     BindingParamsArray *mArray;
     PRUint32 mIndex;
   };
 
   /**
    * Obtains an iterator pointing to the beginning of the array.
    */
   inline iterator begin()
   {
-    NS_ASSERTION(mLocked, "Obtaining an iterator when we are not locked!");
+    NS_ASSERTION(length() != 0,
+                 "Obtaining an iterator to the beginning with no elements!");
     return iterator(this, 0);
   }
 
   /**
    * Obtains an iterator pointing to the end of the array.
    */
   inline iterator end()
   {
-    NS_ASSERTION(mLocked, "Obtaining an iterator when we are not locked!");
-    return iterator(this, mArray.Length());
+    NS_ASSERTION(mLocked,
+                 "Obtaining an iterator to the end when we are not locked!");
+    return iterator(this, length());
   }
 private:
   nsRefPtr<Statement> mOwningStatement;
   nsTArray< nsRefPtr<BindingParams> > mArray;
   bool mLocked;
 
   friend class iterator;
 };
--- a/storage/src/mozStorageConnection.cpp
+++ b/storage/src/mozStorageConnection.cpp
@@ -634,74 +634,34 @@ Connection::ExecuteSimpleSQL(const nsACS
 }
 
 nsresult
 Connection::ExecuteAsync(mozIStorageStatement **aStatements,
                          PRUint32 aNumStatements,
                          mozIStorageStatementCallback *aCallback,
                          mozIStoragePendingStatement **_handle)
 {
-  int rc = SQLITE_OK;
   nsTArray<StatementData> stmts(aNumStatements);
-  for (PRUint32 i = 0; i < aNumStatements && rc == SQLITE_OK; i++) {
-    sqlite3_stmt *old_stmt =
-        static_cast<Statement *>(aStatements[i])->nativeStatement();
-    if (!old_stmt) {
-      rc = SQLITE_MISUSE;
-      break;
-    }
-    NS_ASSERTION(::sqlite3_db_handle(old_stmt) == mDBConn,
+  for (PRUint32 i = 0; i < aNumStatements; i++) {
+    Statement *stmt = static_cast<Statement *>(aStatements[i]);
+
+    // Obtain our StatementData.
+    StatementData data;
+    nsresult rv = stmt->getAsynchronousStatementData(data);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    NS_ASSERTION(::sqlite3_db_handle(stmt->nativeStatement()) == mDBConn,
                  "Statement must be from this database connection!");
 
-    // Clone this statement.  We only need a sqlite3_stmt object, so we can
-    // avoid all the extra work that making a new Statement would normally
-    // involve and use the SQLite API.
-    sqlite3_stmt *new_stmt;
-    rc = ::sqlite3_prepare_v2(mDBConn, ::sqlite3_sql(old_stmt), -1, &new_stmt,
-                              NULL);
-    if (rc != SQLITE_OK)
-      break;
-
-#ifdef PR_LOGGING
-    PR_LOG(gStorageLog, PR_LOG_NOTICE,
-           ("Cloned statement 0x%p to 0x%p", old_stmt, new_stmt));
-#endif
-
-    // Transfer the bindings
-    rc = sqlite3_transfer_bindings(old_stmt, new_stmt);
-    if (rc != SQLITE_OK)
-      break;
-
-    Statement *storageStmt = static_cast<Statement *>(aStatements[i]);
-    StatementData data(new_stmt, storageStmt->bindingParamsArray());
-    if (!stmts.AppendElement(data)) {
-      rc = SQLITE_NOMEM;
-      break;
-    }
+    // Now append it to our array.
+    NS_ENSURE_TRUE(stmts.AppendElement(data), NS_ERROR_OUT_OF_MEMORY);
   }
 
   // Dispatch to the background
-  nsresult rv = NS_OK;
-  if (rc == SQLITE_OK)
-    rv = AsyncExecuteStatements::execute(stmts, this, aCallback, _handle);
-
-  // We had a failure, so we need to clean up...
-  if (rc != SQLITE_OK || NS_FAILED(rv)) {
-    for (PRUint32 i = 0; i < stmts.Length(); i++)
-      (void)::sqlite3_finalize(stmts[i]);
-
-    if (rc != SQLITE_OK)
-      rv = convertResultCode(rc);
-  }
-
-  // Always reset all the statements
-  for (PRUint32 i = 0; i < aNumStatements; i++)
-    (void)aStatements[i]->Reset();
-
-  return rv;
+  return AsyncExecuteStatements::execute(stmts, this, aCallback, _handle);
 }
 
 NS_IMETHODIMP
 Connection::TableExists(const nsACString &aTableName,
                         PRBool *_exists)
 {
     return databaseElementExists(TABLE, aTableName, _exists);
 }
--- a/storage/src/mozStorageStatement.cpp
+++ b/storage/src/mozStorageStatement.cpp
@@ -38,19 +38,23 @@
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 #include <stdio.h>
 
 #include "nsError.h"
 #include "nsMemory.h"
+#include "nsThreadUtils.h"
 #include "nsIClassInfoImpl.h"
 #include "nsIProgrammingLanguage.h"
 
+#include "mozIStorageError.h"
+
+#include "mozStorageBindingParams.h"
 #include "mozStorageConnection.h"
 #include "mozStorageStatementJSHelper.h"
 #include "mozStoragePrivateHelpers.h"
 #include "mozStorageStatementParams.h"
 #include "mozStorageStatementRow.h"
 #include "mozStorageStatement.h"
 
 #include "prlog.h"
@@ -58,16 +62,43 @@
 #ifdef PR_LOGGING
 extern PRLogModuleInfo* gStorageLog;
 #endif
 
 namespace mozilla {
 namespace storage {
 
 ////////////////////////////////////////////////////////////////////////////////
+//// Local Classes
+
+namespace {
+
+/**
+ * Used to finalize an asynchronous statement on the background thread.
+ */
+class AsyncStatementFinalizer : public nsRunnable
+{
+public:
+  AsyncStatementFinalizer(sqlite3_stmt *aStatement)
+  : mStatement(aStatement)
+  {
+  }
+
+  NS_IMETHOD Run()
+  {
+    (void)::sqlite3_finalize(mStatement);
+    return NS_OK;
+  }
+private:
+  sqlite3_stmt *mStatement;
+};
+
+} // anonymous namespace
+
+////////////////////////////////////////////////////////////////////////////////
 //// nsIClassInfo
 
 NS_IMPL_CI_INTERFACE_GETTER2(
   Statement,
   mozIStorageStatement,
   mozIStorageValueArray
 )
 
@@ -146,16 +177,17 @@ static StatementClassInfo sStatementClas
 ////////////////////////////////////////////////////////////////////////////////
 //// Statement
 
 Statement::Statement()
 : mDBConnection(nsnull)
 , mDBStatement(NULL)
 , mColumnNames()
 , mExecuting(false)
+, mCachedAsyncStatement(NULL)
 {
 }
 
 nsresult
 Statement::initialize(Connection *aDBConnection,
                       const nsACString &aSQLStatement)
 {
   NS_ASSERTION(aDBConnection, "No database connection given!");
@@ -226,16 +258,92 @@ Statement::initialize(Connection *aDBCon
     start = e;
     e = end;
   }
 #endif
 
   return NS_OK;
 }
 
+nsresult
+Statement::getAsynchronousStatementData(StatementData &_data)
+{
+  if (!mDBStatement)
+    return NS_ERROR_UNEXPECTED;
+
+  sqlite3_stmt *stmt;
+  int rc = getAsyncStatement(&stmt);
+  if (rc != SQLITE_OK)
+    return convertResultCode(rc);
+
+  _data = StatementData(stmt, bindingParamsArray(), this);
+
+  return NS_OK;
+}
+
+int
+Statement::getAsyncStatement(sqlite3_stmt **_stmt)
+{
+  // If we have no statement, we shouldn't be calling this method!
+  NS_ASSERTION(mDBStatement != NULL, "We have no statement to clone!");
+
+  // If we do not yet have a cached async statement, clone our statement now.
+  if (!mCachedAsyncStatement) {
+    int rc = ::sqlite3_prepare_v2(mDBConnection->GetNativeConnection(),
+                                  ::sqlite3_sql(mDBStatement), -1,
+                                  &mCachedAsyncStatement, NULL);
+    if (rc != SQLITE_OK)
+      return rc;
+
+#ifdef PR_LOGGING
+    PR_LOG(gStorageLog, PR_LOG_NOTICE,
+           ("Cloned statement 0x%p to 0x%p", mDBStatement,
+            mCachedAsyncStatement));
+#endif
+  }
+
+  *_stmt = mCachedAsyncStatement;
+  return SQLITE_OK;
+}
+
+BindingParams *
+Statement::getParams()
+{
+  nsresult rv;
+
+  // If we do not have an array object yet, make it.
+  if (!mParamsArray) {
+    nsCOMPtr<mozIStorageBindingParamsArray> array;
+    rv = NewBindingParamsArray(getter_AddRefs(array));
+    NS_ENSURE_SUCCESS(rv, nsnull);
+
+    mParamsArray = static_cast<BindingParamsArray *>(array.get());
+  }
+
+  // If there isn't already any rows added, we'll have to add one to use.
+  if (mParamsArray->length() == 0) {
+    nsRefPtr<BindingParams> params(new BindingParams(mParamsArray, this));
+    NS_ENSURE_TRUE(params, nsnull);
+
+    rv = mParamsArray->AddParams(params);
+    NS_ENSURE_SUCCESS(rv, nsnull);
+
+    // We have to unlock our params because AddParams locks them.  This is safe
+    // because no reference to the params object was, or ever will be given out.
+    params->unlock();
+
+    // We also want to lock our array at this point - we don't want anything to
+    // be added to it.  Nothing has, or will ever get a reference to it, but we
+    // will get additional safety checks via assertions by doing this.
+    mParamsArray->lock();
+  }
+
+  return *mParamsArray->begin();
+}
+
 Statement::~Statement()
 {
   (void)Finalize();
 }
 
 NS_IMPL_THREADSAFE_ADDREF(Statement)
 NS_IMPL_THREADSAFE_RELEASE(Statement)
 
@@ -275,16 +383,30 @@ Statement::Finalize()
 #ifdef PR_LOGGING
   PR_LOG(gStorageLog, PR_LOG_NOTICE, ("Finalizing statement '%s'",
                                       ::sqlite3_sql(mDBStatement)));
 #endif
 
   int srv = ::sqlite3_finalize(mDBStatement);
   mDBStatement = NULL;
 
+  // We need to finalize our async statement too, but want to make sure that any
+  // queued up statements run first.  Dispatch an event to the background thread
+  // that will do this for us.
+  if (mCachedAsyncStatement) {
+    nsCOMPtr<nsIRunnable> event =
+      new AsyncStatementFinalizer(mCachedAsyncStatement);
+    NS_ENSURE_TRUE(event, NS_ERROR_OUT_OF_MEMORY);
+
+    nsCOMPtr<nsIEventTarget> target = mDBConnection->getAsyncExecutionTarget();
+    nsresult rv = target->Dispatch(event, NS_DISPATCH_NORMAL);
+    NS_ENSURE_SUCCESS(rv, rv);
+    mCachedAsyncStatement = NULL;
+  }
+
   // We are considered dead at this point, so any wrappers for row or params
   // need to lose their reference to us.
   if (mStatementParamsHolder) {
     nsCOMPtr<nsIXPConnectWrappedNative> wrapper =
         do_QueryInterface(mStatementParamsHolder);
     nsCOMPtr<mozIStorageStatementParams> iParams =
         do_QueryWrappedNative(wrapper);
     StatementParams *params = static_cast<StatementParams *>(iParams.get());
@@ -410,104 +532,114 @@ Statement::Reset()
 
 #ifdef DEBUG
   PR_LOG(gStorageLog, PR_LOG_DEBUG, ("Resetting statement: '%s'",
                                      ::sqlite3_sql(mDBStatement)));
 
   checkAndLogStatementPerformance(mDBStatement);
 #endif
 
+  mParamsArray = nsnull;
   (void)sqlite3_reset(mDBStatement);
   (void)sqlite3_clear_bindings(mDBStatement);
 
   mExecuting = false;
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 Statement::BindUTF8StringParameter(PRUint32 aParamIndex,
                                    const nsACString &aValue)
 {
   if (!mDBStatement)
     return NS_ERROR_NOT_INITIALIZED;
 
-  int srv = ::sqlite3_bind_text(mDBStatement, aParamIndex + 1,
-                                PromiseFlatCString(aValue).get(),
-                                aValue.Length(), SQLITE_TRANSIENT);
-  return convertResultCode(srv);
+  BindingParams *params = getParams();
+  NS_ENSURE_TRUE(params, NS_ERROR_OUT_OF_MEMORY);
+
+  return params->BindUTF8StringByIndex(aParamIndex, aValue);
 }
 
 NS_IMETHODIMP
 Statement::BindStringParameter(PRUint32 aParamIndex,
                                const nsAString &aValue)
 {
   if (!mDBStatement)
     return NS_ERROR_NOT_INITIALIZED;
 
-  int srv = ::sqlite3_bind_text16(mDBStatement, aParamIndex + 1,
-                                  PromiseFlatString(aValue).get(),
-                                  aValue.Length() * 2, SQLITE_TRANSIENT);
-  return convertResultCode(srv);
+  BindingParams *params = getParams();
+  NS_ENSURE_TRUE(params, NS_ERROR_OUT_OF_MEMORY);
+
+  return params->BindStringByIndex(aParamIndex, aValue);
 }
 
 NS_IMETHODIMP
 Statement::BindDoubleParameter(PRUint32 aParamIndex,
                                double aValue)
 {
   if (!mDBStatement)
     return NS_ERROR_NOT_INITIALIZED;
 
-  int srv = ::sqlite3_bind_double(mDBStatement, aParamIndex + 1, aValue);
-  return convertResultCode(srv);
+  BindingParams *params = getParams();
+  NS_ENSURE_TRUE(params, NS_ERROR_OUT_OF_MEMORY);
+
+  return params->BindDoubleByIndex(aParamIndex, aValue);
 }
 
 NS_IMETHODIMP
 Statement::BindInt32Parameter(PRUint32 aParamIndex,
                               PRInt32 aValue)
 {
   if (!mDBStatement)
     return NS_ERROR_NOT_INITIALIZED;
 
-  int srv = ::sqlite3_bind_int(mDBStatement, aParamIndex + 1, aValue);
-  return convertResultCode(srv);
+  BindingParams *params = getParams();
+  NS_ENSURE_TRUE(params, NS_ERROR_OUT_OF_MEMORY);
+
+  return params->BindInt32ByIndex(aParamIndex, aValue);
 }
 
 NS_IMETHODIMP
 Statement::BindInt64Parameter(PRUint32 aParamIndex,
                               PRInt64 aValue)
 {
   if (!mDBStatement)
     return NS_ERROR_NOT_INITIALIZED;
 
-  int srv = ::sqlite3_bind_int64(mDBStatement, aParamIndex + 1, aValue);
-  return convertResultCode(srv);
+  BindingParams *params = getParams();
+  NS_ENSURE_TRUE(params, NS_ERROR_OUT_OF_MEMORY);
+
+  return params->BindInt64ByIndex(aParamIndex, aValue);
 }
 
 NS_IMETHODIMP
 Statement::BindNullParameter(PRUint32 aParamIndex)
 {
   if (!mDBStatement)
     return NS_ERROR_NOT_INITIALIZED;
 
-  int srv = ::sqlite3_bind_null(mDBStatement, aParamIndex + 1);
-  return convertResultCode(srv);
+  BindingParams *params = getParams();
+  NS_ENSURE_TRUE(params, NS_ERROR_OUT_OF_MEMORY);
+
+  return params->BindNullByIndex(aParamIndex);
 }
 
 NS_IMETHODIMP
 Statement::BindBlobParameter(PRUint32 aParamIndex,
                              const PRUint8 *aValue,
                              PRUint32 aValueSize)
 {
   if (!mDBStatement)
     return NS_ERROR_NOT_INITIALIZED;
 
-  int srv = ::sqlite3_bind_blob(mDBStatement, aParamIndex + 1, aValue,
-                                aValueSize, SQLITE_TRANSIENT);
-  return convertResultCode(srv);
+  BindingParams *params = getParams();
+  NS_ENSURE_TRUE(params, NS_ERROR_OUT_OF_MEMORY);
+
+  return params->BindBlobByIndex(aParamIndex, aValue, aValueSize);
 }
 
 NS_IMETHODIMP
 Statement::BindParameters(mozIStorageBindingParamsArray *aParameters)
 {
   if (!mDBStatement)
     return NS_ERROR_NOT_INITIALIZED;
 
@@ -546,16 +678,35 @@ Statement::Execute()
 }
 
 NS_IMETHODIMP
 Statement::ExecuteStep(PRBool *_moreResults)
 {
   if (!mDBStatement)
     return NS_ERROR_NOT_INITIALIZED;
 
+  // Bind any parameters first before executing.
+  if (mParamsArray) {
+    // If we have more than one row of parameters to bind, they shouldn't be
+    // calling this method (and instead use executeAsync).
+    if (mParamsArray->length() != 1)
+      return NS_ERROR_UNEXPECTED;
+
+    BindingParamsArray::iterator row = mParamsArray->begin();
+    nsCOMPtr<mozIStorageError> error;
+    error = (*row)->bind(mDBStatement);
+    if (error) {
+      PRInt32 srv;
+      (void)error->GetResult(&srv);
+      return convertResultCode(srv);
+    }
+
+    // We have bound, so now we can clear our array.
+    mParamsArray = nsnull;
+  }
   int srv = ::sqlite3_step(mDBStatement);
 
 #ifdef PR_LOGGING
   if (srv != SQLITE_ROW && srv != SQLITE_DONE) {
       nsCAutoString errStr;
       (void)mDBConnection->GetLastErrorString(errStr);
       PR_LOG(gStorageLog, PR_LOG_DEBUG,
              ("Statement::ExecuteStep error: %s", errStr.get()));
--- a/storage/src/mozStorageStatement.h
+++ b/storage/src/mozStorageStatement.h
@@ -41,25 +41,27 @@
 #define _mozStorageStatement_h_
 
 #include "nsAutoPtr.h"
 #include "nsString.h"
 
 #include "nsTArray.h"
 
 #include "mozStorageBindingParamsArray.h"
+#include "mozStorageStatementData.h"
 #include "mozIStorageStatement.h"
 
 class nsIXPConnectJSObjectHolder;
 struct sqlite3_stmt;
 
 namespace mozilla {
 namespace storage {
 class StatementJSHelper;
 class Connection;
+class BindingParams;
 
 class Statement : public mozIStorageStatement
 {
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_MOZISTORAGESTATEMENT
   NS_DECL_MOZISTORAGEVALUEARRAY
 
@@ -87,33 +89,65 @@ public:
    * Obtains and transfers ownership of the array of parameters that are bound
    * to this statment.  This can be null.
    */
   inline already_AddRefed<BindingParamsArray> bindingParamsArray()
   {
     return mParamsArray.forget();
   }
 
+  /**
+   * Obtains the StatementData needed for asynchronous execution.
+   *
+   * @param _data
+   *        A reference to a StatementData object that will be populated upon
+   *        successful execution of this method.
+   * @return an nsresult indicating success or failure.
+   */
+  nsresult getAsynchronousStatementData(StatementData &_data);
+
 private:
     ~Statement();
 
     nsRefPtr<Connection> mDBConnection;
     sqlite3_stmt *mDBStatement;
     PRUint32 mParamCount;
     PRUint32 mResultColumnCount;
     nsTArray<nsCString> mColumnNames;
     bool mExecuting;
 
     /**
+     * @return a pointer to the BindingParams object to use with our Bind*
+     *         method.
+     */
+    BindingParams *getParams();
+
+    /**
      * Holds the array of parameters to bind to this statement when we execute
      * it asynchronously.
      */
     nsRefPtr<BindingParamsArray> mParamsArray;
 
     /**
+     * Holds a copy of mDBStatement that we can use asynchronously.  Access to
+     * this is serialized on the asynchronous thread, so it does not need to be
+     * protected.  We will finalize this statement in our destructor.
+     */
+    sqlite3_stmt *mCachedAsyncStatement;
+
+    /**
+     * Obtains the statement to use on the background thread.
+     *
+     * @param _stmt
+     *        An outparm where the new statement should be placed.
+     * @return a SQLite result code indicating success or failure.
+     */
+    int getAsyncStatement(sqlite3_stmt **_stmt);
+
+    /**
      * The following two members are only used with the JS helper.  They cache
      * the row and params objects.
      */
     nsCOMPtr<nsIXPConnectJSObjectHolder> mStatementParamsHolder;
     nsCOMPtr<nsIXPConnectJSObjectHolder> mStatementRowHolder;
 
     friend class StatementJSHelper;
 };
--- a/storage/src/mozStorageStatementData.h
+++ b/storage/src/mozStorageStatementData.h
@@ -35,72 +35,94 @@
  * 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 ***** */
 
 #ifndef _mozStorageStatementData_h_
 #define _mozStorageStatementData_h_
 
+#include "sqlite3.h"
+
 #include "nsAutoPtr.h"
 #include "nsTArray.h"
 
 #include "mozStorageBindingParamsArray.h"
 
 struct sqlite3_stmt;
 
 namespace mozilla {
 namespace storage {
 
 class StatementData
 {
 public:
   StatementData(sqlite3_stmt *aStatement,
-                already_AddRefed<BindingParamsArray> aParamsArray)
+                already_AddRefed<BindingParamsArray> aParamsArray,
+                nsISupports *aStatementOwner)
   : mStatement(aStatement)
   , mParamsArray(aParamsArray)
+  , mStatementOwner(aStatementOwner)
   {
   }
   StatementData(const StatementData &aSource)
   : mStatement(aSource.mStatement)
   , mParamsArray(aSource.mParamsArray)
+  , mStatementOwner(aSource.mStatementOwner)
   {
   }
   StatementData()
   {
   }
 
   operator sqlite3_stmt *() const
   {
     NS_ASSERTION(mStatement, "NULL sqlite3_stmt being handed off!");
     return mStatement;
   }
   operator BindingParamsArray *() const { return mParamsArray; }
 
   /**
-   * Finalizes and NULLs out our sqlite3_stmt.  Also releases our parameter
-   * array since we'll no longer need it.
+   * NULLs out our sqlite3_stmt (it is held by the owner) after reseting it and
+   * clear all bindings to it.  Then, NULL out the rest of our data.
    */
   inline void finalize()
   {
-    (void)::sqlite3_finalize(mStatement);
+    (void)::sqlite3_reset(mStatement);
+    (void)::sqlite3_clear_bindings(mStatement);
     mStatement = NULL;
     mParamsArray = nsnull;
+    mStatementOwner = nsnull;
   }
 
   /**
    * Indicates if this statement has parameters to be bound before it is
    * executed.
    *
-   * @returns true if the statement has parameters to bind against, false
-   *          otherwise.
+   * @return true if the statement has parameters to bind against, false
+   *         otherwise.
    */
   inline bool hasParametersToBeBound() const { return mParamsArray != nsnull; }
+  /**
+   * Indicates if this statement needs a transaction for execution.
+   *
+   * @return true if the statement needs a transaction, false otherwise.
+   */
+  inline bool needsTransaction() const
+  {
+    return mParamsArray != nsnull && mParamsArray->length() > 1;
+  }
 
 private:
   sqlite3_stmt *mStatement;
   nsRefPtr<BindingParamsArray> mParamsArray;
+
+  /**
+   * We hold onto a reference of the statement's owner so it doesn't get
+   * destroyed out from under us.
+   */
+  nsCOMPtr<nsISupports> mStatementOwner;
 };
 
 } // namespace storage
 } // namespace mozilla
 
 #endif // _mozStorageStatementData_h_
--- a/storage/test/unit/test_statement_executeAsync.js
+++ b/storage/test/unit/test_statement_executeAsync.js
@@ -91,21 +91,21 @@ function test_create_table()
 }
 
 function test_add_data()
 {
   var stmt = getOpenedDatabase().createStatement(
     "INSERT INTO test (id, string, number, nuller, blober) " +
     "VALUES (?, ?, ?, ?, ?)"
   );
-  stmt.bindInt32Parameter(0, INTEGER);
-  stmt.bindStringParameter(1, TEXT);
+  stmt.bindBlobParameter(4, BLOB, BLOB.length);
+  stmt.bindNullParameter(3);
   stmt.bindDoubleParameter(2, REAL);
-  stmt.bindNullParameter(3);
-  stmt.bindBlobParameter(4, BLOB, BLOB.length);
+  stmt.bindStringParameter(1, TEXT);
+  stmt.bindInt32Parameter(0, INTEGER);
 
   stmt.executeAsync({
     handleResult: function(aResultSet)
     {
       do_throw("unexpected results obtained!");
     },
     handleError: function(aError)
     {
--- a/storage/test/unit/test_storage_connection.js
+++ b/storage/test/unit/test_storage_connection.js
@@ -32,16 +32,19 @@
  * 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 ***** */
 
 // This file tests the functions of mozIStorageConnection
 
+////////////////////////////////////////////////////////////////////////////////
+//// Test Functions
+
 function test_connectionReady_open()
 {
   // there doesn't seem to be a way for the connection to not be ready (unless
   // we close it with mozIStorageConnection::Close(), but we don't for this).
   // It can only fail if GetPath fails on the database file, or if we run out
   // of memory trying to use an in-memory database
 
   var msc = getOpenedDatabase();
@@ -206,16 +209,32 @@ function test_defaultSynchronousAtNormal
     do_check_eq(1, stmt.getInt32(0));
   }
   finally {
     stmt.reset();
     stmt.finalize();
   }
 }
 
+function test_close_succeeds_with_finalized_async_statement()
+{
+  // We want to make sure we create a cached async statement to make sure that
+  // when we finalize our statement, we end up finalizing the async one too so
+  // close will succeed.
+  let stmt = createStatement("SELECT * FROM test");
+  stmt.executeAsync();
+  stmt.finalize();
+
+  // Cleanup calls close, as well as removes the database file.
+  cleanup();
+}
+
+////////////////////////////////////////////////////////////////////////////////
+//// Test Runner
+
 var tests = [
   test_connectionReady_open,
   test_connectionReady_closed,
   test_databaseFile,
   test_tableExists_not_created,
   test_indexExists_not_created,
   test_createTable_not_created,
   test_indexExists_created,
@@ -226,16 +245,17 @@ var tests = [
   test_commitTransaction_no_transaction,
   test_rollbackTransaction_no_transaction,
   test_get_schemaVersion_not_set,
   test_set_schemaVersion,
   test_set_schemaVersion_same,
   test_set_schemaVersion_negative,
   test_createTable,
   test_defaultSynchronousAtNormal,
+  test_close_succeeds_with_finalized_async_statement,
 ];
 
 function run_test()
 {
   for (var i = 0; i < tests.length; i++)
     tests[i]();
     
   cleanup();