Bug 887889 - Fix leak in RemoteSpellCheckingEngineChild r=Ehsan
authorDoug Thayer <dothayer@mozilla.com>
Thu, 19 Apr 2018 14:33:18 -0700
changeset 472028 fad1b59c9f557e5d6f09c51b578c2ee9762d26c1
parent 472027 6480e870dd73969336b30fb2c0ecfc20c91b3d71
child 472029 20e6e6da280d51500e980084e17b83e768268b4c
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersEhsan
bugs887889
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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_