Bug 887889 - Fix leak in RemoteSpellCheckingEngineChild r?ehsan draft
authorDoug Thayer <dothayer@mozilla.com>
Thu, 19 Apr 2018 14:33:18 -0700
changeset 788707 6152e912d0df502095ae69b034b3034e6171d6e9
parent 788706 be5903304b63dffffa17f7e2305243464c537e16
child 788708 f6c24f1b2ec70025d63296291264cbe191af666a
push id108067
push userbmo:dothayer@mozilla.com
push dateThu, 26 Apr 2018 21:12:12 +0000
reviewersehsan
bugs887889
milestone61.0a1
Bug 887889 - Fix leak in RemoteSpellCheckingEngineChild r?ehsan The work to migrate to Sqlite.jsm seems to have caused a timing problem in our tests where shutdown the content process while this IPC message is still unresolved. This causes us to destroy RemoteSpellCheckingEngineChild without it having processed its RecvNotiy..., leading to the promise being leaked. As far as I can tell this resolves all of our leak issues on try. MozReview-Commit-ID: GdwVIp5dj1m
editor/spellchecker/EditorSpellCheck.cpp
extensions/spellcheck/hunspell/glue/RemoteSpellCheckEngineChild.cpp
extensions/spellcheck/hunspell/glue/RemoteSpellCheckEngineChild.h
--- a/editor/spellchecker/EditorSpellCheck.cpp
+++ b/editor/spellchecker/EditorSpellCheck.cpp
@@ -891,17 +891,20 @@ EditorSpellCheck::DictionaryFetched(Dict
           // list.
           self->DeleteSuggestedWordList();
 
           self->EndUpdateDictionary();
           if (fetcher->mCallback) {
             fetcher->mCallback->EditorSpellCheckDone();
           }
         },
-        [self, fetcher]() {
+        [self, fetcher](nsresult aError) {
+          if (aError == NS_ERROR_ABORT) {
+            return;
+          }
           // May be dictionary was uninstalled ?
           // Clear the content preference and continue.
           ClearCurrentDictionary(self->mEditor);
 
           // Priority 2 or later will handled by the following
           self->SetFallbackDictionary(fetcher);
         });
       return NS_OK;
--- a/extensions/spellcheck/hunspell/glue/RemoteSpellCheckEngineChild.cpp
+++ b/extensions/spellcheck/hunspell/glue/RemoteSpellCheckEngineChild.cpp
@@ -12,44 +12,56 @@ RemoteSpellcheckEngineChild::RemoteSpell
 {
 }
 
 RemoteSpellcheckEngineChild::~RemoteSpellcheckEngineChild()
 {
   // null out the owner's SpellcheckEngineChild to prevent state corruption
   // during shutdown
   mOwner->DeleteRemoteEngine();
+
+  // ensure we don't leak any promise holders for which we haven't yet
+  // received responses
+  for (UniquePtr<MozPromiseHolder<GenericPromise>>& promiseHolder : mResponsePromises) {
+    promiseHolder->RejectIfExists(NS_ERROR_ABORT, __func__);
+  }
 }
 
 RefPtr<GenericPromise>
 RemoteSpellcheckEngineChild::SetCurrentDictionaryFromList(
   const nsTArray<nsString>& aList)
 {
-  MozPromiseHolder<GenericPromise>* promiseHolder =
-    new MozPromiseHolder<GenericPromise>();
+  UniquePtr<MozPromiseHolder<GenericPromise>> promiseHolder =
+    MakeUnique<MozPromiseHolder<GenericPromise>>();
   if (!SendSetDictionaryFromList(
          aList,
-         reinterpret_cast<intptr_t>(promiseHolder))) {
-    delete promiseHolder;
+         reinterpret_cast<intptr_t>(promiseHolder.get()))) {
     return GenericPromise::CreateAndReject(NS_ERROR_FAILURE, __func__);
   }
+  RefPtr<GenericPromise> result = promiseHolder->Ensure(__func__);
   // promiseHolder will removed by receive message
-  return promiseHolder->Ensure(__func__);
+  mResponsePromises.AppendElement(Move(promiseHolder));
+  return Move(result);
 }
 
 mozilla::ipc::IPCResult
 RemoteSpellcheckEngineChild::RecvNotifyOfCurrentDictionary(
                                const nsString& aDictionary,
                                const intptr_t& aId)
 {
   MozPromiseHolder<GenericPromise>* promiseHolder =
     reinterpret_cast<MozPromiseHolder<GenericPromise>*>(aId);
   mOwner->mCurrentDictionary = aDictionary;
   if (aDictionary.IsEmpty()) {
     promiseHolder->RejectIfExists(NS_ERROR_NOT_AVAILABLE, __func__);
   } else {
     promiseHolder->ResolveIfExists(true, __func__);
   }
-  delete promiseHolder;
+  for (uint32_t i = 0; i < mResponsePromises.Length(); ++i) {
+    if (mResponsePromises[i].get() == promiseHolder) {
+      mResponsePromises.RemoveElementAt(i);
+      break;
+    }
+  }
   return IPC_OK();
 }
 
 } //namespace mozilla
--- a/extensions/spellcheck/hunspell/glue/RemoteSpellCheckEngineChild.h
+++ b/extensions/spellcheck/hunspell/glue/RemoteSpellCheckEngineChild.h
@@ -23,13 +23,14 @@ public:
                                     const nsString& aDictionary,
                                     const intptr_t& aPromiseId) override;
 
   RefPtr<GenericPromise> SetCurrentDictionaryFromList(
                            const nsTArray<nsString>& aList);
 
 private:
   mozSpellChecker *mOwner;
+  nsTArray<UniquePtr<MozPromiseHolder<GenericPromise>>> mResponsePromises;
 };
 
 } //namespace mozilla
 
 #endif // RemoteSpellcheckEngineChild_h_