Bug 1743454 - Ensure GPU process crash reports are generated regardless of which IPC actor dies first. r=aosmond

GPU process crash reports are handled by calling GenerateCrashReport()
in GPUChild::ActorDestroy() if the reason is AbnormalShutdown. This
ensures we only create crash report if the process actually crashed,
and not when it was deliberately stopped.

However, sometimes actors other than GPUChild are the first to be
destroyed immediately after a crash, for example CompositorBridgeChild
or UiCompositorControllerChild. If such an actor receives an
ActorDestroy message with AbnormalShutdown as the reason, they will
call GPUProcessManager::NotifyRemoteActorDestroyed(), which leads to
GPUProcessHost::Shutdown(), which will close the PGPU channel. This
creates a race condition after a GPU process crash, where sometimes
the channel gets closed gracefully and ActorDestroy will receive a
NormalShutdown reason rather than AbnormalShutdown.

This patch adds a flag to GPUProcessHost::Shutdown() indicating
whether it is being called in response to an unexpected shutdown being
detected by another actor. If set, it sets a flag on the
GPUChild. When GPUChild::ActorDestroy() eventually gets called, it
knows to act in response to a crash if either the reason is
AbnormalShutdown or this flag has been set.

Differential Revision: https://phabricator.services.mozilla.com/D132811

gfx/ipc/GPUChild.cpp

@@ -103,6 +103,8 @@ base::ProcessHandle GPUChild::GetChildProcessHandle() {
   return mHost->GetChildProcessHandle();
 }
 
+void GPUChild::OnUnexpectedShutdown() { mUnexpectedShutdown = true; }
+
 mozilla::ipc::IPCResult GPUChild::RecvInitComplete(const GPUDeviceData& aData) {
   // We synchronously requested GPU parameters before this arrived.
   if (mGPUReady) {
@@ -265,7 +267,7 @@ mozilla::ipc::IPCResult GPUChild::RecvAddMemoryReport(
 }
 
 void GPUChild::ActorDestroy(ActorDestroyReason aWhy) {
-  if (aWhy == AbnormalShutdown) {
+  if (aWhy == AbnormalShutdown || mUnexpectedShutdown) {
     nsAutoString dumpId;
     GenerateCrashReport(OtherPid(), &dumpId);
 

gfx/ipc/GPUChild.h

@@ -35,6 +35,13 @@ class GPUChild final : public ipc::CrashReporterHelper<GeckoProcessType_GPU>,
   bool EnsureGPUReady();
   base::ProcessHandle GetChildProcessHandle();
 
+  // Notifies that an unexpected GPU process shutdown has been noticed by a
+  // different IPDL actor, and the GPU process is being torn down as a result.
+  // ActorDestroy may receive either NormalShutdown or AbnormalShutdown as a
+  // reason, depending on timings, but either way we treat the shutdown as
+  // abnormal.
+  void OnUnexpectedShutdown();
+
   // gfxVarReceiver overrides.
   void OnVarChanged(const GfxVarUpdate& aVar) override;
 
@@ -85,6 +92,7 @@ class GPUChild final : public ipc::CrashReporterHelper<GeckoProcessType_GPU>,
   GPUProcessHost* mHost;
   UniquePtr<MemoryReportRequestHost> mMemoryReportRequest;
   bool mGPUReady;
+  bool mUnexpectedShutdown = false;
 };
 
 }  // namespace gfx

gfx/ipc/GPUProcessHost.cpp

@@ -170,7 +170,7 @@ void GPUProcessHost::InitAfterConnect(bool aSucceeded) {
   }
 }
 
-void GPUProcessHost::Shutdown() {
+void GPUProcessHost::Shutdown(bool aUnexpectedShutdown) {
   MOZ_ASSERT(!mShutdownRequested);
 
   mListener = nullptr;
@@ -180,6 +180,10 @@ void GPUProcessHost::Shutdown() {
     // unexpected.
     mShutdownRequested = true;
 
+    if (aUnexpectedShutdown) {
+      mGPUChild->OnUnexpectedShutdown();
+    }
+
     // The channel might already be closed if we got here unexpectedly.
     if (!mChannelClosed) {
       if (VRGPUChild::IsCreated()) {

gfx/ipc/GPUProcessHost.h

@@ -77,7 +77,11 @@ class GPUProcessHost final : public mozilla::ipc::GeckoChildProcessHost {
   // GPUProcessHost.
   //
   // After this returns, the attached Listener is no longer used.
-  void Shutdown();
+  //
+  // Setting aUnexpectedShutdown = true indicates that this is being called to
+  // clean up resources in response to an unexpected shutdown having been
+  // detected.
+  void Shutdown(bool aUnexpectedShutdown = false);
 
   // Return the actor for the top-level actor of the process. If the process
   // has not connected yet, this returns null.

gfx/ipc/GPUProcessManager.cpp

@@ -642,7 +642,7 @@ void GPUProcessManager::OnProcessUnexpectedShutdown(GPUProcessHost* aHost) {
   }
 
   CompositorManagerChild::OnGPUProcessLost(aHost->GetProcessToken());
-  DestroyProcess();
+  DestroyProcess(/* aUnexpectedShutdown */ true);
 
   if (mUnstableProcessAttempts >
       uint32_t(StaticPrefs::layers_gpu_process_max_restarts())) {
@@ -812,12 +812,12 @@ void GPUProcessManager::KillProcess() {
   mProcess->KillProcess();
 }
 
-void GPUProcessManager::DestroyProcess() {
+void GPUProcessManager::DestroyProcess(bool aUnexpectedShutdown) {
   if (!mProcess) {
     return;
   }
 
-  mProcess->Shutdown();
+  mProcess->Shutdown(aUnexpectedShutdown);
   mProcessToken = 0;
   mProcess = nullptr;
   mGPUChild = nullptr;

gfx/ipc/GPUProcessManager.h

@@ -252,7 +252,11 @@ class GPUProcessManager final : public GPUProcessHost::Listener {
 
   // Shutdown the GPU process.
   void CleanShutdown();
-  void DestroyProcess();
+  // Destroy the process and clean up resources.
+  // Setting aUnexpectedShutdown = true indicates that this is being called to
+  // clean up resources in response to an unexpected shutdown having been
+  // detected.
+  void DestroyProcess(bool aUnexpectedShutdown = false);
 
   void HandleProcessLost();