Bug 1005991 - mozStorage should not use XPCVariant off the main thread. r=asuth, a=lmandel
authorMarco Bonardo <mbonardo@mozilla.com>
Thu, 12 Mar 2015 17:55:56 +0100
changeset 252100 b8c1a399905d
parent 252099 4d306a83ae1b
child 252101 fadc9f270e9f
push id700
push userryanvm@gmail.com
push date2015-03-24 12:29 +0000
treeherdermozilla-release@07c827be741f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth, lmandel
bugs1005991
milestone37.0
Bug 1005991 - mozStorage should not use XPCVariant off the main thread. r=asuth, a=lmandel
storage/src/Variant.h
storage/src/mozStorageAsyncStatement.cpp
storage/src/mozStorageAsyncStatement.h
storage/src/mozStorageAsyncStatementJSHelper.cpp
storage/src/mozStorageAsyncStatementJSHelper.h
storage/src/mozStorageAsyncStatementParams.h
storage/src/mozStorageBindingParams.cpp
storage/src/mozStorageBindingParams.h
storage/src/mozStoragePrivateHelpers.cpp
storage/src/mozStoragePrivateHelpers.h
storage/src/mozStorageStatement.cpp
storage/src/mozStorageStatement.h
storage/src/mozStorageStatementJSHelper.cpp
storage/src/mozStorageStatementJSHelper.h
storage/src/mozStorageStatementParams.h
storage/src/mozStorageStatementRow.h
storage/test/unit/test_statement_executeAsync.js
--- a/storage/src/Variant.h
+++ b/storage/src/Variant.h
@@ -8,16 +8,22 @@
 #define mozilla_storage_Variant_h__
 
 #include <utility>
 
 #include "nsIVariant.h"
 #include "nsString.h"
 #include "nsTArray.h"
 
+#define VARIANT_BASE_IID                                   \
+{ /* 78888042-0fa3-4f7a-8b19-7996f99bf1aa */               \
+  0x78888042, 0x0fa3, 0x4f7a,                              \
+  { 0x8b, 0x19, 0x79, 0x96, 0xf9, 0x9b, 0xf1, 0xaa }       \
+}
+
 /**
  * This class is used by the storage module whenever an nsIVariant needs to be
  * returned.  We provide traits for the basic sqlite types to make use easier.
  * The following types map to the indicated sqlite type:
  * int64_t   -> INTEGER (use IntegerVariant)
  * double    -> FLOAT (use FloatVariant)
  * nsString  -> TEXT (use TextVariant)
  * nsCString -> TEXT (use UTF8TextVariant)
@@ -31,21 +37,25 @@ namespace storage {
 ////////////////////////////////////////////////////////////////////////////////
 //// Base Class
 
 class Variant_base : public nsIVariant
 {
 public:
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSIVARIANT
+  NS_DECLARE_STATIC_IID_ACCESSOR(VARIANT_BASE_IID)
 
 protected:
   virtual ~Variant_base() { }
 };
 
+NS_DEFINE_STATIC_IID_ACCESSOR(Variant_base,
+                              VARIANT_BASE_IID)
+
 ////////////////////////////////////////////////////////////////////////////////
 //// Traits
 
 /**
  * Generics
  */
 
 template <typename DataType>
@@ -361,17 +371,17 @@ public:
     return NS_OK;
   }
 };
 
 ////////////////////////////////////////////////////////////////////////////////
 //// Template Implementation
 
 template <typename DataType, bool Adopting=false>
-class Variant : public Variant_base
+class Variant MOZ_FINAL : public Variant_base
 {
   ~Variant()
   {
     variant_storage_traits<DataType, Adopting>::destroy(mData);
   }
 
 public:
   explicit Variant(const typename variant_storage_traits<DataType, Adopting>::ConstructorType aData)
--- a/storage/src/mozStorageAsyncStatement.cpp
+++ b/storage/src/mozStorageAsyncStatement.cpp
@@ -222,49 +222,31 @@ AsyncStatement::getParams()
  * so it is always safe to destroy our sqlite3_stmt if one exists.  We can be
  * destroyed on the caller thread by garbage-collection/reference counting or on
  * the async thread by the last execution of a statement that already lost its
  * main-thread refs.
  */
 AsyncStatement::~AsyncStatement()
 {
   destructorAsyncFinalize();
-  cleanupJSHelpers();
 
   // If we are getting destroyed on the wrong thread, proxy the connection
   // release to the right thread.  I'm not sure why we do this.
   bool onCallingThread = false;
   (void)mDBConnection->threadOpenedOn->IsOnCurrentThread(&onCallingThread);
   if (!onCallingThread) {
     // NS_ProxyRelase only magic forgets for us if mDBConnection is an
     // nsCOMPtr.  Which it is not; it's an nsRefPtr.
     Connection *forgottenConn = nullptr;
     mDBConnection.swap(forgottenConn);
     (void)::NS_ProxyRelease(forgottenConn->threadOpenedOn,
                             static_cast<mozIStorageConnection *>(forgottenConn));
   }
 }
 
-void
-AsyncStatement::cleanupJSHelpers()
-{
-  // 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);
-    AsyncStatementParams *params =
-      static_cast<AsyncStatementParams *>(iParams.get());
-    params->mStatement = nullptr;
-    mStatementParamsHolder = nullptr;
-  }
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 //// nsISupports
 
 NS_IMPL_ADDREF(AsyncStatement)
 NS_IMPL_RELEASE(AsyncStatement)
 
 NS_INTERFACE_MAP_BEGIN(AsyncStatement)
   NS_INTERFACE_MAP_ENTRY(mozIStorageAsyncStatement)
@@ -364,17 +346,19 @@ AsyncStatement::Finalize()
     return NS_OK;
 
   mFinalized = true;
 
   PR_LOG(gStorageLog, PR_LOG_NOTICE, ("Finalizing statement '%s'",
                                       mSQLString.get()));
 
   asyncFinalize();
-  cleanupJSHelpers();
+
+  // Release the params holder, so it can release the reference to us.
+  mStatementParamsHolder = nullptr;
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 AsyncStatement::BindParameters(mozIStorageBindingParamsArray *aParameters)
 {
   if (mFinalized)
--- a/storage/src/mozStorageAsyncStatement.h
+++ b/storage/src/mozStorageAsyncStatement.h
@@ -63,23 +63,16 @@ public:
     return mParamsArray.forget();
   }
 
 
 private:
   ~AsyncStatement();
 
   /**
-   * Clean up the references JS helpers hold to us.  For cycle-avoidance reasons
-   * they do not hold reference-counted references to us, so it is important
-   * we do this.
-   */
-  void cleanupJSHelpers();
-
-  /**
    * @return a pointer to the BindingParams object to use with our Bind*
    *         method.
    */
   mozIStorageBindingParams *getParams();
 
   /**
    * The SQL string as passed by the user.  We store it because we create the
    * async statement on-demand on the async thread.
@@ -90,17 +83,17 @@ private:
    * Holds the array of parameters to bind to this statement when we execute
    * it asynchronously.
    */
   nsRefPtr<BindingParamsArray> mParamsArray;
 
   /**
    * Caches the JS 'params' helper for this statement.
    */
-  nsCOMPtr<nsIXPConnectJSObjectHolder> mStatementParamsHolder;
+  nsMainThreadPtrHandle<nsIXPConnectJSObjectHolder> mStatementParamsHolder;
 
   /**
    * Have we been explicitly finalized by the user?
    */
   bool mFinalized;
 
   /**
    * Required for access to private mStatementParamsHolder field by
--- a/storage/src/mozStorageAsyncStatementJSHelper.cpp
+++ b/storage/src/mozStorageAsyncStatementJSHelper.cpp
@@ -25,40 +25,46 @@ namespace storage {
 //// AsyncStatementJSHelper
 
 nsresult
 AsyncStatementJSHelper::getParams(AsyncStatement *aStatement,
                                   JSContext *aCtx,
                                   JSObject *aScopeObj,
                                   jsval *_params)
 {
+  MOZ_ASSERT(NS_IsMainThread());
   nsresult rv;
 
 #ifdef DEBUG
   int32_t state;
   (void)aStatement->GetState(&state);
   NS_ASSERTION(state == mozIStorageAsyncStatement::MOZ_STORAGE_STATEMENT_READY,
                "Invalid state to get the params object - all calls will fail!");
 #endif
 
   if (!aStatement->mStatementParamsHolder) {
     nsCOMPtr<mozIStorageStatementParams> params =
       new AsyncStatementParams(aStatement);
     NS_ENSURE_TRUE(params, NS_ERROR_OUT_OF_MEMORY);
 
     JS::RootedObject scope(aCtx, aScopeObj);
+    nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
     nsCOMPtr<nsIXPConnect> xpc(Service::getXPConnect());
     rv = xpc->WrapNative(
       aCtx,
       ::JS_GetGlobalForObject(aCtx, scope),
       params,
       NS_GET_IID(mozIStorageStatementParams),
-      getter_AddRefs(aStatement->mStatementParamsHolder)
+      getter_AddRefs(holder)
     );
     NS_ENSURE_SUCCESS(rv, rv);
+    nsRefPtr<AsyncStatementParamsHolder> paramsHolder =
+      new AsyncStatementParamsHolder(holder);
+    aStatement->mStatementParamsHolder =
+      new nsMainThreadPtrHolder<nsIXPConnectJSObjectHolder>(paramsHolder);
   }
 
   JS::Rooted<JSObject*> obj(aCtx);
   obj = aStatement->mStatementParamsHolder->GetJSObject();
   NS_ENSURE_STATE(obj);
 
   *_params = OBJECT_TO_JSVAL(obj);
   return NS_OK;
@@ -107,10 +113,39 @@ AsyncStatementJSHelper::GetProperty(nsIX
 #endif
 
   if (::JS_FlatStringEqualsAscii(JSID_TO_FLAT_STRING(id), "params"))
     return getParams(stmt, aCtx, scope, _result);
 
   return NS_OK;
 }
 
+////////////////////////////////////////////////////////////////////////////////
+//// AsyncStatementParamsHolder
+
+NS_IMPL_ISUPPORTS(AsyncStatementParamsHolder, nsIXPConnectJSObjectHolder);
+
+JSObject*
+AsyncStatementParamsHolder::GetJSObject()
+{
+  return mHolder->GetJSObject();
+}
+
+AsyncStatementParamsHolder::AsyncStatementParamsHolder(nsIXPConnectJSObjectHolder* aHolder)
+  : mHolder(aHolder)
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  MOZ_ASSERT(mHolder);
+}
+
+AsyncStatementParamsHolder::~AsyncStatementParamsHolder()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  // We are considered dead at this point, so any wrappers for row or params
+  // need to lose their reference to the statement.
+  nsCOMPtr<nsIXPConnectWrappedNative> wrapper = do_QueryInterface(mHolder);
+  nsCOMPtr<mozIStorageStatementParams> iObj = do_QueryWrappedNative(wrapper);
+  AsyncStatementParams *obj = static_cast<AsyncStatementParams *>(iObj.get());
+  obj->mStatement = nullptr;
+}
+
 } // namespace storage
 } // namespace mozilla
--- a/storage/src/mozStorageAsyncStatementJSHelper.h
+++ b/storage/src/mozStorageAsyncStatementJSHelper.h
@@ -3,16 +3,17 @@
  * 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/. */
 
 #ifndef mozilla_storage_mozStorageAsyncStatementJSHelper_h_
 #define mozilla_storage_mozStorageAsyncStatementJSHelper_h_
 
 #include "nsIXPCScriptable.h"
+#include "nsIXPConnect.h"
 
 class AsyncStatement;
 
 namespace mozilla {
 namespace storage {
 
 /**
  * A modified version of StatementJSHelper that only exposes the async-specific
@@ -24,12 +25,30 @@ class AsyncStatementJSHelper : public ns
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIXPCSCRIPTABLE
 
 private:
   nsresult getParams(AsyncStatement *, JSContext *, JSObject *, JS::Value *);
 };
 
+/**
+ * Wrapper used to clean up the references JS helpers hold to the statement.
+ * For cycle-avoidance reasons they do not hold reference-counted references,
+ * so it is important we do this.
+ */
+class AsyncStatementParamsHolder MOZ_FINAL : public nsIXPConnectJSObjectHolder
+{
+public:
+  NS_DECL_ISUPPORTS
+  NS_DECL_NSIXPCONNECTJSOBJECTHOLDER
+
+  explicit AsyncStatementParamsHolder(nsIXPConnectJSObjectHolder* aHolder);
+
+private:
+  virtual ~AsyncStatementParamsHolder();
+  nsCOMPtr<nsIXPConnectJSObjectHolder> mHolder;
+};
+
 } // namespace storage
 } // namespace mozilla
 
 #endif // mozilla_storage_mozStorageAsyncStatementJSHelper_h_
--- a/storage/src/mozStorageAsyncStatementParams.h
+++ b/storage/src/mozStorageAsyncStatementParams.h
@@ -11,18 +11,16 @@
 #include "nsIXPCScriptable.h"
 #include "mozilla/Attributes.h"
 
 class mozIStorageAsyncStatement;
 
 namespace mozilla {
 namespace storage {
 
-class AsyncStatement;
-
 /*
  * Since mozIStorageStatementParams is just a tagging interface we do not have
  * an async variant.
  */
 class AsyncStatementParams MOZ_FINAL : public mozIStorageStatementParams
                                      , public nsIXPCScriptable
 {
 public:
@@ -33,15 +31,15 @@ public:
   NS_DECL_MOZISTORAGESTATEMENTPARAMS
   NS_DECL_NSIXPCSCRIPTABLE
 
 protected:
   virtual ~AsyncStatementParams() {}
 
   AsyncStatement *mStatement;
 
-  friend class AsyncStatement;
+  friend class AsyncStatementParamsHolder;
 };
 
 } // namespace storage
 } // namespace mozilla
 
 #endif // mozilla_storage_mozStorageAsyncStatementParams_h_
--- a/storage/src/mozStorageBindingParams.cpp
+++ b/storage/src/mozStorageBindingParams.cpp
@@ -210,17 +210,17 @@ NS_IMPL_ISUPPORTS(
 
 ////////////////////////////////////////////////////////////////////////////////
 //// IStorageBindingParamsInternal
 
 already_AddRefed<mozIStorageError>
 BindingParams::bind(sqlite3_stmt *aStatement)
 {
   // Iterate through all of our stored data, and bind it.
-  for (int32_t i = 0; i < mParameters.Count(); i++) {
+  for (size_t i = 0; i < mParameters.Length(); i++) {
     int rc = variantToSQLiteT(BindingColumnData(aStatement, i), mParameters[i]);
     if (rc != SQLITE_OK) {
       // We had an error while trying to bind.  Now we need to create an error
       // object with the right message.  Note that we special case
       // SQLITE_MISMATCH, but otherwise get the message from SQLite.
       const char *msg = "Could not covert nsIVariant to SQLite type.";
       if (rc != SQLITE_MISMATCH)
         msg = ::sqlite3_errmsg(::sqlite3_db_handle(aStatement));
@@ -269,17 +269,21 @@ BindingParams::BindByName(const nsACStri
 }
 
 NS_IMETHODIMP
 AsyncBindingParams::BindByName(const nsACString &aName,
                                nsIVariant *aValue)
 {
   NS_ENSURE_FALSE(mLocked, NS_ERROR_UNEXPECTED);
 
-  mNamedParameters.Put(aName, aValue);
+  nsRefPtr<Variant_base> variant = convertVariantToStorageVariant(aValue);
+  if (!variant)
+    return NS_ERROR_UNEXPECTED;
+
+  mNamedParameters.Put(aName, variant);
   return NS_OK;
 }
 
 
 NS_IMETHODIMP
 BindingParams::BindUTF8StringByName(const nsACString &aName,
                                     const nsACString &aValue)
 {
@@ -373,32 +377,49 @@ BindingParams::BindAdoptedBlobByName(con
 NS_IMETHODIMP
 BindingParams::BindByIndex(uint32_t 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.ReplaceObjectAt(aValue, aIndex),
-                 NS_ERROR_OUT_OF_MEMORY);
+  nsRefPtr<Variant_base> variant = convertVariantToStorageVariant(aValue);
+  if (!variant)
+    return NS_ERROR_UNEXPECTED;
+  if (mParameters.Length() <= aIndex) {
+    (void)mParameters.SetLength(aIndex);
+    (void)mParameters.AppendElement(variant);
+  }
+  else {
+    NS_ENSURE_TRUE(mParameters.ReplaceElementAt(aIndex, variant),
+                   NS_ERROR_OUT_OF_MEMORY);
+  }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 AsyncBindingParams::BindByIndex(uint32_t aIndex,
                                 nsIVariant *aValue)
 {
   NS_ENSURE_FALSE(mLocked, NS_ERROR_UNEXPECTED);
   // In the asynchronous case we do not know how many parameters there are to
   // bind to, so we cannot check the validity of aIndex.
 
-  // Store the variant for later use.
-  NS_ENSURE_TRUE(mParameters.ReplaceObjectAt(aValue, aIndex),
-                 NS_ERROR_OUT_OF_MEMORY);
+  nsRefPtr<Variant_base> variant = convertVariantToStorageVariant(aValue);
+  if (!variant)
+    return NS_ERROR_UNEXPECTED;
+  if (mParameters.Length() <= aIndex) {
+    mParameters.SetLength(aIndex);
+    mParameters.AppendElement(variant);
+  }
+  else {
+    NS_ENSURE_TRUE(mParameters.ReplaceElementAt(aIndex, variant),
+                   NS_ERROR_OUT_OF_MEMORY);
+  }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BindingParams::BindUTF8StringByIndex(uint32_t aIndex,
                                      const nsACString &aValue)
 {
   nsCOMPtr<nsIVariant> value(new UTF8TextVariant(aValue));
--- a/storage/src/mozStorageBindingParams.h
+++ b/storage/src/mozStorageBindingParams.h
@@ -9,16 +9,17 @@
 
 #include "nsCOMArray.h"
 #include "nsIVariant.h"
 #include "nsInterfaceHashtable.h"
 
 #include "mozStorageBindingParamsArray.h"
 #include "mozStorageStatement.h"
 #include "mozStorageAsyncStatement.h"
+#include "Variant.h"
 
 #include "mozIStorageBindingParams.h"
 #include "IStorageBindingParamsInternal.h"
 
 namespace mozilla {
 namespace storage {
 
 class BindingParams : public mozIStorageBindingParams
@@ -52,17 +53,19 @@ public:
   const mozIStorageBindingParamsArray *getOwner() const;
 
   BindingParams(mozIStorageBindingParamsArray *aOwningArray,
                 Statement *aOwningStatement);
 protected:
   virtual ~BindingParams() {}
 
   explicit BindingParams(mozIStorageBindingParamsArray *aOwningArray);
-  nsCOMArray<nsIVariant> mParameters;
+  // Note that this is managed as a sparse array, so particular caution should
+  // be used for out-of-bounds usage.
+  nsTArray<nsRefPtr<Variant_base> > mParameters;
   bool mLocked;
 
 private:
 
   /**
    * Track the BindingParamsArray that created us until we are added to it.
    * (Once we are added we are locked and no one needs to look up our owner.)
    * Ref-counted since there is no invariant that guarantees it stays alive
--- a/storage/src/mozStoragePrivateHelpers.cpp
+++ b/storage/src/mozStoragePrivateHelpers.cpp
@@ -149,16 +149,105 @@ convertJSValToVariant(
     int64_t msec = msecd;
 
     return new IntegerVariant(msec);
   }
 
   return nullptr;
 }
 
+Variant_base *
+convertVariantToStorageVariant(nsIVariant* aVariant)
+{
+  nsRefPtr<Variant_base> variant = do_QueryObject(aVariant);
+  if (variant) {
+    // JS helpers already convert the JS representation to a Storage Variant,
+    // in such a case there's nothing left to do here, so just pass-through.
+    return variant;
+  }
+
+  if (!aVariant)
+    return new NullVariant();
+
+  uint16_t dataType;
+  nsresult rv = aVariant->GetDataType(&dataType);
+  NS_ENSURE_SUCCESS(rv, nullptr);
+
+  switch (dataType) {
+    case nsIDataType::VTYPE_BOOL:
+    case nsIDataType::VTYPE_INT8:
+    case nsIDataType::VTYPE_INT16:
+    case nsIDataType::VTYPE_INT32:
+    case nsIDataType::VTYPE_UINT8:
+    case nsIDataType::VTYPE_UINT16:
+    case nsIDataType::VTYPE_UINT32:
+    case nsIDataType::VTYPE_INT64:
+    case nsIDataType::VTYPE_UINT64: {
+      int64_t v;
+      rv = aVariant->GetAsInt64(&v);
+      NS_ENSURE_SUCCESS(rv, nullptr);
+      return new IntegerVariant(v);
+    }
+    case nsIDataType::VTYPE_FLOAT:
+    case nsIDataType::VTYPE_DOUBLE: {
+      double v;
+      rv = aVariant->GetAsDouble(&v);
+      NS_ENSURE_SUCCESS(rv, nullptr);
+      return new FloatVariant(v);
+    }
+    case nsIDataType::VTYPE_CHAR:
+    case nsIDataType::VTYPE_CHAR_STR:
+    case nsIDataType::VTYPE_STRING_SIZE_IS:
+    case nsIDataType::VTYPE_UTF8STRING:
+    case nsIDataType::VTYPE_CSTRING: {
+      nsCString v;
+      rv = aVariant->GetAsAUTF8String(v);
+      NS_ENSURE_SUCCESS(rv, nullptr);
+      return new UTF8TextVariant(v);
+    }
+    case nsIDataType::VTYPE_WCHAR:
+    case nsIDataType::VTYPE_DOMSTRING:
+    case nsIDataType::VTYPE_WCHAR_STR:
+    case nsIDataType::VTYPE_WSTRING_SIZE_IS:
+    case nsIDataType::VTYPE_ASTRING: {
+      nsString v;
+      rv = aVariant->GetAsAString(v);
+      NS_ENSURE_SUCCESS(rv, nullptr);
+      return new TextVariant(v);
+    }
+    case nsIDataType::VTYPE_ARRAY: {
+      uint16_t type;
+      nsIID iid;
+      uint32_t len;
+      void *rawArray;
+      // Note this copies the array data.
+      rv = aVariant->GetAsArray(&type, &iid, &len, &rawArray);
+      NS_ENSURE_SUCCESS(rv, nullptr);
+      if (type == nsIDataType::VTYPE_UINT8) {
+        std::pair<uint8_t *, int> v(static_cast<uint8_t *>(rawArray), len);
+        // Take ownership of the data avoiding a further copy.
+        return new AdoptedBlobVariant(v);
+      }
+    }
+    case nsIDataType::VTYPE_EMPTY:
+    case nsIDataType::VTYPE_EMPTY_ARRAY:
+    case nsIDataType::VTYPE_VOID:
+      return new NullVariant();
+    case nsIDataType::VTYPE_ID:
+    case nsIDataType::VTYPE_INTERFACE:
+    case nsIDataType::VTYPE_INTERFACE_IS:
+    default:
+      NS_WARNING("Unsupported variant type");
+      return nullptr;
+  }
+
+  return nullptr;
+
+}
+
 namespace {
 class CallbackEvent : public nsRunnable
 {
 public:
   explicit CallbackEvent(mozIStorageCompletionCallback *aCallback)
   : mCallback(aCallback)
   {
   }
--- a/storage/src/mozStoragePrivateHelpers.h
+++ b/storage/src/mozStoragePrivateHelpers.h
@@ -11,16 +11,17 @@
  * This file contains convenience methods for mozStorage.
  */
 
 #include "sqlite3.h"
 #include "nsIVariant.h"
 #include "nsError.h"
 #include "nsAutoPtr.h"
 #include "js/TypeDecls.h"
+#include "Variant.h"
 
 class mozIStorageCompletionCallback;
 class mozIStorageBaseStatement;
 class mozIStorageBindingParams;
 class nsIRunnable;
 
 namespace mozilla {
 namespace storage {
@@ -64,16 +65,26 @@ void checkAndLogStatementPerformance(sql
  *        but only Date objects are supported from the Date family.  Date
  *        objects are coerced to PRTime (nanoseconds since epoch) values.
  * @return the variant if conversion was successful, nullptr if conversion
  *         failed.  The caller is responsible for addref'ing if non-null.
  */
 nsIVariant *convertJSValToVariant(JSContext *aCtx, JS::Value aValue);
 
 /**
+ * Convert a provided nsIVariant implementation to our own thread-safe
+ * refcounting implementation, if needed.
+ *
+ * @param aValue
+ *        The original nsIVariant to be converted.
+ * @return a thread-safe refcounting nsIVariant implementation.
+ */
+Variant_base *convertVariantToStorageVariant(nsIVariant *aVariant);
+
+/**
  * Obtains an event that will notify a completion callback about completion.
  *
  * @param aCallback
  *        The callback to be notified.
  * @return an nsIRunnable that can be dispatched to the calling thread.
  */
 already_AddRefed<nsIRunnable> newCompletionEvent(
   mozIStorageCompletionCallback *aCallback
--- a/storage/src/mozStorageStatement.cpp
+++ b/storage/src/mozStorageStatement.cpp
@@ -426,37 +426,19 @@ Statement::internalFinalize(bool aDestru
     // If the destructor called us, there are no pending async statements (they
     // hold a reference to us) and we can/must just kill the statement directly.
     if (aDestructing)
       destructorAsyncFinalize();
     else
       asyncFinalize();
   }
 
-  // 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());
-    params->mStatement = nullptr;
-    mStatementParamsHolder = nullptr;
-  }
-
-  if (mStatementRowHolder) {
-    nsCOMPtr<nsIXPConnectWrappedNative> wrapper =
-        do_QueryInterface(mStatementRowHolder);
-    nsCOMPtr<mozIStorageStatementRow> iRow =
-        do_QueryWrappedNative(wrapper);
-    StatementRow *row = static_cast<StatementRow *>(iRow.get());
-    row->mStatement = nullptr;
-    mStatementRowHolder = nullptr;
-  }
+  // Release the holders, so they can release the reference to us.
+  mStatementParamsHolder = nullptr;
+  mStatementRowHolder = nullptr;
 
   return convertResultCode(srv);
 }
 
 NS_IMETHODIMP
 Statement::GetParameterCount(uint32_t *_parameterCount)
 {
   if (!mDBStatement)
--- a/storage/src/mozStorageStatement.h
+++ b/storage/src/mozStorageStatement.h
@@ -91,28 +91,28 @@ private:
      * it asynchronously.
      */
     nsRefPtr<BindingParamsArray> mParamsArray;
 
     /**
      * The following two members are only used with the JS helper.  They cache
      * the row and params objects.
      */
-    nsCOMPtr<nsIXPConnectJSObjectHolder> mStatementParamsHolder;
-    nsCOMPtr<nsIXPConnectJSObjectHolder> mStatementRowHolder;
+    nsMainThreadPtrHandle<nsIXPConnectJSObjectHolder> mStatementParamsHolder;
+    nsMainThreadPtrHandle<nsIXPConnectJSObjectHolder> mStatementRowHolder;
 
   /**
    * Internal version of finalize that allows us to tell it if it is being
    * called from the destructor so it can know not to dispatch events that
    * require a reference to us.
    *
    * @param aDestructing
    *        Is the destructor calling?
    */
   nsresult internalFinalize(bool aDestructing);
 
-    friend class StatementJSHelper;
+  friend class StatementJSHelper;
 };
 
 } // storage
 } // mozilla
 
 #endif // mozStorageStatement_h
--- a/storage/src/mozStorageStatementJSHelper.cpp
+++ b/storage/src/mozStorageStatementJSHelper.cpp
@@ -79,79 +79,90 @@ stepFunc(JSContext *aCtx,
 //// StatementJSHelper
 
 nsresult
 StatementJSHelper::getRow(Statement *aStatement,
                           JSContext *aCtx,
                           JSObject *aScopeObj,
                           jsval *_row)
 {
+  MOZ_ASSERT(NS_IsMainThread());
   nsresult rv;
 
 #ifdef DEBUG
   int32_t state;
   (void)aStatement->GetState(&state);
   NS_ASSERTION(state == mozIStorageStatement::MOZ_STORAGE_STATEMENT_EXECUTING,
                "Invalid state to get the row object - all calls will fail!");
 #endif
 
   if (!aStatement->mStatementRowHolder) {
     JS::RootedObject scope(aCtx, aScopeObj);
     nsCOMPtr<mozIStorageStatementRow> row(new StatementRow(aStatement));
     NS_ENSURE_TRUE(row, NS_ERROR_OUT_OF_MEMORY);
 
+    nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
     nsCOMPtr<nsIXPConnect> xpc(Service::getXPConnect());
     rv = xpc->WrapNative(
       aCtx,
       ::JS_GetGlobalForObject(aCtx, scope),
       row,
       NS_GET_IID(mozIStorageStatementRow),
-      getter_AddRefs(aStatement->mStatementRowHolder)
+      getter_AddRefs(holder)
     );
     NS_ENSURE_SUCCESS(rv, rv);
+    nsRefPtr<StatementRowHolder> rowHolder = new StatementRowHolder(holder);
+    aStatement->mStatementRowHolder =
+      new nsMainThreadPtrHolder<nsIXPConnectJSObjectHolder>(rowHolder);
   }
 
   JS::Rooted<JSObject*> obj(aCtx);
   obj = aStatement->mStatementRowHolder->GetJSObject();
   NS_ENSURE_STATE(obj);
 
   *_row = OBJECT_TO_JSVAL(obj);
   return NS_OK;
 }
 
 nsresult
 StatementJSHelper::getParams(Statement *aStatement,
                              JSContext *aCtx,
                              JSObject *aScopeObj,
                              jsval *_params)
 {
+  MOZ_ASSERT(NS_IsMainThread());
   nsresult rv;
 
 #ifdef DEBUG
   int32_t state;
   (void)aStatement->GetState(&state);
   NS_ASSERTION(state == mozIStorageStatement::MOZ_STORAGE_STATEMENT_READY,
                "Invalid state to get the params object - all calls will fail!");
 #endif
 
   if (!aStatement->mStatementParamsHolder) {
     JS::RootedObject scope(aCtx, aScopeObj);
     nsCOMPtr<mozIStorageStatementParams> params =
       new StatementParams(aStatement);
     NS_ENSURE_TRUE(params, NS_ERROR_OUT_OF_MEMORY);
 
+    nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
     nsCOMPtr<nsIXPConnect> xpc(Service::getXPConnect());
     rv = xpc->WrapNative(
       aCtx,
       ::JS_GetGlobalForObject(aCtx, scope),
       params,
       NS_GET_IID(mozIStorageStatementParams),
-      getter_AddRefs(aStatement->mStatementParamsHolder)
+      getter_AddRefs(holder)
     );
     NS_ENSURE_SUCCESS(rv, rv);
+    nsRefPtr<StatementParamsHolder> paramsHolder =
+      new StatementParamsHolder(holder);
+    aStatement->mStatementParamsHolder =
+      new nsMainThreadPtrHolder<nsIXPConnectJSObjectHolder>(paramsHolder);
   }
 
   JS::Rooted<JSObject*> obj(aCtx);
   obj = aStatement->mStatementParamsHolder->GetJSObject();
   NS_ENSURE_STATE(obj);
 
   *_params = OBJECT_TO_JSVAL(obj);
   return NS_OK;
@@ -225,10 +236,50 @@ StatementJSHelper::Resolve(nsIXPConnectW
     *_retval = ::JS_DefineFunction(aCtx, scope, "step", stepFunc,
                                    0, 0) != nullptr;
     *aResolvedp = true;
     return NS_OK;
   }
   return NS_OK;
 }
 
+////////////////////////////////////////////////////////////////////////////////
+//// StatementJSObjectHolder
+
+NS_IMPL_ISUPPORTS(StatementJSObjectHolder, nsIXPConnectJSObjectHolder);
+
+JSObject*
+StatementJSObjectHolder::GetJSObject()
+{
+  return mHolder->GetJSObject();
+}
+
+StatementJSObjectHolder::StatementJSObjectHolder(nsIXPConnectJSObjectHolder* aHolder)
+  : mHolder(aHolder)
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  MOZ_ASSERT(mHolder);
+}
+
+StatementParamsHolder::~StatementParamsHolder()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  // We are considered dead at this point, so any wrappers for row or params
+  // need to lose their reference to the statement.
+  nsCOMPtr<nsIXPConnectWrappedNative> wrapper = do_QueryInterface(mHolder);
+  nsCOMPtr<mozIStorageStatementParams> iObj = do_QueryWrappedNative(wrapper);
+  StatementParams *obj = static_cast<StatementParams *>(iObj.get());
+  obj->mStatement = nullptr;
+}
+
+StatementRowHolder::~StatementRowHolder()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  // We are considered dead at this point, so any wrappers for row or params
+  // need to lose their reference to the statement.
+  nsCOMPtr<nsIXPConnectWrappedNative> wrapper = do_QueryInterface(mHolder);
+  nsCOMPtr<mozIStorageStatementRow> iObj = do_QueryWrappedNative(wrapper);
+  StatementRow *obj = static_cast<StatementRow *>(iObj.get());
+  obj->mStatement = nullptr;
+}
+
 } // namespace storage
 } // namespace mozilla
--- a/storage/src/mozStorageStatementJSHelper.h
+++ b/storage/src/mozStorageStatementJSHelper.h
@@ -20,12 +20,50 @@ public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIXPCSCRIPTABLE
 
 private:
   nsresult getRow(Statement *, JSContext *, JSObject *, JS::Value *);
   nsresult getParams(Statement *, JSContext *, JSObject *, JS::Value *);
 };
 
+/**
+ * Wrappers used to clean up the references JS helpers hold to the statement.
+ * For cycle-avoidance reasons they do not hold reference-counted references,
+ * so it is important we do this.
+ */
+class StatementJSObjectHolder : public nsIXPConnectJSObjectHolder
+{
+public:
+  NS_DECL_ISUPPORTS
+  NS_DECL_NSIXPCONNECTJSOBJECTHOLDER
+
+  explicit StatementJSObjectHolder(nsIXPConnectJSObjectHolder* aHolder);
+
+protected:
+  virtual ~StatementJSObjectHolder() {};
+  nsCOMPtr<nsIXPConnectJSObjectHolder> mHolder;
+};
+
+class StatementParamsHolder MOZ_FINAL: public StatementJSObjectHolder {
+public:
+  explicit StatementParamsHolder(nsIXPConnectJSObjectHolder* aHolder)
+    : StatementJSObjectHolder(aHolder) {
+  }
+
+private:
+  virtual ~StatementParamsHolder();
+};
+
+class StatementRowHolder MOZ_FINAL: public StatementJSObjectHolder {
+public:
+  explicit StatementRowHolder(nsIXPConnectJSObjectHolder* aHolder)
+    : StatementJSObjectHolder(aHolder) {
+  }
+
+private:
+  virtual ~StatementRowHolder();
+};
+
 } // namespace storage
 } // namespace mozilla
 
 #endif // MOZSTORAGESTATEMENTJSHELPER_H
--- a/storage/src/mozStorageStatementParams.h
+++ b/storage/src/mozStorageStatementParams.h
@@ -11,18 +11,16 @@
 #include "nsIXPCScriptable.h"
 #include "mozilla/Attributes.h"
 
 class mozIStorageStatement;
 
 namespace mozilla {
 namespace storage {
 
-class Statement;
-
 class StatementParams MOZ_FINAL : public mozIStorageStatementParams
                                 , public nsIXPCScriptable
 {
 public:
   explicit StatementParams(mozIStorageStatement *aStatement);
 
   // interfaces
   NS_DECL_ISUPPORTS
@@ -30,15 +28,16 @@ public:
   NS_DECL_NSIXPCSCRIPTABLE
 
 protected:
   ~StatementParams() {}
 
   mozIStorageStatement *mStatement;
   uint32_t mParamCount;
 
-  friend class Statement;
+  friend class StatementParamsHolder;
+  friend class StatementRowHolder;
 };
 
 } // namespace storage
 } // namespace mozilla
 
 #endif /* MOZSTORAGESTATEMENTPARAMS_H */
--- a/storage/src/mozStorageStatementRow.h
+++ b/storage/src/mozStorageStatementRow.h
@@ -9,32 +9,30 @@
 
 #include "mozIStorageStatementRow.h"
 #include "nsIXPCScriptable.h"
 #include "mozilla/Attributes.h"
 
 namespace mozilla {
 namespace storage {
 
-class Statement;
-
 class StatementRow MOZ_FINAL : public mozIStorageStatementRow
                              , public nsIXPCScriptable
 {
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_MOZISTORAGESTATEMENTROW
   NS_DECL_NSIXPCSCRIPTABLE
 
   explicit StatementRow(Statement *aStatement);
 protected:
 
   ~StatementRow() {}
 
   Statement *mStatement;
 
-  friend class Statement;
+  friend class StatementRowHolder;
 };
 
 } // namespace storage
 } // namespace mozilla
 
 #endif /* MOZSTORAGESTATEMENTROW_H */
--- a/storage/test/unit/test_statement_executeAsync.js
+++ b/storage/test/unit/test_statement_executeAsync.js
@@ -710,43 +710,33 @@ function test_bind_bogus_type_by_index()
   // We try to bind a JS Object here that should fail to bind.
   let stmt = makeTestStatement(
     "INSERT INTO test (blober) " +
     "VALUES (?)"
   );
 
   let array = stmt.newBindingParamsArray();
   let bp = array.newBindingParams();
-  // We get an error after calling executeAsync, not when we bind.
-  bp.bindByIndex(0, run_test);
-  array.addParams(bp);
-  stmt.bindParameters(array);
-
-  execAsync(stmt, {error: Ci.mozIStorageError.MISMATCH});
+  Assert.throws(() => bp.bindByIndex(0, run_test), /NS_ERROR_UNEXPECTED/);
 
   stmt.finalize();
   run_next_test();
 }
 
 function test_bind_bogus_type_by_name()
 {
   // We try to bind a JS Object here that should fail to bind.
   let stmt = makeTestStatement(
     "INSERT INTO test (blober) " +
     "VALUES (:blob)"
   );
 
   let array = stmt.newBindingParamsArray();
   let bp = array.newBindingParams();
-  // We get an error after calling executeAsync, not when we bind.
-  bp.bindByName("blob", run_test);
-  array.addParams(bp);
-  stmt.bindParameters(array);
-
-  execAsync(stmt, {error: Ci.mozIStorageError.MISMATCH});
+  Assert.throws(() => bp.bindByName("blob", run_test), /NS_ERROR_UNEXPECTED/);
 
   stmt.finalize();
   run_next_test();
 }
 
 function test_bind_params_already_locked()
 {
   let stmt = makeTestStatement(