Bug 1827222 - GPUProcessManager should not allow (re)initialization during shutdown. r=jnicol

We already avoid launching the GPU process during shutdown, but shutdown could happen after we initiated the launch and before it completes. Top level IPDL protocols assert that we don't create them during shutdown, so we need to be more diligent about checking for that. Differential Revision: https://phabricator.services.mozilla.com/D175054
2023-05-02 14:56:26 +00:00 · 2023-05-02 14:56:26 +00:00 · 90299327da
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@ -3232,6 +3232,7 @@ bool ContentParent::InitInternal(ProcessPriority aInitialPriority) {
                                 &vrBridge, &videoManager, &namespaces)) {
    // This can fail if we've already started shutting down the compositor
    // thread. See Bug 1562763 comment 8.
+    MOZ_ASSERT(AppShutdown::IsInOrBeyond(ShutdownPhase::AppShutdown));
    return false;
  }

@ -3407,10 +3408,11 @@ void ContentParent::OnCompositorUnexpectedShutdown() {
  Endpoint<PRemoteDecoderManagerChild> videoManager;
  AutoTArray<uint32_t, 3> namespaces;

-  DebugOnly<bool> opened =
-      gpm->CreateContentBridges(OtherPid(), &compositor, &imageBridge,
-                                &vrBridge, &videoManager, &namespaces);
-  MOZ_ASSERT(opened);
+  if (!gpm->CreateContentBridges(OtherPid(), &compositor, &imageBridge,
+                                 &vrBridge, &videoManager, &namespaces)) {
+    MOZ_ASSERT(AppShutdown::IsInOrBeyond(ShutdownPhase::AppShutdown));
+    return;
+  }

  Unused << SendReinitRendering(std::move(compositor), std::move(imageBridge),
                                std::move(vrBridge), std::move(videoManager),
--- a/gfx/ipc/GPUProcessManager.cpp
+++ b/gfx/ipc/GPUProcessManager.cpp
@ -300,7 +300,9 @@ bool GPUProcessManager::MaybeDisableGPUProcess(const char* aMessage,
  // process equivalent, and we need to make sure those services are setup
  // correctly. We cannot re-enter DisableGPUProcess from this call because we
  // know that it is disabled in the config above.
-  EnsureProtocolsReady();
+  DebugOnly<bool> ready = EnsureProtocolsReady();
+  MOZ_ASSERT_IF(!ready,
+                AppShutdown::IsInOrBeyond(ShutdownPhase::XPCOMShutdown));

  // If we disable the GPU process during reinitialization after a previous
  // crash, then we need to tell the content processes again, because they
@ -309,28 +311,41 @@ bool GPUProcessManager::MaybeDisableGPUProcess(const char* aMessage,
  return true;
 }

-bool GPUProcessManager::EnsureGPUReady() {
+nsresult GPUProcessManager::EnsureGPUReady() {
  MOZ_ASSERT(NS_IsMainThread());

+  // We only wait to fail with NS_ERROR_ILLEGAL_DURING_SHUTDOWN if we would
+  // cause a state change or if we are in the middle of relaunching the GPU
+  // process.
+  bool inShutdown = AppShutdown::IsInOrBeyond(ShutdownPhase::XPCOMShutdown);
+
  // Launch the GPU process if it is enabled but hasn't been (re-)launched yet.
  if (!mProcess && gfxConfig::IsEnabled(Feature::GPU_PROCESS)) {
+    if (NS_WARN_IF(inShutdown)) {
+      return NS_ERROR_ILLEGAL_DURING_SHUTDOWN;
+    }
+
    if (!LaunchGPUProcess()) {
-      return false;
+      return NS_ERROR_NOT_AVAILABLE;
    }
  }

  if (mProcess && !mProcess->IsConnected()) {
+    if (NS_WARN_IF(inShutdown)) {
+      return NS_ERROR_ILLEGAL_DURING_SHUTDOWN;
+    }
+
    if (!mProcess->WaitForLaunch()) {
      // If this fails, we should have fired OnProcessLaunchComplete and
      // removed the process.
      MOZ_ASSERT(!mProcess && !mGPUChild);
-      return false;
+      return NS_ERROR_NOT_AVAILABLE;
    }
  }

  if (mGPUChild) {
    if (mGPUChild->EnsureGPUReady()) {
-      return true;
+      return NS_OK;
    }

    // If the initialization above fails, we likely have a GPU process teardown
@ -343,87 +358,106 @@ bool GPUProcessManager::EnsureGPUReady() {

  // This is the first time we are trying to use the in-process compositor.
  if (mTotalProcessAttempts == 0) {
+    if (NS_WARN_IF(inShutdown)) {
+      return NS_ERROR_ILLEGAL_DURING_SHUTDOWN;
+    }
    ResetProcessStable();
  }
-  return false;
+  return NS_ERROR_NOT_AVAILABLE;
 }

-void GPUProcessManager::EnsureProtocolsReady() {
-  EnsureCompositorManagerChild();
-  EnsureImageBridgeChild();
-  EnsureVRManager();
+bool GPUProcessManager::EnsureProtocolsReady() {
+  return EnsureCompositorManagerChild() && EnsureImageBridgeChild() &&
+         EnsureVRManager();
 }

-void GPUProcessManager::EnsureCompositorManagerChild() {
-  bool gpuReady = EnsureGPUReady();
-  if (CompositorManagerChild::IsInitialized(mProcessToken)) {
-    return;
+bool GPUProcessManager::EnsureCompositorManagerChild() {
+  nsresult rv = EnsureGPUReady();
+  if (NS_WARN_IF(rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN)) {
+    return false;
  }

-  if (!gpuReady) {
+  if (CompositorManagerChild::IsInitialized(mProcessToken)) {
+    return true;
+  }
+
+  if (NS_FAILED(rv)) {
    CompositorManagerChild::InitSameProcess(AllocateNamespace(), mProcessToken);
-    return;
+    return true;
  }

  ipc::Endpoint<PCompositorManagerParent> parentPipe;
  ipc::Endpoint<PCompositorManagerChild> childPipe;
-  nsresult rv = PCompositorManager::CreateEndpoints(
+  rv = PCompositorManager::CreateEndpoints(
      mGPUChild->OtherPid(), base::GetCurrentProcId(), &parentPipe, &childPipe);
  if (NS_FAILED(rv)) {
    DisableGPUProcess("Failed to create PCompositorManager endpoints");
-    return;
+    return true;
  }

  mGPUChild->SendInitCompositorManager(std::move(parentPipe));
  CompositorManagerChild::Init(std::move(childPipe), AllocateNamespace(),
                               mProcessToken);
+  return true;
 }

-void GPUProcessManager::EnsureImageBridgeChild() {
+bool GPUProcessManager::EnsureImageBridgeChild() {
  if (ImageBridgeChild::GetSingleton()) {
-    return;
+    return true;
  }

-  if (!EnsureGPUReady()) {
+  nsresult rv = EnsureGPUReady();
+  if (NS_WARN_IF(rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN)) {
+    return false;
+  }
+
+  if (NS_FAILED(rv)) {
    ImageBridgeChild::InitSameProcess(AllocateNamespace());
-    return;
+    return true;
  }

  ipc::Endpoint<PImageBridgeParent> parentPipe;
  ipc::Endpoint<PImageBridgeChild> childPipe;
-  nsresult rv = PImageBridge::CreateEndpoints(
+  rv = PImageBridge::CreateEndpoints(
      mGPUChild->OtherPid(), base::GetCurrentProcId(), &parentPipe, &childPipe);
  if (NS_FAILED(rv)) {
    DisableGPUProcess("Failed to create PImageBridge endpoints");
-    return;
+    return true;
  }

  mGPUChild->SendInitImageBridge(std::move(parentPipe));
  ImageBridgeChild::InitWithGPUProcess(std::move(childPipe),
                                       AllocateNamespace());
+  return true;
 }

-void GPUProcessManager::EnsureVRManager() {
+bool GPUProcessManager::EnsureVRManager() {
  if (VRManagerChild::IsCreated()) {
-    return;
+    return true;
  }

-  if (!EnsureGPUReady()) {
+  nsresult rv = EnsureGPUReady();
+  if (NS_WARN_IF(rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN)) {
+    return false;
+  }
+
+  if (NS_FAILED(rv)) {
    VRManagerChild::InitSameProcess();
-    return;
+    return true;
  }

  ipc::Endpoint<PVRManagerParent> parentPipe;
  ipc::Endpoint<PVRManagerChild> childPipe;
-  nsresult rv = PVRManager::CreateEndpoints(
+  rv = PVRManager::CreateEndpoints(
      mGPUChild->OtherPid(), base::GetCurrentProcId(), &parentPipe, &childPipe);
  if (NS_FAILED(rv)) {
    DisableGPUProcess("Failed to create PVRManager endpoints");
-    return;
+    return true;
  }

  mGPUChild->SendInitVRManager(std::move(parentPipe));
  VRManagerChild::InitWithGPUProcess(std::move(childPipe));
+  return true;
 }

 #if defined(MOZ_WIDGET_ANDROID)
@ -432,14 +466,19 @@ GPUProcessManager::CreateUiCompositorController(nsBaseWidget* aWidget,
                                                const LayersId aId) {
  RefPtr<UiCompositorControllerChild> result;

-  if (!EnsureGPUReady()) {
+  nsresult rv = EnsureGPUReady();
+  if (NS_WARN_IF(rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN)) {
+    return nullptr;
+  }
+
+  if (NS_FAILED(rv)) {
    result = UiCompositorControllerChild::CreateForSameProcess(aId, aWidget);
  } else {
    ipc::Endpoint<PUiCompositorControllerParent> parentPipe;
    ipc::Endpoint<PUiCompositorControllerChild> childPipe;
-    nsresult rv = PUiCompositorController::CreateEndpoints(
-        mGPUChild->OtherPid(), base::GetCurrentProcId(), &parentPipe,
-        &childPipe);
+    rv = PUiCompositorController::CreateEndpoints(mGPUChild->OtherPid(),
+                                                  base::GetCurrentProcId(),
+                                                  &parentPipe, &childPipe);
    if (NS_FAILED(rv)) {
      DisableGPUProcess("Failed to create PUiCompositorController endpoints");
      return nullptr;
@ -972,11 +1011,20 @@ already_AddRefed<CompositorSession> GPUProcessManager::CreateTopLevelCompositor(

  LayersId layerTreeId = AllocateLayerTreeId();

-  EnsureProtocolsReady();
+  if (!EnsureProtocolsReady()) {
+    *aRetryOut = false;
+    return nullptr;
+  }

  RefPtr<CompositorSession> session;

-  if (EnsureGPUReady()) {
+  nsresult rv = EnsureGPUReady();
+  if (NS_WARN_IF(rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN)) {
+    *aRetryOut = false;
+    return nullptr;
+  }
+
+  if (NS_SUCCEEDED(rv)) {
    session = CreateRemoteSession(aWidget, aLayerManager, layerTreeId, aScale,
                                  aOptions, aUseExternalSurfaceSize,
                                  aSurfaceSize, aInnerWindowId);
@ -998,6 +1046,7 @@ already_AddRefed<CompositorSession> GPUProcessManager::CreateTopLevelCompositor(
    // Nothing to do if controller gets a nullptr
    RefPtr<UiCompositorControllerChild> controller =
        CreateUiCompositorController(aWidget, session->RootLayerTreeId());
+    MOZ_ASSERT(controller);
    session->SetUiCompositorControllerChild(controller);
  }
 #endif  // defined(MOZ_WIDGET_ANDROID)
@ -1106,11 +1155,16 @@ bool GPUProcessManager::CreateContentCompositorManager(
  ipc::Endpoint<PCompositorManagerParent> parentPipe;
  ipc::Endpoint<PCompositorManagerChild> childPipe;

-  base::ProcessId parentPid =
-      EnsureGPUReady() ? mGPUChild->OtherPid() : base::GetCurrentProcId();
+  nsresult rv = EnsureGPUReady();
+  if (NS_WARN_IF(rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN)) {
+    return false;
+  }

-  nsresult rv = PCompositorManager::CreateEndpoints(parentPid, aOtherProcess,
-                                                    &parentPipe, &childPipe);
+  base::ProcessId parentPid =
+      NS_SUCCEEDED(rv) ? mGPUChild->OtherPid() : base::GetCurrentProcId();
+
+  rv = PCompositorManager::CreateEndpoints(parentPid, aOtherProcess,
+                                           &parentPipe, &childPipe);
  if (NS_FAILED(rv)) {
    gfxCriticalNote << "Could not create content compositor manager: "
                    << hexa(int(rv));
@ -1131,15 +1185,22 @@ bool GPUProcessManager::CreateContentCompositorManager(
 bool GPUProcessManager::CreateContentImageBridge(
    base::ProcessId aOtherProcess,
    ipc::Endpoint<PImageBridgeChild>* aOutEndpoint) {
-  EnsureImageBridgeChild();
+  if (!EnsureImageBridgeChild()) {
+    return false;
+  }
+
+  nsresult rv = EnsureGPUReady();
+  if (NS_WARN_IF(rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN)) {
+    return false;
+  }

  base::ProcessId parentPid =
-      EnsureGPUReady() ? mGPUChild->OtherPid() : base::GetCurrentProcId();
+      NS_SUCCEEDED(rv) ? mGPUChild->OtherPid() : base::GetCurrentProcId();

  ipc::Endpoint<PImageBridgeParent> parentPipe;
  ipc::Endpoint<PImageBridgeChild> childPipe;
-  nsresult rv = PImageBridge::CreateEndpoints(parentPid, aOtherProcess,
-                                              &parentPipe, &childPipe);
+  rv = PImageBridge::CreateEndpoints(parentPid, aOtherProcess, &parentPipe,
+                                     &childPipe);
  if (NS_FAILED(rv)) {
    gfxCriticalNote << "Could not create content compositor bridge: "
                    << hexa(int(rv));
@ -1167,15 +1228,22 @@ base::ProcessId GPUProcessManager::GPUProcessPid() {
 bool GPUProcessManager::CreateContentVRManager(
    base::ProcessId aOtherProcess,
    ipc::Endpoint<PVRManagerChild>* aOutEndpoint) {
-  EnsureVRManager();
+  if (NS_WARN_IF(!EnsureVRManager())) {
+    return false;
+  }
+
+  nsresult rv = EnsureGPUReady();
+  if (NS_WARN_IF(rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN)) {
+    return false;
+  }

  base::ProcessId parentPid =
-      EnsureGPUReady() ? mGPUChild->OtherPid() : base::GetCurrentProcId();
+      NS_SUCCEEDED(rv) ? mGPUChild->OtherPid() : base::GetCurrentProcId();

  ipc::Endpoint<PVRManagerParent> parentPipe;
  ipc::Endpoint<PVRManagerChild> childPipe;
-  nsresult rv = PVRManager::CreateEndpoints(parentPid, aOtherProcess,
-                                            &parentPipe, &childPipe);
+  rv = PVRManager::CreateEndpoints(parentPid, aOtherProcess, &parentPipe,
+                                   &childPipe);
  if (NS_FAILED(rv)) {
    gfxCriticalNote << "Could not create content compositor bridge: "
                    << hexa(int(rv));
@ -1197,7 +1265,12 @@ bool GPUProcessManager::CreateContentVRManager(
 void GPUProcessManager::CreateContentRemoteDecoderManager(
    base::ProcessId aOtherProcess,
    ipc::Endpoint<PRemoteDecoderManagerChild>* aOutEndpoint) {
-  if (!EnsureGPUReady() || !StaticPrefs::media_gpu_process_decoder() ||
+  nsresult rv = EnsureGPUReady();
+  if (NS_WARN_IF(rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN)) {
+    return;
+  }
+
+  if (NS_FAILED(rv) || !StaticPrefs::media_gpu_process_decoder() ||
      !mDecodeVideoOnGpuProcess) {
    return;
  }
@ -1205,7 +1278,7 @@ void GPUProcessManager::CreateContentRemoteDecoderManager(
  ipc::Endpoint<PRemoteDecoderManagerParent> parentPipe;
  ipc::Endpoint<PRemoteDecoderManagerChild> childPipe;

-  nsresult rv = PRemoteDecoderManager::CreateEndpoints(
+  rv = PRemoteDecoderManager::CreateEndpoints(
      mGPUChild->OtherPid(), aOtherProcess, &parentPipe, &childPipe);
  if (NS_FAILED(rv)) {
    gfxCriticalNote << "Could not create content video decoder: "
@ -1221,14 +1294,24 @@ void GPUProcessManager::CreateContentRemoteDecoderManager(
 void GPUProcessManager::InitVideoBridge(
    ipc::Endpoint<PVideoBridgeParent>&& aVideoBridge,
    layers::VideoBridgeSource aSource) {
-  if (EnsureGPUReady()) {
+  nsresult rv = EnsureGPUReady();
+  if (NS_WARN_IF(rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN)) {
+    return;
+  }
+
+  if (NS_SUCCEEDED(rv)) {
    mGPUChild->SendInitVideoBridge(std::move(aVideoBridge), aSource);
  }
 }

 void GPUProcessManager::MapLayerTreeId(LayersId aLayersId,
                                       base::ProcessId aOwningId) {
-  if (EnsureGPUReady()) {
+  nsresult rv = EnsureGPUReady();
+  if (NS_WARN_IF(rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN)) {
+    return;
+  }
+
+  if (NS_SUCCEEDED(rv)) {
    mGPUChild->SendAddLayerTreeIdMapping(
        LayerTreeIdMapping(aLayersId, aOwningId));
  }
@ -1241,7 +1324,12 @@ void GPUProcessManager::MapLayerTreeId(LayersId aLayersId,

 void GPUProcessManager::UnmapLayerTreeId(LayersId aLayersId,
                                         base::ProcessId aOwningId) {
-  if (EnsureGPUReady()) {
+  nsresult rv = EnsureGPUReady();
+  if (NS_WARN_IF(rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN)) {
+    return;
+  }
+
+  if (NS_SUCCEEDED(rv)) {
    mGPUChild->SendRemoveLayerTreeIdMapping(
        LayerTreeIdMapping(aLayersId, aOwningId));
  } else {
@ -1346,7 +1434,7 @@ void GPUProcessManager::RemoveListener(GPUProcessListener* aListener) {
 }

 bool GPUProcessManager::NotifyGpuObservers(const char* aTopic) {
-  if (!EnsureGPUReady()) {
+  if (NS_FAILED(EnsureGPUReady())) {
    return false;
  }
  nsCString topic(aTopic);
@ -1402,7 +1490,7 @@ class GPUMemoryReporter : public MemoryReportingProcess {
 RefPtr<MemoryReportingProcess> GPUProcessManager::GetProcessMemoryReporter() {
  // Ensure mProcess is non-null before calling EnsureGPUReady, to avoid
  // launching the process if it has not already been launched.
-  if (!mProcess || !EnsureGPUReady()) {
+  if (!mProcess || NS_FAILED(EnsureGPUReady())) {
    return nullptr;
  }
  return new GPUMemoryReporter();
--- a/gfx/ipc/GPUProcessManager.h
+++ b/gfx/ipc/GPUProcessManager.h
@ -96,7 +96,7 @@ class GPUProcessManager final : public GPUProcessHost::Listener {
  // If the GPU process is enabled but has not yet been launched then this will
  // launch the process. If that is not desired then check that return value of
  // Process() is non-null before calling.
-  bool EnsureGPUReady();
+  nsresult EnsureGPUReady();

  already_AddRefed<CompositorSession> CreateTopLevelCompositor(
      nsBaseWidget* aWidget, WebRenderLayerManager* aLayerManager,
@ -284,10 +284,10 @@ class GPUProcessManager final : public GPUProcessHost::Listener {
  void EnsureVsyncIOThread();
  void ShutdownVsyncIOThread();

-  void EnsureProtocolsReady();
-  void EnsureCompositorManagerChild();
-  void EnsureImageBridgeChild();
-  void EnsureVRManager();
+  bool EnsureProtocolsReady();
+  bool EnsureCompositorManagerChild();
+  bool EnsureImageBridgeChild();
+  bool EnsureVRManager();

 #if defined(XP_WIN)
  void SetProcessIsForeground();
--- a/gfx/thebes/gfxPlatform.cpp
+++ b/gfx/thebes/gfxPlatform.cpp
@ -25,6 +25,7 @@
 #include "mozilla/gfx/CanvasManagerParent.h"
 #include "mozilla/gfx/CanvasRenderThread.h"
 #include "mozilla/ClearOnShutdown.h"
+#include "mozilla/DebugOnly.h"
 #include "mozilla/EnumTypeTraits.h"
 #include "mozilla/StaticPrefs_accessibility.h"
 #include "mozilla/StaticPrefs_apz.h"
@ -3775,7 +3776,8 @@ void gfxPlatform::BuildContentDeviceData(
  MOZ_ASSERT(XRE_IsParentProcess());

  // Make sure our settings are synchronized from the GPU process.
-  GPUProcessManager::Get()->EnsureGPUReady();
+  DebugOnly<nsresult> rv = GPUProcessManager::Get()->EnsureGPUReady();
+  MOZ_ASSERT(rv != NS_ERROR_ILLEGAL_DURING_SHUTDOWN);

  aOut->prefs().hwCompositing() = gfxConfig::GetValue(Feature::HW_COMPOSITING);
  aOut->prefs().oglCompositing() =
--- a/widget/GfxInfoBase.cpp
+++ b/widget/GfxInfoBase.cpp
@ -1919,7 +1919,8 @@ GfxInfoBase::ControlGPUProcessForXPCShell(bool aEnable, bool* _retval) {
    if (!gfxConfig::IsEnabled(gfx::Feature::GPU_PROCESS)) {
      gfxConfig::UserForceEnable(gfx::Feature::GPU_PROCESS, "xpcshell-test");
    }
-    gpm->EnsureGPUReady();
+    DebugOnly<nsresult> rv = gpm->EnsureGPUReady();
+    MOZ_ASSERT(rv != NS_ERROR_ILLEGAL_DURING_SHUTDOWN);
  } else {
    gfxConfig::UserDisable(gfx::Feature::GPU_PROCESS, "xpcshell-test");
    gpm->KillProcess();
--- a/widget/nsBaseWidget.cpp
+++ b/widget/nsBaseWidget.cpp
@ -1368,7 +1368,10 @@ already_AddRefed<WebRenderLayerManager> nsBaseWidget::CreateCompositorSession(
    // Make sure GPU process is ready for use.
    // If it failed to connect to GPU process, GPU process usage is disabled in
    // EnsureGPUReady(). It could update gfxVars and gfxConfigs.
-    gpu->EnsureGPUReady();
+    nsresult rv = gpu->EnsureGPUReady();
+    if (NS_WARN_IF(rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN)) {
+      return nullptr;
+    }

    // If widget type does not supports acceleration, we may be allowed to use
    // software WebRender instead.