Bug 1186745 part 6 - Fix event leak when using NS_DispatchToCurrentThread. r=froydnj
☠☠ backed out by 00a507c0f662 ☠ ☠
authorXidorn Quan <quanxunzhen@gmail.com>
Tue, 29 Sep 2015 09:28:22 +1000
changeset 264807 c6142b815de04b37c1b798b79729dbdb4e827330
parent 264806 d8f740ef24304bb4088be28ba1c0204962aeadf1
child 264835 78f4f7457830efa88a71b4f61d765973354bc343
push id65754
push userxquan@mozilla.com
push dateMon, 28 Sep 2015 23:28:58 +0000
treeherdermozilla-inbound@c6142b815de0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1186745
milestone44.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 1186745 part 6 - Fix event leak when using NS_DispatchToCurrentThread. r=froydnj
xpcom/glue/nsThreadUtils.cpp
xpcom/glue/nsThreadUtils.h
--- a/xpcom/glue/nsThreadUtils.cpp
+++ b/xpcom/glue/nsThreadUtils.cpp
@@ -137,36 +137,54 @@ NS_IsMainThread()
     do_GetService(NS_THREADMANAGER_CONTRACTID);
   if (mgr) {
     mgr->GetIsMainThread(&result);
   }
   return bool(result);
 }
 #endif
 
-// It is common to call NS_DispatchToCurrentThread with a newly
-// allocated runnable with a refcount of zero. To keep us from leaking
-// the runnable if the dispatch method fails, we take a death grip.
 NS_METHOD
-NS_DispatchToCurrentThread(nsIRunnable* aEvent)
+NS_DispatchToCurrentThread(already_AddRefed<nsIRunnable>&& aEvent)
 {
-  nsCOMPtr<nsIRunnable> deathGrip = aEvent;
+  nsresult rv;
+  nsCOMPtr<nsIRunnable> event(aEvent);
 #ifdef MOZILLA_INTERNAL_API
   nsIThread* thread = NS_GetCurrentThread();
   if (!thread) {
     return NS_ERROR_UNEXPECTED;
   }
 #else
   nsCOMPtr<nsIThread> thread;
-  nsresult rv = NS_GetCurrentThread(getter_AddRefs(thread));
+  rv = NS_GetCurrentThread(getter_AddRefs(thread));
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 #endif
-  return thread->Dispatch(aEvent, NS_DISPATCH_NORMAL);
+  // To keep us from leaking the runnable if dispatch method fails,
+  // we grab the reference on failures and release it.
+  nsIRunnable* temp = event.get();
+  rv = thread->Dispatch(event.forget(), NS_DISPATCH_NORMAL);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    // Dispatch() leaked the reference to the event, but due to caller's
+    // assumptions, we shouldn't leak here. And given we are on the same
+    // thread as the dispatch target, it's mostly safe to do it here.
+    NS_RELEASE(temp);
+  }
+  return rv;
+}
+
+// It is common to call NS_DispatchToCurrentThread with a newly
+// allocated runnable with a refcount of zero. To keep us from leaking
+// the runnable if the dispatch method fails, we take a death grip.
+NS_METHOD
+NS_DispatchToCurrentThread(nsIRunnable* aEvent)
+{
+  nsCOMPtr<nsIRunnable> event(aEvent);
+  return NS_DispatchToCurrentThread(event.forget());
 }
 
 NS_METHOD
 NS_DispatchToMainThread(already_AddRefed<nsIRunnable>&& aEvent, uint32_t aDispatchFlags)
 {
   LeakRefPtr<nsIRunnable> event(Move(aEvent));
   nsCOMPtr<nsIThread> thread;
   nsresult rv = NS_GetMainThread(getter_AddRefs(thread));
--- a/xpcom/glue/nsThreadUtils.h
+++ b/xpcom/glue/nsThreadUtils.h
@@ -101,16 +101,18 @@ extern NS_METHOD NS_GetCurrentThread(nsI
  *
  * @param aEvent
  *   The event to dispatch.
  *
  * @returns NS_ERROR_INVALID_ARG
  *   If event is null.
  */
 extern NS_METHOD NS_DispatchToCurrentThread(nsIRunnable* aEvent);
+extern NS_METHOD
+NS_DispatchToCurrentThread(already_AddRefed<nsIRunnable>&& aEvent);
 
 /**
  * Dispatch the given event to the main thread.
  *
  * @param aEvent
  *   The event to dispatch.
  * @param aDispatchFlags
  *   The flags to pass to the main thread's dispatch method.