From 43bb3444bc80b28dc31a7a53f98362cf7718eced Mon Sep 17 00:00:00 2001
From: Chris Martin <cmartin@mozilla.com>
Date: Tue, 15 Dec 2020 00:01:19 +0000
Subject: [PATCH] Bug 1681750 - Fix random
 GamepadPlatformService::RemoveChannelParent crashes r=haik

The most likely cause for the crashes seems like IPDL intermittently calling GamepadEventChannelParent::ActorDestroy() multiple times on the same object.

This adds a wrapper to stop that behavior (if it's the root cause), and also to fire off release assertions at several other potential causes.

Hopefully we see this crash signature disappear.

Differential Revision: https://phabricator.services.mozilla.com/D99720
---
 dom/gamepad/GamepadPlatformService.cpp        | 16 +++++++++++-----
 dom/gamepad/ipc/GamepadEventChannelParent.cpp |  8 +++++++-
 dom/gamepad/ipc/GamepadEventChannelParent.h   |  1 +
 3 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/dom/gamepad/GamepadPlatformService.cpp b/dom/gamepad/GamepadPlatformService.cpp
index 70a72d887010..10e9d7e03c6b 100644
--- a/dom/gamepad/GamepadPlatformService.cpp
+++ b/dom/gamepad/GamepadPlatformService.cpp
@@ -264,7 +264,7 @@ void GamepadPlatformService::AddChannelParentInternal(
     const RefPtr<GamepadEventChannelParent>& aParent) {
   MutexAutoLock autoLock(mMutex);
 
-  MOZ_ASSERT(!mChannelParents.Contains(aParent));
+  MOZ_RELEASE_ASSERT(!mChannelParents.Contains(aParent));
   mChannelParents.AppendElement(aParent);
 
   // Inform the new channel of all the gamepads that have already been added
@@ -279,7 +279,7 @@ bool GamepadPlatformService::RemoveChannelParentInternal(
     GamepadEventChannelParent* aParent) {
   MutexAutoLock autoLock(mMutex);
 
-  MOZ_ASSERT(mChannelParents.Contains(aParent));
+  MOZ_RELEASE_ASSERT(mChannelParents.Contains(aParent));
 
   // If there is only one channel left, we destroy the singleton instead of
   // unregistering the channel
@@ -318,7 +318,8 @@ void GamepadPlatformService::RemoveChannelParent(
   // is created or removed in Background thread
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(aParent);
-  MOZ_ASSERT(gGamepadPlatformServiceSingleton);
+
+  MOZ_RELEASE_ASSERT(gGamepadPlatformServiceSingleton);
 
   // RemoveChannelParentInternal will refuse to remove the last channel
   // In that case, we should destroy the singleton
@@ -328,9 +329,14 @@ void GamepadPlatformService::RemoveChannelParent(
 
   GamepadMonitoringState::GetSingleton().Set(false);
   StopGamepadMonitoring();
+  // At this point, any monitor threads should be stopped so we don't need
+  // synchronization
 
-  // At this point, any monitor threads should be stopped and the only
-  // reference to the singleton should be the global one
+  // We should never be destroying the singleton with event channels left in it
+  MOZ_RELEASE_ASSERT(
+      gGamepadPlatformServiceSingleton->mChannelParents.Length() == 1);
+
+  // The only reference to the singleton should be the global one
   MOZ_RELEASE_ASSERT(gGamepadPlatformServiceSingleton->mRefCnt.get() == 1);
 
   gGamepadPlatformServiceSingleton = nullptr;
diff --git a/dom/gamepad/ipc/GamepadEventChannelParent.cpp b/dom/gamepad/ipc/GamepadEventChannelParent.cpp
index c4a521f7a9f0..12976478cb49 100644
--- a/dom/gamepad/ipc/GamepadEventChannelParent.cpp
+++ b/dom/gamepad/ipc/GamepadEventChannelParent.cpp
@@ -56,7 +56,13 @@ GamepadEventChannelParent::GamepadEventChannelParent() {
 void GamepadEventChannelParent::ActorDestroy(ActorDestroyReason aWhy) {
   AssertIsOnBackgroundThread();
 
-  GamepadPlatformService::RemoveChannelParent(this);
+  // TODO - This is here because I suspect that IPDL is calling ActorDestroy
+  // multiple times. If this fixes the random crashes in Bug 1681750, it might
+  // be worth investigating further.
+  if (!mShutdown) {
+    GamepadPlatformService::RemoveChannelParent(this);
+    mShutdown = true;
+  }
 }
 
 mozilla::ipc::IPCResult GamepadEventChannelParent::RecvVibrateHaptic(
diff --git a/dom/gamepad/ipc/GamepadEventChannelParent.h b/dom/gamepad/ipc/GamepadEventChannelParent.h
index ac0879eb904b..4a825931ee7e 100644
--- a/dom/gamepad/ipc/GamepadEventChannelParent.h
+++ b/dom/gamepad/ipc/GamepadEventChannelParent.h
@@ -41,6 +41,7 @@ class GamepadEventChannelParent final : public PGamepadEventChannelParent {
   GamepadEventChannelParent();
   ~GamepadEventChannelParent() = default;
 
+  bool mShutdown{false};
   nsCOMPtr<nsIEventTarget> mBackgroundEventTarget;
 };