Bug 1221730 - Postpone singleton release in GamepadPlatformService::MaybeShutdown. r=baku

--HG--
extra : rebase_source : 4031bb8e791fa1c915261f3b2eaece28ebadb283

dom/gamepad/GamepadMonitoring.cpp

@@ -17,7 +17,7 @@ void
 MaybeStopGamepadMonitoring()
 {
   AssertIsOnBackgroundThread();
-  GamepadPlatformService* service =
+  RefPtr<GamepadPlatformService> service =
     GamepadPlatformService::GetParentService();
   MOZ_ASSERT(service);
   if(service->HasGamepadListeners()) {

dom/gamepad/GamepadPlatformService.cpp

@@ -24,7 +24,7 @@ namespace {
 
 // This is the singleton instance of GamepadPlatformService, can be called
 // by both background and monitor thread.
-StaticAutoPtr<GamepadPlatformService> gGamepadPlatformServiceSingleton;
+StaticRefPtr<GamepadPlatformService> gGamepadPlatformServiceSingleton;
 
 } //namepsace
 
@@ -39,15 +39,21 @@ GamepadPlatformService::~GamepadPlatformService()
 }
 
 // static
-GamepadPlatformService*
+already_AddRefed<GamepadPlatformService>
 GamepadPlatformService::GetParentService()
 {
   //GamepadPlatformService can only be accessed in parent process
   MOZ_ASSERT(XRE_IsParentProcess());
-  if(!gGamepadPlatformServiceSingleton) {
-    gGamepadPlatformServiceSingleton = new GamepadPlatformService();
+  if (!gGamepadPlatformServiceSingleton) {
+    // Only Background Thread can create new GamepadPlatformService instance.
+    if (IsOnBackgroundThread()) {
+      gGamepadPlatformServiceSingleton = new GamepadPlatformService();
+    } else {
+      return nullptr;
+    }
   }
-  return gGamepadPlatformServiceSingleton;
+  RefPtr<GamepadPlatformService> service(gGamepadPlatformServiceSingleton);
+  return service.forget();
 }
 
 template<class T>
@@ -195,13 +201,21 @@ GamepadPlatformService::MaybeShutdown()
   // an IPDL channel is going to be destroyed
   AssertIsOnBackgroundThread();
 
+  // We have to release gGamepadPlatformServiceSingleton ouside
+  // the mutex as well as making upcoming GetParentService() call
+  // recreate new singleton, so we use this RefPtr to temporarily
+  // hold the reference, postponing the release process until this
+  // method ends.
+  RefPtr<GamepadPlatformService> kungFuDeathGrip;
+
   bool isChannelParentEmpty;
   {
     MutexAutoLock autoLock(mMutex);
     isChannelParentEmpty = mChannelParents.IsEmpty();
-  }
-  if(isChannelParentEmpty) {
-    gGamepadPlatformServiceSingleton = nullptr;
+    if(isChannelParentEmpty) {
+      kungFuDeathGrip = gGamepadPlatformServiceSingleton;
+      gGamepadPlatformServiceSingleton = nullptr;
+    }
   }
 }
 

dom/gamepad/GamepadPlatformService.h

@@ -31,10 +31,10 @@ class GamepadEventChannelParent;
 //    is in charge of processing gamepad hardware events from OS
 class GamepadPlatformService final
 {
+  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(GamepadPlatformService)
  public:
-  ~GamepadPlatformService();
   //Get the singleton service
-  static GamepadPlatformService* GetParentService();
+  static already_AddRefed<GamepadPlatformService> GetParentService();
 
   // Add a gamepad to the list of known gamepads, and return its index.
   uint32_t AddGamepad(const char* aID, GamepadMappingType aMapping,
@@ -72,6 +72,7 @@ class GamepadPlatformService final
 
  private:
   GamepadPlatformService();
+  ~GamepadPlatformService();
   template<class T> void NotifyGamepadChange(const T& aInfo);
   void Cleanup();
 

dom/gamepad/cocoa/CocoaGamepad.cpp

@@ -247,6 +247,12 @@ class DarwinGamepadServiceStartupRunnable final : public Runnable
 void
 DarwinGamepadService::DeviceAdded(IOHIDDeviceRef device)
 {
+  RefPtr<GamepadPlatformService> service =
+    GamepadPlatformService::GetParentService();
+  if (!service) {
+    return;
+  }
+
   size_t slot = size_t(-1);
   for (size_t i = 0; i < mGamepads.size(); i++) {
     if (mGamepads[i] == device)
@@ -276,9 +282,6 @@ DarwinGamepadService::DeviceAdded(IOHIDDeviceRef device)
                      sizeof(product_name), kCFStringEncodingASCII);
   char buffer[256];
   sprintf(buffer, "%x-%x-%s", vendorId, productId, product_name);
-  GamepadPlatformService* service =
-    GamepadPlatformService::GetParentService();
-  MOZ_ASSERT(service);
   uint32_t index = service->AddGamepad(buffer,
                                        mozilla::dom::GamepadMappingType::_empty,
                                        (int)mGamepads[slot].numButtons(),
@@ -289,9 +292,11 @@ DarwinGamepadService::DeviceAdded(IOHIDDeviceRef device)
 void
 DarwinGamepadService::DeviceRemoved(IOHIDDeviceRef device)
 {
-  GamepadPlatformService* service =
+  RefPtr<GamepadPlatformService> service =
     GamepadPlatformService::GetParentService();
-  MOZ_ASSERT(service);
+  if (!service) {
+    return;
+  }
   for (size_t i = 0; i < mGamepads.size(); i++) {
     if (mGamepads[i] == device) {
       service->RemoveGamepad(mGamepads[i].mSuperIndex);
@@ -343,6 +348,12 @@ UnpackDpad(int dpad_value, int min, int max, dpad_buttons& buttons)
 void
 DarwinGamepadService::InputValueChanged(IOHIDValueRef value)
 {
+  RefPtr<GamepadPlatformService> service =
+    GamepadPlatformService::GetParentService();
+  if (!service) {
+    return;
+  }
+
   uint32_t value_length = IOHIDValueGetLength(value);
   if (value_length > 4) {
     // Workaround for bizarre issue with PS3 controllers that try to return
@@ -351,9 +362,7 @@ DarwinGamepadService::InputValueChanged(IOHIDValueRef value)
   }
   IOHIDElementRef element = IOHIDValueGetElement(value);
   IOHIDDeviceRef device = IOHIDElementGetDevice(element);
-  GamepadPlatformService* service =
-    GamepadPlatformService::GetParentService();
-  MOZ_ASSERT(service);
+
   for (unsigned i = 0; i < mGamepads.size(); i++) {
     Gamepad &gamepad = mGamepads[i];
     if (gamepad == device) {

dom/gamepad/ipc/GamepadEventChannelParent.cpp

@@ -42,7 +42,7 @@ class SendGamepadUpdateRunnable final : public Runnable
 GamepadEventChannelParent::GamepadEventChannelParent()
   : mHasGamepadListener(false)
 {
-  GamepadPlatformService* service =
+  RefPtr<GamepadPlatformService> service =
     GamepadPlatformService::GetParentService();
   MOZ_ASSERT(service);
   service->AddChannelParent(this);
@@ -65,7 +65,7 @@ GamepadEventChannelParent::RecvGamepadListenerRemoved()
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(mHasGamepadListener);
   mHasGamepadListener = false;
-  GamepadPlatformService* service =
+  RefPtr<GamepadPlatformService> service =
     GamepadPlatformService::GetParentService();
   MOZ_ASSERT(service);
   service->RemoveChannelParent(this);
@@ -82,7 +82,7 @@ GamepadEventChannelParent::ActorDestroy(ActorDestroyReason aWhy)
   // not receive RecvGamepadListenerRemoved in that case
   if (mHasGamepadListener) {
     mHasGamepadListener = false;
-    GamepadPlatformService* service =
+    RefPtr<GamepadPlatformService> service =
       GamepadPlatformService::GetParentService();
     MOZ_ASSERT(service);
     service->RemoveChannelParent(this);

dom/gamepad/ipc/GamepadTestChannelParent.cpp

@@ -15,7 +15,7 @@ GamepadTestChannelParent::RecvGamepadTestEvent(const uint32_t& aID,
                                                const GamepadChangeEvent& aEvent)
 {
   mozilla::ipc::AssertIsOnBackgroundThread();
-  GamepadPlatformService*  service =
+  RefPtr<GamepadPlatformService>  service =
     GamepadPlatformService::GetParentService();
   MOZ_ASSERT(service);
   if (aEvent.type() == GamepadChangeEvent::TGamepadAdded) {

dom/gamepad/linux/LinuxGamepad.cpp

@@ -86,6 +86,12 @@ LinuxGamepadService* gService = nullptr;
 void
 LinuxGamepadService::AddDevice(struct udev_device* dev)
 {
+  RefPtr<GamepadPlatformService> service =
+    GamepadPlatformService::GetParentService();
+  if (!service) {
+    return;
+  }
+
   const char* devpath = mUdev.udev_device_get_devnode(dev);
   if (!devpath) {
     return;
@@ -134,9 +140,6 @@ LinuxGamepadService::AddDevice(struct udev_device* dev)
            name);
 
   char numAxes = 0, numButtons = 0;
-  GamepadPlatformService* service =
-    GamepadPlatformService::GetParentService();
-  MOZ_ASSERT(service);
   ioctl(fd, JSIOCGAXES, &numAxes);
   gamepad.numAxes = numAxes;
   ioctl(fd, JSIOCGBUTTONS, &numButtons);
@@ -160,13 +163,17 @@ LinuxGamepadService::AddDevice(struct udev_device* dev)
 void
 LinuxGamepadService::RemoveDevice(struct udev_device* dev)
 {
+  RefPtr<GamepadPlatformService> service =
+    GamepadPlatformService::GetParentService();
+  if (!service) {
+    return;
+  }
+
   const char* devpath = mUdev.udev_device_get_devnode(dev);
   if (!devpath) {
     return;
   }
-  GamepadPlatformService* service =
-    GamepadPlatformService::GetParentService();
-  MOZ_ASSERT(service);
+
   for (unsigned int i = 0; i < mGamepads.Length(); i++) {
     if (strcmp(mGamepads[i].devpath, devpath) == 0) {
       g_source_remove(mGamepads[i].source_id);
@@ -300,10 +307,12 @@ LinuxGamepadService::OnGamepadData(GIOChannel* source,
                                    GIOCondition condition,
                                    gpointer data)
 {
-  int index = GPOINTER_TO_INT(data);
-  GamepadPlatformService* service =
+  RefPtr<GamepadPlatformService> service =
     GamepadPlatformService::GetParentService();
-  MOZ_ASSERT(service);
+  if (!service) {
+    return TRUE;
+  }
+  int index = GPOINTER_TO_INT(data);
   //TODO: remove gamepad?
   if (condition & G_IO_ERR || condition & G_IO_HUP)
     return FALSE;

dom/gamepad/windows/WindowsGamepad.cpp

@@ -431,6 +431,12 @@ WindowsGamepadService::ScanForXInputDevices()
   MOZ_ASSERT(mXInput, "XInput should be present!");
 
   bool found = false;
+  RefPtr<GamepadPlatformService> service =
+    GamepadPlatformService::GetParentService();
+  if (!service) {
+    return found;
+  }
+
   for (int i = 0; i < XUSER_MAX_COUNT; i++) {
     XINPUT_STATE state = {};
     if (mXInput.mXInputGetState(i, &state) != ERROR_SUCCESS) {
@@ -443,9 +449,6 @@ WindowsGamepadService::ScanForXInputDevices()
     }
 
     // Not already present, add it.
-    GamepadPlatformService* service =
-      GamepadPlatformService::GetParentService();
-    MOZ_ASSERT(service);
     Gamepad gamepad = {};
     gamepad.type = kXInputGamepad;
     gamepad.present = true;
@@ -466,6 +469,12 @@ WindowsGamepadService::ScanForXInputDevices()
 void
 WindowsGamepadService::ScanForDevices()
 {
+  RefPtr<GamepadPlatformService> service =
+    GamepadPlatformService::GetParentService();
+  if (!service) {
+    return;
+  }
+
   for (int i = mGamepads.Length() - 1; i >= 0; i--) {
     mGamepads[i].present = false;
   }
@@ -486,9 +495,6 @@ WindowsGamepadService::ScanForDevices()
   }
 
   // Look for devices that are no longer present and remove them.
-  GamepadPlatformService* service =
-    GamepadPlatformService::GetParentService();
-  MOZ_ASSERT(service);
   for (int i = mGamepads.Length() - 1; i >= 0; i--) {
     if (!mGamepads[i].present) {
       service->RemoveGamepad(mGamepads[i].id);
@@ -516,9 +522,11 @@ WindowsGamepadService::PollXInput()
 
 void WindowsGamepadService::CheckXInputChanges(Gamepad& gamepad,
                                                XINPUT_STATE& state) {
-  GamepadPlatformService* service =
+  RefPtr<GamepadPlatformService> service =
     GamepadPlatformService::GetParentService();
-  MOZ_ASSERT(service);
+  if (!service) {
+    return;
+  }
   // Handle digital buttons first
   for (size_t b = 0; b < kNumMappings; b++) {
     if (state.Gamepad.wButtons & kXIButtonMap[b].button &&
@@ -586,6 +594,12 @@ public:
 bool
 WindowsGamepadService::GetRawGamepad(HANDLE handle)
 {
+  RefPtr<GamepadPlatformService> service =
+    GamepadPlatformService::GetParentService();
+  if (!service) {
+    return true;
+  }
+
   if (!mHID) {
     return false;
   }
@@ -722,10 +736,6 @@ WindowsGamepadService::GetRawGamepad(HANDLE handle)
   gamepad.type = kRawInputGamepad;
   gamepad.handle = handle;
   gamepad.present = true;
-
-  GamepadPlatformService* service =
-    GamepadPlatformService::GetParentService();
-  MOZ_ASSERT(service);
   gamepad.id = service->AddGamepad(gamepad_id,
                                    GamepadMappingType::_empty,
                                    gamepad.numButtons,
@@ -741,6 +751,12 @@ WindowsGamepadService::HandleRawInput(HRAWINPUT handle)
     return false;
   }
 
+  RefPtr<GamepadPlatformService> service =
+    GamepadPlatformService::GetParentService();
+  if (service) {
+    return true;
+  }
+
   // First, get data from the handle
   UINT size;
   GetRawInputData(handle, RID_INPUT, nullptr, &size, sizeof(RAWINPUTHEADER));
@@ -773,9 +789,6 @@ WindowsGamepadService::HandleRawInput(HRAWINPUT handle)
     reinterpret_cast<PHIDP_PREPARSED_DATA>(parsedbytes.Elements());
 
   // Get all the pressed buttons.
-  GamepadPlatformService* service =
-    GamepadPlatformService::GetParentService();
-  MOZ_ASSERT(service);
   nsTArray<USAGE> usages(gamepad->numButtons);
   usages.SetLength(gamepad->numButtons);
   ULONG usageLength = gamepad->numButtons;

widget/android/nsAppShell.cpp

@@ -934,9 +934,11 @@ nsAppShell::LegacyGeckoEvent::Run()
 
     case AndroidGeckoEvent::GAMEPAD_ADDREMOVE: {
 #ifdef MOZ_GAMEPAD
-            GamepadPlatformService* service;
+            RefPtr<GamepadPlatformService> service;
             service = GamepadPlatformService::GetParentService();
-            MOZ_ASSERT(service);
+            if (!service) {
+              break;
+            }
             if (curEvent->Action() == AndroidGeckoEvent::ACTION_GAMEPAD_ADDED) {
               int svc_id = service->AddGamepad("android",
                                                dom::GamepadMappingType::Standard,
@@ -954,9 +956,11 @@ nsAppShell::LegacyGeckoEvent::Run()
     case AndroidGeckoEvent::GAMEPAD_DATA: {
 #ifdef MOZ_GAMEPAD
             int id = curEvent->ID();
-            GamepadPlatformService* service;
+            RefPtr<GamepadPlatformService> service;
             service = GamepadPlatformService::GetParentService();
-            MOZ_ASSERT(service);
+            if (!service) {
+              break;
+            }
             if (curEvent->Action() == AndroidGeckoEvent::ACTION_GAMEPAD_BUTTON) {
               service->NewButtonEvent(id, curEvent->GamepadButton(),
                                       curEvent->GamepadButtonPressed(),