Bug 1286798 - Part 23: Fix GetOrCreateForCurrentThread() when the runnable for calling SendInitBackground() on the main thread was already dispatched, but the main thread then entered a nested event loop; r=asuth
authorJan Varga <jan.varga@gmail.com>
Thu, 29 Nov 2018 21:48:28 +0100
changeset 505241 bd8fd9a22dc08d63cec68de475182a1b8eeeca4b
parent 505240 076cc59fa79341fd5cd472d336cbf6e8dac53029
child 505242 c0d40572c29bc3be2611f6e40bb554a55fcc54a3
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs1286798
milestone65.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 1286798 - Part 23: Fix GetOrCreateForCurrentThread() when the runnable for calling SendInitBackground() on the main thread was already dispatched, but the main thread then entered a nested event loop; r=asuth See the big comment in the code for more details.
ipc/glue/BackgroundImpl.cpp
--- a/ipc/glue/BackgroundImpl.cpp
+++ b/ipc/glue/BackgroundImpl.cpp
@@ -19,16 +19,18 @@
 #include "mozilla/ClearOnShutdown.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/Services.h"
 #include "mozilla/StaticPtr.h"
 #include "mozilla/Unused.h"
 #include "mozilla/dom/ContentChild.h"
 #include "mozilla/dom/ContentParent.h"
 #include "mozilla/dom/File.h"
+#include "mozilla/dom/WorkerPrivate.h"
+#include "mozilla/dom/WorkerRef.h"
 #include "mozilla/ipc/ProtocolTypes.h"
 #include "nsAutoPtr.h"
 #include "nsCOMPtr.h"
 #include "nsIEventTarget.h"
 #include "nsIMutable.h"
 #include "nsIObserver.h"
 #include "nsIObserverService.h"
 #include "nsIRunnable.h"
@@ -288,17 +290,17 @@ class ChildImpl final : public Backgroun
 {
   friend class mozilla::ipc::BackgroundChild;
   friend class mozilla::ipc::BackgroundChildImpl;
 
   typedef base::ProcessId ProcessId;
   typedef mozilla::ipc::Transport Transport;
 
   class ShutdownObserver;
-  class ActorCreatedRunnable;
+  class SendInitBackgroundRunnable;
 
   // A thread-local index that is not valid.
   static const unsigned int kBadThreadLocalIndex =
     static_cast<unsigned int>(-1);
 
   // This is only modified on the main thread. It is the thread-local index that
   // we use to store the BackgroundChild for each thread.
   static unsigned int sThreadLocalIndex;
@@ -308,16 +310,17 @@ class ChildImpl final : public Backgroun
     ThreadLocalInfo()
 #ifdef DEBUG
       : mClosed(false)
 #endif
     {
     }
 
     RefPtr<ChildImpl> mActor;
+    RefPtr<SendInitBackgroundRunnable> mSendInitBackgroundRunnable;
     nsAutoPtr<BackgroundChildImpl::ThreadLocal> mConsumerThreadLocal;
 #ifdef DEBUG
     bool mClosed;
 #endif
   };
 
   // On the main thread, we store TLS in this global instead of in
   // sThreadLocalIndex. That way, cooperative main threads all share the same
@@ -405,30 +408,17 @@ private:
   static void
   CloseForCurrentThread();
 
   // Forwarded from BackgroundChildImpl.
   static BackgroundChildImpl::ThreadLocal*
   GetThreadLocalForCurrentThread();
 
   static void
-  ThreadLocalDestructor(void* aThreadLocal)
-  {
-    auto threadLocalInfo = static_cast<ThreadLocalInfo*>(aThreadLocal);
-
-    if (threadLocalInfo) {
-      MOZ_ASSERT(threadLocalInfo->mClosed);
-
-      if (threadLocalInfo->mActor) {
-        threadLocalInfo->mActor->Close();
-        threadLocalInfo->mActor->AssertActorDestroyed();
-      }
-      delete threadLocalInfo;
-    }
-  }
+  ThreadLocalDestructor(void* aThreadLocal);
 
   // This class is reference counted.
   ~ChildImpl()
   {
     MOZ_ASSERT_IF(mActorWasAlive, mActorDestroyed);
   }
 
   // Only called by IPDL.
@@ -618,16 +608,53 @@ public:
 
 private:
   ~ShutdownObserver()
   {
     AssertIsOnMainThread();
   }
 };
 
+class ChildImpl::SendInitBackgroundRunnable final
+  : public CancelableRunnable
+{
+  nsCOMPtr<nsISerialEventTarget> mOwningEventTarget;
+  RefPtr<StrongWorkerRef> mWorkerRef;
+  Endpoint<PBackgroundParent> mParent;
+  mozilla::Mutex mMutex;
+  bool mSentInitBackground;
+
+public:
+  static already_AddRefed<SendInitBackgroundRunnable>
+  Create(Endpoint<PBackgroundParent>&& aParent);
+
+  void
+  ClearEventTarget()
+  {
+    mWorkerRef = nullptr;
+
+    mozilla::MutexAutoLock lock(mMutex);
+    mOwningEventTarget = nullptr;
+  }
+
+private:
+  explicit SendInitBackgroundRunnable(Endpoint<PBackgroundParent>&& aParent)
+    : CancelableRunnable("Background::ChildImpl::SendInitBackgroundRunnable")
+    , mOwningEventTarget(GetCurrentThreadSerialEventTarget())
+    , mParent(std::move(aParent))
+    , mMutex("SendInitBackgroundRunnable::mMutex")
+    , mSentInitBackground(false)
+  { }
+
+  ~SendInitBackgroundRunnable()
+  { }
+
+  NS_DECL_NSIRUNNABLE
+};
+
 } // namespace
 
 namespace mozilla {
 namespace ipc {
 
 bool
 IsOnBackgroundThread()
 {
@@ -1484,16 +1511,39 @@ ChildImpl::GetOrCreateForCurrentThread(n
         return nullptr;
       }
     }
 
     threadLocalInfo = newInfo.forget();
   }
 
   if (threadLocalInfo->mActor) {
+    RefPtr<SendInitBackgroundRunnable>& runnable =
+      threadLocalInfo->mSendInitBackgroundRunnable;
+
+    if (aMainEventTarget && runnable) {
+      // The SendInitBackgroundRunnable was already dispatched to the main
+      // thread to finish initialization of a new background child actor.
+      // However, the caller passed a custom main event target which indicates
+      // that synchronous blocking of the main thread is happening (done by
+      // creating a nested event target and spinning the event loop).
+      // It can happen that the SendInitBackgroundRunnable didn't have a chance
+      // to run before the synchronous blocking has occured. Unblocking of the
+      // main thread can depend on an IPC message received on this thread, so
+      // we have to dispatch the SendInitBackgroundRunnable to the custom main
+      // event target too, otherwise IPC will be only queueing messages on this
+      // thread. The runnable will run twice in the end, but that's a harmless
+      // race between the main and nested event queue of the main thread.
+      // There's a guard in the runnable implementation for calling
+      // SendInitBackground only once.
+
+      MOZ_ALWAYS_SUCCEEDS(aMainEventTarget->Dispatch(runnable,
+                                                     NS_DISPATCH_NORMAL));
+    }
+
     return threadLocalInfo->mActor;
   }
 
   if (XRE_IsParentProcess()) {
     RefPtr<ChildImpl> strongActor =
       ParentImpl::CreateActorForSameProcess(aMainEventTarget);
     if (NS_WARN_IF(!strongActor)) {
       return nullptr;
@@ -1520,44 +1570,48 @@ ChildImpl::GetOrCreateForCurrentThread(n
   rv = PBackground::CreateEndpoints(content->OtherPid(),
                                     base::GetCurrentProcId(),
                                     &parent, &child);
   if (NS_FAILED(rv)) {
     NS_WARNING("Failed to create top level actor!");
     return nullptr;
   }
 
+  RefPtr<SendInitBackgroundRunnable> runnable;
+  if (!NS_IsMainThread()) {
+    runnable = SendInitBackgroundRunnable::Create(std::move(parent));
+    if (!runnable) {
+      return nullptr;
+    }
+  }
+
   RefPtr<ChildImpl> strongActor = new ChildImpl();
 
   if (!child.Bind(strongActor)) {
     CRASH_IN_CHILD_PROCESS("Failed to bind ChildImpl!");
 
     return nullptr;
   }
 
   strongActor->SetActorAlive();
 
   if (NS_IsMainThread()) {
     if (!content->SendInitBackground(std::move(parent))) {
       NS_WARNING("Failed to create top level actor!");
       return nullptr;
     }
   } else {
-    nsCOMPtr<nsIRunnable> runnable =
-      NewRunnableMethod<Endpoint<PBackgroundParent>&&>(
-        "dom::ContentChild::SendInitBackground",
-        content,
-        &ContentChild::SendInitBackground,
-        std::move(parent));
     if (aMainEventTarget) {
       MOZ_ALWAYS_SUCCEEDS(aMainEventTarget->Dispatch(runnable,
                                                      NS_DISPATCH_NORMAL));
     } else {
       MOZ_ALWAYS_SUCCEEDS(NS_DispatchToMainThread(runnable));
     }
+
+    threadLocalInfo->mSendInitBackgroundRunnable = runnable;
   }
 
   RefPtr<ChildImpl>& actor = threadLocalInfo->mActor;
   strongActor.swap(actor);
 
   return actor;
 }
 
@@ -1606,16 +1660,38 @@ ChildImpl::GetThreadLocalForCurrentThrea
   if (!threadLocalInfo->mConsumerThreadLocal) {
     threadLocalInfo->mConsumerThreadLocal =
       new BackgroundChildImpl::ThreadLocal();
   }
 
   return threadLocalInfo->mConsumerThreadLocal;
 }
 
+// static
+void
+ChildImpl::ThreadLocalDestructor(void* aThreadLocal)
+{
+  auto threadLocalInfo = static_cast<ThreadLocalInfo*>(aThreadLocal);
+
+  if (threadLocalInfo) {
+    MOZ_ASSERT(threadLocalInfo->mClosed);
+
+    if (threadLocalInfo->mActor) {
+      threadLocalInfo->mActor->Close();
+      threadLocalInfo->mActor->AssertActorDestroyed();
+    }
+
+    if (threadLocalInfo->mSendInitBackgroundRunnable) {
+      threadLocalInfo->mSendInitBackgroundRunnable->ClearEventTarget();
+    }
+
+    delete threadLocalInfo;
+  }
+}
+
 void
 ChildImpl::ActorDestroy(ActorDestroyReason aWhy)
 {
   AssertIsOnOwningThread();
 
 #ifdef DEBUG
   MOZ_ASSERT(!mActorDestroyed);
   mActorDestroyed = true;
@@ -1633,8 +1709,85 @@ ChildImpl::ShutdownObserver::Observe(nsI
 {
   AssertIsOnMainThread();
   MOZ_ASSERT(!strcmp(aTopic, NS_XPCOM_SHUTDOWN_THREADS_OBSERVER_ID));
 
   ChildImpl::Shutdown();
 
   return NS_OK;
 }
+
+// static
+already_AddRefed<ChildImpl::SendInitBackgroundRunnable>
+ChildImpl::
+SendInitBackgroundRunnable::Create(Endpoint<PBackgroundParent>&& aParent)
+{
+  MOZ_ASSERT(!NS_IsMainThread());
+
+  RefPtr<SendInitBackgroundRunnable> runnable =
+    new SendInitBackgroundRunnable(std::move(aParent));
+
+  WorkerPrivate* workerPrivate = mozilla::dom::GetCurrentThreadWorkerPrivate();
+  if (!workerPrivate) {
+    return runnable.forget();
+  }
+
+  workerPrivate->AssertIsOnWorkerThread();
+
+  runnable->mWorkerRef =
+    StrongWorkerRef::Create(workerPrivate,
+                            "ChildImpl::SendInitBackgroundRunnable");
+  if (NS_WARN_IF(!runnable->mWorkerRef)) {
+    return nullptr;
+  }
+
+  return runnable.forget();
+}
+
+NS_IMETHODIMP
+ChildImpl::
+SendInitBackgroundRunnable::Run()
+{
+  if (NS_IsMainThread()) {
+    if (mSentInitBackground) {
+      return NS_OK;
+    }
+
+    mSentInitBackground = true;
+
+    RefPtr<ContentChild> content = ContentChild::GetSingleton();
+    MOZ_ASSERT(content);
+
+    if (!content->SendInitBackground(std::move(mParent))) {
+      MOZ_CRASH("Failed to create top level actor!");
+    }
+
+    nsCOMPtr<nsISerialEventTarget> owningEventTarget;
+    {
+      mozilla::MutexAutoLock lock(mMutex);
+      owningEventTarget = mOwningEventTarget;
+    }
+
+    if (!owningEventTarget) {
+      return NS_OK;
+    }
+
+    nsresult rv = owningEventTarget->Dispatch(this, NS_DISPATCH_NORMAL);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
+
+    return NS_OK;
+  }
+
+  ClearEventTarget();
+
+  auto threadLocalInfo =
+    static_cast<ThreadLocalInfo*>(PR_GetThreadPrivate(sThreadLocalIndex));
+
+  if (!threadLocalInfo) {
+    return NS_OK;
+  }
+
+  threadLocalInfo->mSendInitBackgroundRunnable = nullptr;
+
+  return NS_OK;
+}