Bug 1374278 - Fix a race condition between GPUChild and CompositorManagerChild when the GPU process crashes. r=dvander
authorAndrew Osmond <aosmond@mozilla.com>
Fri, 23 Jun 2017 16:12:35 -0400
changeset 366036 b59e819dfaa55698c4c8dfbd26f516e512f15dbd
parent 366035 02c5f667487d295285ec16491195940f2628acfb
child 366037 78d9a157ab16782948b63080bfcbac4435283a12
push id32090
push usercbook@mozilla.com
push dateMon, 26 Jun 2017 11:27:03 +0000
treeherdermozilla-central@af9a8cebd43f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdvander
bugs1374278
milestone56.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 1374278 - Fix a race condition between GPUChild and CompositorManagerChild when the GPU process crashes. r=dvander
gfx/ipc/GPUProcessManager.cpp
gfx/layers/ipc/CompositorManagerChild.cpp
gfx/layers/ipc/CompositorManagerChild.h
--- a/gfx/ipc/GPUProcessManager.cpp
+++ b/gfx/ipc/GPUProcessManager.cpp
@@ -198,17 +198,21 @@ GPUProcessManager::EnsureGPUReady()
   }
 
   return false;
 }
 
 void
 GPUProcessManager::EnsureCompositorManagerChild()
 {
-  if (CompositorManagerChild::IsInitialized()) {
+  base::ProcessId gpuPid = EnsureGPUReady()
+                           ? mGPUChild->OtherPid()
+                           : base::GetCurrentProcId();
+
+  if (CompositorManagerChild::IsInitialized(gpuPid)) {
     return;
   }
 
   if (!EnsureGPUReady()) {
     CompositorManagerChild::InitSameProcess(AllocateNamespace());
     return;
   }
 
--- a/gfx/layers/ipc/CompositorManagerChild.cpp
+++ b/gfx/layers/ipc/CompositorManagerChild.cpp
@@ -14,46 +14,52 @@
 #include "VsyncSource.h"
 
 namespace mozilla {
 namespace layers {
 
 StaticRefPtr<CompositorManagerChild> CompositorManagerChild::sInstance;
 
 /* static */ bool
-CompositorManagerChild::IsInitialized()
+CompositorManagerChild::IsInitialized(base::ProcessId aGPUPid)
 {
   MOZ_ASSERT(NS_IsMainThread());
-  return sInstance && sInstance->CanSend();
+  // Since GPUChild and CompositorManagerChild will race on ActorDestroy, we
+  // cannot know if the CompositorManagerChild is about to be released but has
+  // yet to be. As such, we need to verify the GPU PID matches as well.
+  return sInstance && sInstance->CanSend() && sInstance->OtherPid() == aGPUPid;
 }
 
 /* static */ bool
 CompositorManagerChild::InitSameProcess(uint32_t aNamespace)
 {
   MOZ_ASSERT(NS_IsMainThread());
-  if (NS_WARN_IF(sInstance &&
-                 sInstance->OtherPid() == base::GetCurrentProcId())) {
-    MOZ_ASSERT_UNREACHABLE("Already initialized");
+  if (NS_WARN_IF(IsInitialized(base::GetCurrentProcId()))) {
+    MOZ_ASSERT_UNREACHABLE("Already initialized same process");
     return false;
   }
 
   RefPtr<CompositorManagerParent> parent =
     CompositorManagerParent::CreateSameProcess();
   sInstance = new CompositorManagerChild(parent, aNamespace);
   return true;
 }
 
 /* static */ bool
 CompositorManagerChild::Init(Endpoint<PCompositorManagerChild>&& aEndpoint,
                              uint32_t aNamespace)
 {
   MOZ_ASSERT(NS_IsMainThread());
   if (sInstance) {
+    // Since GPUChild and CompositorManagerChild will race on ActorDestroy, we
+    // cannot know if the CompositorManagerChild has yet to be released or not.
+    // To avoid an unnecessary reinitialization, we verify the GPU PID actually
+    // changed.
     MOZ_ASSERT(sInstance->mNamespace != aNamespace);
-    MOZ_RELEASE_ASSERT(!sInstance->CanSend());
+    MOZ_RELEASE_ASSERT(sInstance->OtherPid() != aEndpoint.OtherPid());
   }
 
   sInstance = new CompositorManagerChild(Move(aEndpoint), aNamespace);
   return sInstance->CanSend();
 }
 
 /* static */ void
 CompositorManagerChild::Shutdown()
--- a/gfx/layers/ipc/CompositorManagerChild.h
+++ b/gfx/layers/ipc/CompositorManagerChild.h
@@ -19,17 +19,17 @@ namespace layers {
 class CompositorManagerParent;
 class LayerManager;
 
 class CompositorManagerChild : public PCompositorManagerChild
 {
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(CompositorManagerChild)
 
 public:
-  static bool IsInitialized();
+  static bool IsInitialized(base::ProcessId aPid);
   static bool InitSameProcess(uint32_t aNamespace);
   static bool Init(Endpoint<PCompositorManagerChild>&& aEndpoint,
                    uint32_t aNamespace);
   static void Shutdown();
 
   static bool
   CreateContentCompositorBridge(uint32_t aNamespace);