Fix possible race condition crash in windows
authorChih-Yi Leu <cleu@mozilla.com>
Wed, 22 Jun 2016 15:12:50 +0800
changeset 785999 f791c1fa823533be6ca2af651cb12713127e5ee5
parent 785319 a67504ac36c0bbea8fabc1ee6493ad8c03e2de69
child 786000 fd5f105e63864927785c2c04dc1332cefb5f2b5b
child 786085 ce6c8d92367e7e7a6ef1c51b82041def051e4c78
push id130709
push usercleu@mozilla.com
push dateWed, 22 Jun 2016 09:17:12 +0000
treeherdertry@fd5f105e6386 [default view] [failures only]
milestone50.0a1
Fix possible race condition crash in windows
dom/gamepad/GamepadMonitoring.cpp
dom/gamepad/GamepadPlatformService.cpp
dom/gamepad/GamepadPlatformService.h
dom/gamepad/cocoa/CocoaGamepad.cpp
dom/gamepad/ipc/GamepadEventChannelParent.cpp
dom/gamepad/ipc/GamepadTestChannelParent.cpp
dom/gamepad/linux/LinuxGamepad.cpp
dom/gamepad/windows/WindowsGamepad.cpp
--- a/dom/gamepad/GamepadMonitoring.cpp
+++ b/dom/gamepad/GamepadMonitoring.cpp
@@ -12,17 +12,17 @@ using namespace mozilla::ipc;
 
 namespace mozilla {
 namespace dom {
 
 void
 MaybeStopGamepadMonitoring()
 {
   AssertIsOnBackgroundThread();
-  GamepadPlatformService* service =
+  RefPtr<GamepadPlatformService> service =
     GamepadPlatformService::GetParentService();
   MOZ_ASSERT(service);
   if(service->HasGamepadListeners()) {
     return;
   }
   StopGamepadMonitoring();
   service->ResetGamepadIndexes();
   service->MaybeShutdown();
--- a/dom/gamepad/GamepadPlatformService.cpp
+++ b/dom/gamepad/GamepadPlatformService.cpp
@@ -19,40 +19,42 @@ using namespace mozilla::ipc;
 
 namespace mozilla {
 namespace dom {
 
 namespace {
 
 // This is the singleton instance of GamepadPlatformService, can be called
 // by both background and monitor thread.
-StaticAutoPtr<GamepadPlatformService> gGamepadPlatformServiceSingleton;
+StaticRefPtr<GamepadPlatformService> gGamepadPlatformServiceSingleton;
 
 } //namepsace
 
+
 GamepadPlatformService::GamepadPlatformService()
   : mGamepadIndex(0),
     mMutex("mozilla::dom::GamepadPlatformService")
 {}
 
 GamepadPlatformService::~GamepadPlatformService()
 {
   Cleanup();
 }
 
 // static
-GamepadPlatformService*
+already_AddRefed<GamepadPlatformService>
 GamepadPlatformService::GetParentService()
 {
   //GamepadPlatformService can only be accessed in parent process
   MOZ_ASSERT(XRE_IsParentProcess());
   if(!gGamepadPlatformServiceSingleton) {
     gGamepadPlatformServiceSingleton = new GamepadPlatformService();
   }
-  return gGamepadPlatformServiceSingleton;
+  RefPtr<GamepadPlatformService> service(gGamepadPlatformServiceSingleton);
+  return service.forget();
 }
 
 template<class T>
 void
 GamepadPlatformService::NotifyGamepadChange(const T& aInfo)
 {
   // This method is called by monitor populated in
   // platform-dependent backends
@@ -189,32 +191,32 @@ GamepadPlatformService::HasGamepadListen
 }
 
 void
 GamepadPlatformService::MaybeShutdown()
 {
   // This method is invoked in MaybeStopGamepadMonitoring when
   // an IPDL channel is going to be destroyed
   AssertIsOnBackgroundThread();
-
+  RefPtr<GamepadPlatformService> tmpGrab;
   bool isChannelParentEmpty;
   {
     MutexAutoLock autoLock(mMutex);
     isChannelParentEmpty = mChannelParents.IsEmpty();
-  }
-  if(isChannelParentEmpty) {
-    gGamepadPlatformServiceSingleton = nullptr;
+    if(isChannelParentEmpty) {
+      tmpGrab = gGamepadPlatformServiceSingleton;
+      gGamepadPlatformServiceSingleton = nullptr;
+    }
   }
 }
 
 void
 GamepadPlatformService::Cleanup()
 {
   // This method is called when GamepadPlatformService is
   // successfully distructed in background thread
   AssertIsOnBackgroundThread();
 
-  MutexAutoLock autoLock(mMutex);
   mChannelParents.Clear();
 }
 
 } // namespace dom
 } // namespace mozilla
--- a/dom/gamepad/GamepadPlatformService.h
+++ b/dom/gamepad/GamepadPlatformService.h
@@ -26,20 +26,20 @@ class GamepadEventChannelParent;
 //    This thread takes charge of IPDL communications
 //    between here and DOM side
 //
 // 2. Monitor Thread:
 //    This thread is populated in platform-dependent backends, which
 //    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,
                       uint32_t aNumButtons, uint32_t aNumAxes);
   // Remove the gamepad at |aIndex| from the list of known gamepads.
   void RemoveGamepad(uint32_t aIndex);
 
   // Update the state of |aButton| for the gamepad at |aIndex| for all
@@ -67,16 +67,17 @@ class GamepadPlatformService final
   void RemoveChannelParent(GamepadEventChannelParent* aParent);
 
   bool HasGamepadListeners();
 
   void MaybeShutdown();
 
  private:
   GamepadPlatformService();
+  ~GamepadPlatformService();
   template<class T> void NotifyGamepadChange(const T& aInfo);
   void Cleanup();
 
   // mGamepadIndex can only be accessed by monitor thread
   uint32_t mGamepadIndex;
 
   // mChannelParents stores all the GamepadEventChannelParent instances
   // which may be accessed by both background thread and monitor thread
--- a/dom/gamepad/cocoa/CocoaGamepad.cpp
+++ b/dom/gamepad/cocoa/CocoaGamepad.cpp
@@ -271,30 +271,30 @@ DarwinGamepadService::DeviceAdded(IOHIDD
   int vendorId, productId;
   CFNumberGetValue(vendorIdRef, kCFNumberIntType, &vendorId);
   CFNumberGetValue(productIdRef, kCFNumberIntType, &productId);
   char product_name[128];
   CFStringGetCString(productRef, product_name,
                      sizeof(product_name), kCFStringEncodingASCII);
   char buffer[256];
   sprintf(buffer, "%x-%x-%s", vendorId, productId, product_name);
-  GamepadPlatformService* service =
+  RefPtr<GamepadPlatformService> service =
     GamepadPlatformService::GetParentService();
   MOZ_ASSERT(service);
   uint32_t index = service->AddGamepad(buffer,
                                        mozilla::dom::GamepadMappingType::_empty,
                                        (int)mGamepads[slot].numButtons(),
                                        (int)mGamepads[slot].numAxes());
   mGamepads[slot].mSuperIndex = index;
 }
 
 void
 DarwinGamepadService::DeviceRemoved(IOHIDDeviceRef device)
 {
-  GamepadPlatformService* service =
+  RefPtr<GamepadPlatformService> service =
     GamepadPlatformService::GetParentService();
   MOZ_ASSERT(service);
   for (size_t i = 0; i < mGamepads.size(); i++) {
     if (mGamepads[i] == device) {
       service->RemoveGamepad(mGamepads[i].mSuperIndex);
       mGamepads[i].clear();
       return;
     }
@@ -346,17 +346,17 @@ DarwinGamepadService::InputValueChanged(
   uint32_t value_length = IOHIDValueGetLength(value);
   if (value_length > 4) {
     // Workaround for bizarre issue with PS3 controllers that try to return
     // massive (30+ byte) values and crash IOHIDValueGetIntegerValue
     return;
   }
   IOHIDElementRef element = IOHIDValueGetElement(value);
   IOHIDDeviceRef device = IOHIDElementGetDevice(element);
-  GamepadPlatformService* service =
+  RefPtr<GamepadPlatformService> service =
     GamepadPlatformService::GetParentService();
   MOZ_ASSERT(service);
   for (unsigned i = 0; i < mGamepads.size(); i++) {
     Gamepad &gamepad = mGamepads[i];
     if (gamepad == device) {
       if (gamepad.isDpad(element)) {
         const dpad_buttons& oldState = gamepad.getDpadState();
         dpad_buttons newState = { false, false, false, false };
--- a/dom/gamepad/ipc/GamepadEventChannelParent.cpp
+++ b/dom/gamepad/ipc/GamepadEventChannelParent.cpp
@@ -37,17 +37,17 @@ class SendGamepadUpdateRunnable final : 
   }
 };
 
 } // namespace
 
 GamepadEventChannelParent::GamepadEventChannelParent()
   : mHasGamepadListener(false)
 {
-  GamepadPlatformService* service =
+  RefPtr<GamepadPlatformService> service =
     GamepadPlatformService::GetParentService();
   MOZ_ASSERT(service);
   service->AddChannelParent(this);
   mBackgroundThread = NS_GetCurrentThread();
 }
 
 bool
 GamepadEventChannelParent::RecvGamepadListenerAdded()
@@ -60,34 +60,34 @@ GamepadEventChannelParent::RecvGamepadLi
 }
 
 bool
 GamepadEventChannelParent::RecvGamepadListenerRemoved()
 {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(mHasGamepadListener);
   mHasGamepadListener = false;
-  GamepadPlatformService* service =
+  RefPtr<GamepadPlatformService> service =
     GamepadPlatformService::GetParentService();
   MOZ_ASSERT(service);
   service->RemoveChannelParent(this);
   Unused << Send__delete__(this);
   return true;
 }
 
 void
 GamepadEventChannelParent::ActorDestroy(ActorDestroyReason aWhy)
 {
   AssertIsOnBackgroundThread();
 
   // It may be called because IPDL child side crashed, we'll
   // not receive RecvGamepadListenerRemoved in that case
   if (mHasGamepadListener) {
     mHasGamepadListener = false;
-    GamepadPlatformService* service =
+    RefPtr<GamepadPlatformService> service =
       GamepadPlatformService::GetParentService();
     MOZ_ASSERT(service);
     service->RemoveChannelParent(this);
   }
   MaybeStopGamepadMonitoring();
 }
 
 void
--- a/dom/gamepad/ipc/GamepadTestChannelParent.cpp
+++ b/dom/gamepad/ipc/GamepadTestChannelParent.cpp
@@ -10,17 +10,17 @@
 namespace mozilla {
 namespace dom {
 
 bool
 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) {
     const GamepadAdded& a = aEvent.get_GamepadAdded();
     nsCString gamepadID;
     LossyCopyUTF16toASCII(a.id(), gamepadID);
     uint32_t index = service->AddGamepad(gamepadID.get(),
                                          (GamepadMappingType)a.mapping(),
--- a/dom/gamepad/linux/LinuxGamepad.cpp
+++ b/dom/gamepad/linux/LinuxGamepad.cpp
@@ -129,17 +129,17 @@ LinuxGamepadService::AddDevice(struct ud
   }
   snprintf(gamepad.idstring, sizeof(gamepad.idstring),
            "%s-%s-%s",
            vendor_id ? vendor_id : "unknown",
            model_id ? model_id : "unknown",
            name);
 
   char numAxes = 0, numButtons = 0;
-  GamepadPlatformService* service =
+  RefPtr<GamepadPlatformService> service =
     GamepadPlatformService::GetParentService();
   MOZ_ASSERT(service);
   ioctl(fd, JSIOCGAXES, &numAxes);
   gamepad.numAxes = numAxes;
   ioctl(fd, JSIOCGBUTTONS, &numButtons);
   gamepad.numButtons = numButtons;
 
   gamepad.index = service->AddGamepad(gamepad.idstring,
@@ -159,17 +159,17 @@ LinuxGamepadService::AddDevice(struct ud
 
 void
 LinuxGamepadService::RemoveDevice(struct udev_device* dev)
 {
   const char* devpath = mUdev.udev_device_get_devnode(dev);
   if (!devpath) {
     return;
   }
-  GamepadPlatformService* service =
+  RefPtr<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);
       service->RemoveGamepad(mGamepads[i].index);
       mGamepads.RemoveElementAt(i);
       break;
@@ -296,17 +296,17 @@ LinuxGamepadService::ReadUdevChange()
 
 // static
 gboolean
 LinuxGamepadService::OnGamepadData(GIOChannel* source,
                                    GIOCondition condition,
                                    gpointer data)
 {
   int index = GPOINTER_TO_INT(data);
-  GamepadPlatformService* service =
+  RefPtr<GamepadPlatformService> service =
     GamepadPlatformService::GetParentService();
   MOZ_ASSERT(service);
   //TODO: remove gamepad?
   if (condition & G_IO_ERR || condition & G_IO_HUP)
     return FALSE;
 
   while (true) {
     struct js_event event;
--- a/dom/gamepad/windows/WindowsGamepad.cpp
+++ b/dom/gamepad/windows/WindowsGamepad.cpp
@@ -438,17 +438,17 @@ WindowsGamepadService::ScanForXInputDevi
     }
     found = true;
     // See if this device is already present in our list.
     if (HaveXInputGamepad(i)) {
       continue;
     }
 
     // Not already present, add it.
-    GamepadPlatformService* service =
+    RefPtr<GamepadPlatformService> service =
       GamepadPlatformService::GetParentService();
     MOZ_ASSERT(service);
     Gamepad gamepad = {};
     gamepad.type = kXInputGamepad;
     gamepad.present = true;
     gamepad.state = state;
     gamepad.userIndex = i;
     gamepad.numButtons = kStandardGamepadButtons;
@@ -481,17 +481,17 @@ WindowsGamepadService::ScanForDevices()
                                          kXInputPollInterval,
                                          nsITimer::TYPE_ONE_SHOT);
     } else {
       mIsXInputMonitoring = false;
     }
   }
 
   // Look for devices that are no longer present and remove them.
-  GamepadPlatformService* service =
+  RefPtr<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);
       mGamepads.RemoveElementAt(i);
     }
   }
@@ -511,17 +511,17 @@ WindowsGamepadService::PollXInput()
         && state.dwPacketNumber != mGamepads[i].state.dwPacketNumber) {
         CheckXInputChanges(mGamepads[i], state);
     }
   }
 }
 
 void WindowsGamepadService::CheckXInputChanges(Gamepad& gamepad,
                                                XINPUT_STATE& state) {
-  GamepadPlatformService* service =
+  RefPtr<GamepadPlatformService> service =
     GamepadPlatformService::GetParentService();
   MOZ_ASSERT(service);
   // Handle digital buttons first
   for (size_t b = 0; b < kNumMappings; b++) {
     if (state.Gamepad.wButtons & kXIButtonMap[b].button &&
         !(gamepad.state.Gamepad.wButtons & kXIButtonMap[b].button)) {
       // Button pressed
       service->NewButtonEvent(gamepad.id, kXIButtonMap[b].mapped, true);
@@ -718,17 +718,17 @@ WindowsGamepadService::GetRawGamepad(HAN
       break;
     }
     gamepad.axes[i].caps = axes[i];
   }
   gamepad.type = kRawInputGamepad;
   gamepad.handle = handle;
   gamepad.present = true;
 
-  GamepadPlatformService* service =
+  RefPtr<GamepadPlatformService> service =
     GamepadPlatformService::GetParentService();
   MOZ_ASSERT(service);
   gamepad.id = service->AddGamepad(gamepad_id,
                                    GamepadMappingType::_empty,
                                    gamepad.numButtons,
                                    gamepad.numAxes);
   mGamepads.AppendElement(gamepad);
   return true;
@@ -768,17 +768,17 @@ WindowsGamepadService::HandleRawInput(HR
   nsTArray<uint8_t> parsedbytes;
   if (!GetPreparsedData(raw->header.hDevice, parsedbytes)) {
     return false;
   }
   PHIDP_PREPARSED_DATA parsed =
     reinterpret_cast<PHIDP_PREPARSED_DATA>(parsedbytes.Elements());
 
   // Get all the pressed buttons.
-  GamepadPlatformService* service =
+  RefPtr<GamepadPlatformService> service =
     GamepadPlatformService::GetParentService();
   MOZ_ASSERT(service);
   nsTArray<USAGE> usages(gamepad->numButtons);
   usages.SetLength(gamepad->numButtons);
   ULONG usageLength = gamepad->numButtons;
   if (mHID.mHidP_GetUsages(HidP_Input, kButtonUsagePage, 0, usages.Elements(),
                      &usageLength, parsed, (PCHAR)raw->data.hid.bRawData,
                      raw->data.hid.dwSizeHid) != HIDP_STATUS_SUCCESS) {