Bug 696362: OpenDatabaseHelper can release objects on the wrong thread. r=bent
authorKyle Huey <khuey@kylehuey.com>
Sat, 22 Oct 2011 07:56:19 -0400
changeset 79083 475896b92d6c189a53fbf680995b32dac8cc9513
parent 79082 e79245e249c486bf7ebb3748846b4e2872a56b16
child 79103 7ba4cea5382dda70acbc94ac91013958d8dfaad7
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersbent
bugs696362
milestone10.0a1
Bug 696362: OpenDatabaseHelper can release objects on the wrong thread. r=bent
dom/indexedDB/AsyncConnectionHelper.cpp
dom/indexedDB/AsyncConnectionHelper.h
dom/indexedDB/OpenDatabaseHelper.cpp
dom/indexedDB/OpenDatabaseHelper.h
--- a/dom/indexedDB/AsyncConnectionHelper.cpp
+++ b/dom/indexedDB/AsyncConnectionHelper.cpp
@@ -115,16 +115,34 @@ ConvertCloneBuffersToArrayInternal(
   }
 
   *aResult = OBJECT_TO_JSVAL(array);
   return NS_OK;
 }
 
 } // anonymous namespace
 
+HelperBase::~HelperBase()
+{
+  if (!NS_IsMainThread()) {
+    IDBRequest* request;
+    mRequest.forget(&request);
+
+    if (request) {
+      nsCOMPtr<nsIThread> mainThread;
+      NS_GetMainThread(getter_AddRefs(mainThread));
+      NS_WARN_IF_FALSE(mainThread, "Couldn't get the main thread!");
+
+      if (mainThread) {
+        NS_ProxyRelease(mainThread, static_cast<nsIDOMEventTarget*>(request));
+      }
+    }
+  }
+}
+
 nsresult
 HelperBase::WrapNative(JSContext* aCx,
                        nsISupports* aNative,
                        jsval* aResult)
 {
   NS_ASSERTION(aCx, "Null context!");
   NS_ASSERTION(aNative, "Null pointer!");
   NS_ASSERTION(aResult, "Null pointer!");
@@ -136,16 +154,24 @@ HelperBase::WrapNative(JSContext* aCx,
 
   nsresult rv =
     nsContentUtils::WrapNative(aCx, global, aNative, aResult);
   NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
 
   return NS_OK;
 }
 
+void
+HelperBase::ReleaseMainThreadObjects()
+{
+  NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
+
+  mRequest = nsnull;
+}
+
 AsyncConnectionHelper::AsyncConnectionHelper(IDBDatabase* aDatabase,
                                              IDBRequest* aRequest)
 : HelperBase(aRequest),
   mDatabase(aDatabase),
   mTimeoutDuration(TimeDuration::FromMilliseconds(kDefaultTimeoutMS)),
   mResultCode(NS_OK),
   mDispatched(false)
 {
@@ -168,34 +194,28 @@ AsyncConnectionHelper::~AsyncConnectionH
 {
   if (!NS_IsMainThread()) {
     IDBDatabase* database;
     mDatabase.forget(&database);
 
     IDBTransaction* transaction;
     mTransaction.forget(&transaction);
 
-    IDBRequest* request;
-    mRequest.forget(&request);
-
     nsCOMPtr<nsIThread> mainThread;
     NS_GetMainThread(getter_AddRefs(mainThread));
     NS_WARN_IF_FALSE(mainThread, "Couldn't get the main thread!");
 
     if (mainThread) {
       if (database) {
         NS_ProxyRelease(mainThread, static_cast<nsIIDBDatabase*>(database));
       }
       if (transaction) {
         NS_ProxyRelease(mainThread,
                         static_cast<nsIIDBTransaction*>(transaction));
       }
-      if (request) {
-        NS_ProxyRelease(mainThread, static_cast<nsIDOMEventTarget*>(request));
-      }
     }
   }
 
   NS_ASSERTION(!mOldProgressHandler, "Should not have anything here!");
 }
 
 NS_IMPL_THREADSAFE_ISUPPORTS2(AsyncConnectionHelper, nsIRunnable,
                                                      mozIStorageProgressHandler)
@@ -486,17 +506,18 @@ AsyncConnectionHelper::GetSuccessResult(
 
 void
 AsyncConnectionHelper::ReleaseMainThreadObjects()
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
 
   mDatabase = nsnull;
   mTransaction = nsnull;
-  mRequest = nsnull;
+
+  HelperBase::ReleaseMainThreadObjects();
 }
 
 // static
 nsresult
 AsyncConnectionHelper::ConvertCloneBuffersToArray(
                                 JSContext* aCx,
                                 nsTArray<JSAutoStructuredCloneBuffer>& aBuffers,
                                 jsval* aResult)
--- a/dom/indexedDB/AsyncConnectionHelper.h
+++ b/dom/indexedDB/AsyncConnectionHelper.h
@@ -70,24 +70,33 @@ public:
   virtual nsresult GetSuccessResult(JSContext* aCx,
                                     jsval* aVal) = 0;
 
 protected:
   HelperBase(IDBRequest* aRequest)
     : mRequest(aRequest)
   { }
 
+  virtual ~HelperBase();
+
   /**
    * Helper to wrap a native into a jsval. Uses the global object of the request
    * to parent the native.
    */
   nsresult WrapNative(JSContext* aCx,
                       nsISupports* aNative,
                       jsval* aResult);
 
+  /**
+   * Gives the subclass a chance to release any objects that must be released
+   * on the main thread, regardless of success or failure. Subclasses that
+   * implement this method *MUST* call the base class implementation as well.
+   */
+  virtual void ReleaseMainThreadObjects();
+
   nsRefPtr<IDBRequest> mRequest;
 };
 
 /**
  * Must be subclassed. The subclass must implement DoDatabaseWork. It may then
  * choose to implement OnSuccess and OnError depending on the needs of the
  * subclass. If the default implementation of OnSuccess is desired then the
  * subclass can implement GetSuccessResult to properly set the result of the
--- a/dom/indexedDB/OpenDatabaseHelper.cpp
+++ b/dom/indexedDB/OpenDatabaseHelper.cpp
@@ -727,16 +727,18 @@ OpenDatabaseHelper::Run()
     NS_ASSERTION(mState == eFiringEvents, "Why are we here?");
 
     if (NS_FAILED(mResultCode)) {
       DispatchErrorEvent();
     } else {
       DispatchSuccessEvent();
     }
 
+    ReleaseMainThreadObjects();
+
     return NS_OK;
   }
 
   // If we're on the DB thread, do that
   NS_ASSERTION(mState == eDBWork, "Why are we here?");
   mResultCode = DoDatabaseWork();
   NS_ASSERTION(mState != eDBWork, "We should be doing something else now.");
 
@@ -896,16 +898,27 @@ OpenDatabaseHelper::DispatchErrorEvent()
   if (!errorCode) {
     mOpenDBRequest->SetError(mResultCode);
   }
 
   bool dummy;
   mOpenDBRequest->DispatchEvent(event, &dummy);
 }
 
+void
+OpenDatabaseHelper::ReleaseMainThreadObjects()
+{
+  NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
+
+  mOpenDBRequest = nsnull;
+  mDatabase = nsnull;
+
+  HelperBase::ReleaseMainThreadObjects();
+}
+
 NS_IMPL_ISUPPORTS_INHERITED0(SetVersionHelper, AsyncConnectionHelper);
 
 nsresult
 SetVersionHelper::DoDatabaseWork(mozIStorageConnection* aConnection)
 {
   NS_ASSERTION(aConnection, "Passing a null connection!");
 
   nsCOMPtr<mozIStorageStatement> stmt;
--- a/dom/indexedDB/OpenDatabaseHelper.h
+++ b/dom/indexedDB/OpenDatabaseHelper.h
@@ -86,16 +86,17 @@ public:
 protected:
   // Methods only called on the main thread
   nsresult EnsureSuccessResult();
   nsresult StartSetVersion();
   nsresult GetSuccessResult(JSContext* aCx,
                           jsval* aVal);
   void DispatchSuccessEvent();
   void DispatchErrorEvent();
+  void ReleaseMainThreadObjects();
 
   // Methods only called on the DB thread
   nsresult DoDatabaseWork();
 
 private:
   // In-params.
   nsRefPtr<IDBOpenDBRequest> mOpenDBRequest;
   nsString mName;